feat: first-run activation menu + bundled dbt sample - #1046
Merged
sahrizvi merged 23 commits intoJul 29, 2026
Merged
Conversation
added 12 commits
July 24, 2026 19:33
New shipping asset at `packages/opencode/sample-projects/jaffle-shop-duckdb/`.
Ships alongside the wrapper npm package (publish.ts wiring lands in the
next commit); a runtime resolver in `sample-source-resolver.ts` finds it
across dev / test / dist install layouts. A `/starter` slash command (Phase 4)
materializes a copy into a user-owned directory so a fresh user can walk a
real dbt project without connecting a datasource.
**Shape:**
- `dbt_project.yml` / `profiles.yml` — DuckDB profile with project-relative
path, so no host paths bake into the shipped source.
- 4 models (2 staging, 2 marts) + 2 seed CSVs — jaffle-shop layout,
forked from the dbt-tools test fixture and given its own life so a
test-fixture edit can't accidentally change the shipped sample.
- `models/staging/schema.yml` + `models/marts/schema.yml` — per-column
descriptions + `unique` / `not_null` / `relationships` tests, so the
static reviewer surfaces (/discover, /review) have real material to
work with.
- `sample-manifest.json` — version-stamped project metadata used by
the marker-based conflict-detection when the sample is materialized to
the user's filesystem (Phase 4).
- `target/manifest.json` — pre-compiled dbt manifest committed alongside
the source. Ships so that static workflows work with ZERO external
tools installed (no dbt-core, no dbt-duckdb needed for /discover or
/review). Sanitized to strip host paths (replaced with
`{{SAMPLE_ROOT}}` sentinels) and to zero timestamps + invocation_id so
regenerations are deterministic.
- `regenerate.sh` — maintainer script that re-runs `dbt compile` +
sanitizes + stages the manifest. Run after editing sample source; the
freshness test (Phase 5) will fail if source changes without a matching
manifest refresh.
**Not shipped:** `target/graph.gpickle` (Python pickle, no JS consumer),
`target/catalog.json` (warehouse introspection with env-specific
metadata), `target/run_results.json` (run-specific, no static value).
Wire the shipping side of the starter sample so the wrapper npm package
carries it and runtime code can find it across install layouts.
**publish.ts::copyAssets** — copies `packages/opencode/sample-projects/`
into the wrapper package alongside the existing `bin/` and `skills/`
copies. Only the shippable subset of `target/` (manifest.json) is copied
— the rest is excluded so a stale `dbt build` on a materialized user
copy can't contaminate the shipped source.
**sample-source-resolver.ts** — runtime lookup for the shipped sample.
Hunts across four candidate layouts so a single code path works in dev
(bun run src/index.ts), test (bun test), production (compiled bun exe
under `<wrapper>/bin/altimate-code`), and less-common install layouts
(pnpm content-addressable, npx cache) where the exe sits two hops from
the wrapper root. `ALTIMATE_STARTER_SAMPLE_DIR` env override is honored
first — used by tests to point at a fixture and by users pointing at a
hand-curated fork of the sample.
Also exports `loadShippedManifest()` — reads the pre-compiled
`target/manifest.json` and rehydrates the `{{SAMPLE_ROOT}}` sentinels
with the user's materialized target path. This is what the static
review-pipeline consumers (/discover, /review) call to walk the sample
DAG without dbt on PATH.
…eview fixes
Applied four gaps from the Phase 3 codex adversarial review:
1. **Resolver honors symlinked `process.execPath`.** npm global installs
symlink the binary from `/usr/local/bin/altimate-code` to the actual
location under `lib/node_modules/`, Homebrew uses `libexec`, pnpm uses
`.bin` shims. The previous resolver walked from the shim's dirname and
would miss `../sample-projects/`. Now `fs.realpathSync(process.execPath)`
first, then hunt from the real location.
2. **`loadShippedManifest` JSON-safe rehydration.** The prior text-level
substitution of `{{SAMPLE_ROOT}}` sentinels corrupted JSON if the
materialized target path contained JSON-significant characters. A path
like `/tmp/a"b` broke the JSON with an unescaped double-quote; a
Windows path like `C:\Users\name\sample` produced invalid `\U` escape
sequences. Now: parse the manifest first, walk the tree with a new
exported `rehydrateSentinels()`, substitute ONLY inside string leaf
values, preserve object keys / numbers / booleans / nulls. Same fix
applied in `regenerate.sh` so a maintainer's home directory can't
accidentally sentinelize legitimate content (a model description or
compiled SQL literal that happens to contain the maintainer's absolute
path).
3. **`generated_at` pinned to a plausible past date instead of the epoch.**
Zero-epoch (`1970-01-01`) is a trap for downstream freshness-check /
staleness-detection tooling that may treat it as pathological. Fixed
to `2026-07-24T00:00:00Z` — bumped only when the maintainer wants to
signal a manifest-shape refresh, not on every regenerate.
4. **Longest-sentinel-first replace order.** In `rehydrateSentinels` the
`{{SAMPLE_ROOT_PARENT}}` sentinel is substituted before
`{{SAMPLE_ROOT}}` so the shorter one can't match inside the longer
one's expansion window.
Windows `cp`/`mkdir` portability in `publish.ts::copyAssets` (codex point
#3) was DEFERRED: matches the existing shell-based pattern used elsewhere
in the file, and release CI runs on Ubuntu only. Track as a follow-up
when the release moves off Ubuntu.
…r, materialize, tools, KV) Phase 4a — the non-UI logic behind the activation prompt and /starter slash command. Adds five modules under `packages/opencode/src/altimate/onboarding/`: - **kv-keys.ts** — four KV keys (`onboarding.activation.dismissed_at`, `.completed_choice`, `onboarding.sample_project.path`, `.version`) plus the `ActivationChoice` enum. Keys deliberately split so a support engineer can inspect each state independently and so KV / on-disk marker divergence stays reasonable to debug. - **tool-detection.ts** — `detectDbtRuntime()` probes `dbt --version` once per process and parses its plugin list for the "duckdb" adapter line. Cached for the process lifetime; force-refresh available for tests. Post-materialize UX filters "run" options (dbt build, live query) when `hasDbtDuckdb` is false, so shipped commands can't silently fail on users without the Python side installed. - **marker.ts** — `.altimate-sample.json` marker semantics. `classifyTarget(dir, version)` returns one of `empty` / `our-sample-current` / `our-sample-different-version` / `unknown-dir`. `findSafeTarget()` walks the `<name>`, `<name>-2`, `<name>-3` sequence until a non-unknown-dir slot appears — never overwrites an unknown directory. Marker is the AUTHORITATIVE source of truth for filesystem safety; the KV path is merely a convenience index (per codex feedback: "marker wins on divergence"). - **materialize.ts** — copies the shipped sample source into the user's chosen target. Whitelist-driven (no wholesale recursive copy) so future contributor scratch files don't accidentally leak into user installs. Enforces the marker-based conflict policy from marker.ts. `rejectUnsafeHome()` refuses to materialize into `/root` (sudo mistake), `/tmp/*` (ephemeral runner), or `/` (misconfigured container) with an actionable error the caller surfaces verbatim to the user. - **detection.ts** — `detectUsableSetup(cwd)` returns `"usable" | "detected-not-usable" | "nothing"` for ordering the three options in the activation dialog. Wraps the existing `detectDbtProject()` primitive with a targeted `profiles.yml` regex scan that respects dbt's precedence (project-local → `$DBT_PROFILES_DIR` → `~/.dbt/`). Doesn't validate credentials — that would require a warehouse handshake we're not spending on activation. Phase 4b (next commit) wires the TUI dialog, the slash commands, and the app.tsx / onboard-connect.txt integration points that call into this logic.
Four adjustments from the Phase 4a codex adversarial pass:
1. **detection.ts — broadened profile-key regex.** The old regex
`^<name>:\s*$` only matched an unquoted, non-commented top-level key
with nothing after the colon — false-negatived quoted keys
(`"jaffle-shop":`), inline mappings (`jaffle_shop: {target: dev}`),
and trailing comments (`jaffle_shop: # local`). New pattern accepts
optional matching single/double quotes and any trailing content.
Known Jinja-wrapped / anchor-referenced false-negatives are
documented as accepted for v1 — verdict impact is only option
ordering (dialog still shows every choice), so erring on the
sample-side is the safer bias.
2. **marker.ts — attemptLimit 10 → 100 + randomized fallback.** A user
with `altimate-sample-dbt-{1..10}` (retries, benchmark runs, support
copies) would previously get a hard `Error: No safe target found`
at activation time. Now the loop tries 100 numeric slots; if all
are held by unrelated content, one final randomized `-<6hex>` slot
is attempted. Only after both fall through does it throw — the
collision odds on the randomized slot alone are ~1-in-16.7M, so the
only realistic path to failure is genuine environmental hostility.
3. **marker.ts + materialize.ts — parent-writable pre-check.** New
`checkParentWritable()` in marker.ts is called in materialize.ts
BEFORE candidate hunting. Turns a raw `EACCES` from `mkdirSync`
deep in the copy step into a clear "Target parent directory X is
not writable: <reason>" error the caller can surface. Handles
read-only enterprise homes, NFS glitches, container mounts.
4. **materialize.ts — README.md added to MATERIALIZE_ENTRIES.** New
`sample-projects/jaffle-shop-duckdb/README.md` documents what's
inside the sample and what to try (works-with-zero-tools vs
needs-dbt-duckdb). Codex flagged that shipping a sample directory
with no accompanying "what next" reading material was a
context-loss risk. README is also copied into the wrapper package
via publish.ts.
5. **tool-detection.ts — call-site guidance for cache staleness.**
Docstring on `detectDbtRuntime` now explicitly documents WHEN
callers must pass `{ force: true }` (after materialization, before
any run-workflow invocation). This is Phase 4b's problem to obey,
but calling it out here makes the intent visible in the module.
Refactor: `MaterializeResult.suffixIndex` → `suffix` (renamed for the
new `number | string` shape now that randomized fallback is a possible
value).
Phase 4b — the customer-facing surfaces that consume Phase 4a's core logic. Adds three touch-points: **1. `DialogActivation` (packages/tui/src/component/dialog-activation.tsx)** The 3-option picker that fires when the scan-gate "No" path resolves and also from the `/activation` slash command. Custom `<box>` layout matching the visual style of `DialogScanGate` and `DialogModelWelcome` (part 1 + part 2a of onboarding). Keyboard: 1/2/3 shortcuts, up/down + Enter, Escape → dismissed. On any selection: persists both `onboarding.activation.completed_choice` and `onboarding.activation.dismissed_at` in KV so the dialog does not auto-fire on future launches, then calls the injected `onChoose` callback. Options ordered by an async `detectUsableSetup(cwd)` probe: a `dbt_project.yml` + resolvable `profiles.yml` verdict leads with "Connect data"; otherwise "Open sample project" is first. `packages/tui/src/altimate/onboarding/` — TUI-side `kv-keys.ts` + `detection.ts` colocated here because the TUI package cannot import from `packages/opencode` (workspace dep is `@opencode-ai/core` only). Detection is a small self-contained fs walk mirroring `detectDbtProject()` from opencode-side project-scan. **2. `app.tsx` scan-gate "No" → open DialogActivation** The scan-gate at app.tsx:585 previously hardcoded `if (arg === "skip") return` — dropped the user into an empty chat with no continuation. Now the "skip" branch calls `dialog.replace(() => <DialogActivation ... />)` with a shared `dispatchActivationChoice` handler that routes each of the four possible choices to the right follow-up: `connect_data` runs `/onboard-connect scan`, `sample_project` runs `/starter`, `describe_use_case` prefills the prompt buffer with a starter hint (does NOT auto-submit — user finishes the sentence), `dismissed` is a no-op with an already-persisted KV timestamp. **3. Slash commands: `/starter` + `/activation`** - `starter.txt` — LLM template that invokes the (Phase 4a) materialize logic, then reports the result with one of three branch outputs: reused, fresh, or fresh-with-suffix. Post-materialize UX suggestions are strictly static-workflow only (dbt-duckdb install caveat goes at the end for users who want live queries). - `activation.txt` — one-line placeholder. The real work happens in `appCommands` where a slash-name registration intercepts `/activation` BEFORE it reaches the LLM and directly reopens the dialog. Escape hatch for users who dismissed the dialog too early — addresses the design consult's #1 concern ("if 'skip all' hides every useful recovery affordance, users are stranded"). Both commands are registered in `packages/opencode/src/command/index.ts` under `Default.STARTER` and `Default.ACTIVATION`, joining the existing `/onboard-connect` follow-up family. **4. `onboard-connect.txt` branch 4 → mentions `/starter`** The "genuinely nothing yet" branch of the scan template previously advertised "scaffold a project" as an aspirational offer with nothing behind it. Now it explicitly points at `/starter` alongside "cd into your project and run /discover" and "paste SQL / describe your use case" — the same three routes the activation dialog offers, so both entry points (scan-gate "No" AND scan-found-nothing) converge on the same next-action set.
Four fixes from the Phase 4b codex adversarial pass: 1. **Rename TUI detection → `tui-detection.ts`** to make the ownership split explicit: this module is DISPLAY ORDER ONLY, not an authoritative "usable setup" verdict. Opencode-side consumers (agents, tools, /discover flows) must NOT import from here — they own their own detection surface. Rewrote the file header to spell this out so a future contributor doesn't wire this into a slash command by accident. 2. **Gate keyboard input on detection completion** in `DialogActivation`. `selected` starts at -1 while `detectUsableSetup()` is in flight; only Escape works during that window. This closes a race where a user with a valid dbt setup could Enter on the sample-first fallback ordering (because verdict was undefined) and end up on /starter when "Connect data" was the intended default. Detection is ~100ms so the "Checking local project…" label is one-frame territory. Also documented the `process.cwd()` assumption + wrapper-installer edge case for the Phase 5 e2e test to cover. 3. **Prefill wording** for the "Describe your own use case" path changed from the sentence fragment `"I'd like to "` to `"Describe what you're trying to do: "`. If a user accidentally hits Enter on the preamble, the LLM still receives a coherent question rather than a broken fragment. Comment now also flags the `parts: []` clear as a low-risk-but-non-zero draft-loss for future recovery-flow work. 4. **`/activation` template becomes a real non-TUI fallback.** Previous version claimed the dialog was already open even when invoked in headless / ACP / `--print` mode where the TUI intercept doesn't fire — the model would then advertise a picker that didn't exist. Now the template presents the three choices as a plain-text list and asks the user to pick by name. The TUI palette intercept still short-circuits this template entirely for interactive sessions, so the fallback only renders where it's needed.
…ple-source-resolver Four unit test files landing 47 tests total, plus an off-by-one fix in the resolver that the first test-run flushed out. **marker.test.ts** — 22 tests covering `readMarker`/`writeMarker` round-trip, the four `classifyTarget` decision-table branches (empty / our-current / our-different-version / unknown-dir), and `findSafeTarget`'s numeric-suffix loop + randomized-hex fallback when all numbered slots are held by unrelated content. Plus the codex-flagged `checkParentWritable` pre-check with writable and nonexistent parents. **materialize.test.ts** — 8 tests. Fresh copy verifies all whitelisted files land + profiles.yml is intact + marker is correct. Second-call reuse-detection asserts no re-copy + no marker rewrite (materialization timestamps preserved). Preferred-collision → `-2` suffix without touching user's original file. Version-bump paths cover both the "prompt before upgrading" (allowInPlaceUpgrade=false) and "upgrade-in-place" (allowInPlaceUpgrade=true) branches. Unsafe HOME scenarios exercised via `rejectUnsafeHome` (undefined, `/`, `/tmp/*`, `/root` under non-root uid). **sample-source-resolver.test.ts** — 12 tests. Env override path, default dev-source-tree path, and the JSON-safe `rehydrateSentinels` tree walk with adversarial inputs codex flagged in Phase 3 review: strings with double quotes (`/tmp/a"b`), Windows backslash paths (`C:\Users\...`), object keys that happen to match the sentinel literally (must be preserved), and the parent-then-root replace order (so the shorter sentinel can't shadow the longer one). Plus end-to-end `loadShippedManifest` against the real committed manifest — asserts no dangling sentinels after substitution AND the target path appears where expected. **tool-detection.test.ts** — 5 tests pinning the `dbt --version` parser regex against representative dbt 1.x outputs. Covers the codex- flagged "dbt present, no dbt-duckdb" scenario, the "'duckdb' as a substring in an upgrade hint (not a plugin line)" false-positive guard, and the strict-formatting requirement so bare-word "duckdb" without a colon never counts. **Resolver off-by-one fix**: the `dev-source-tree` candidate went 4 hops up from `packages/opencode/src/altimate/onboarding/` — which lands at `packages/` — but sample-projects lives at `packages/opencode/sample-projects/`. Fixed to 3 hops. The mistake was invisible to Phase 3's smoke path (it happened to fall through to a different candidate); Phase 5's tests forced a real assertion and exposed it. 47 pass, 0 fail. Full altimate suite: 3773 pass, 640 skip, 0 fail.
`starter-sample.tape` — VHS script that drives the demo. Renders `docs/media/starter-sample.gif` when run with `vhs docs/media/starter-sample.tape`. The rendered GIF is git-ignored (regenerable, ~700KB binary blob not worth committing to a public repo); reviewers render locally or view attachments on the PR. `starter-sample-demo.sh` — helper shell script that the tape shells out to. VHS's `Type` command can't reliably escape long nested-quote shell one-liners (bun -e with an inline JS string that imports materializeSample), so the tape delegates each demo step to a named action on this script. Also lets a reader `bash` this file directly for a non-recorded reproduction. **What the recorded demo shows** (in order): 1. `materialize` — fresh copy to `$HOME/altimate-sample-demo/` 2. `ls` — expected files (README, dbt_project.yml, profiles.yml, .altimate-sample.json marker, models/, seeds/, target/) 3. `find` — full tree layout 4. `cat .altimate-sample.json` — the conflict-detection marker 5. `wc -l target/manifest.json` — pre-compiled manifest present 6. README head — what-to-try guidance 7. Second `materialize` — reports `reused: true`, no re-copy **NOT in this recording** — the interactive TUI activation dialog itself. That path gates on `useReady()` which needs a live provider or the setupComplete signal from finishing OAuth; scripting it in VHS needs an auth-mock harness we don't have yet. Tracked as a follow-up. This recording proves the LEAF flow (sample materialization) works end-to-end against the shipped asset resolver + marker + copy logic committed in Phases 3 & 4a. Also gitignores `docs/media/*.gif` so future renders don't accidentally get staged.
…nd materializeSample
Fills the vaporware gap in the `/starter` slash-command flow: the
template at `packages/opencode/src/command/template/starter.txt`
already asks the LLM to *"Call the `starter_materialize` tool exactly
once, with no arguments"* — but I had never actually registered such a
tool. Users typing `/starter` would have hit an "unknown tool" error
and the whole materialization flow would silently fail.
**`packages/opencode/src/altimate/tools/starter-materialize.ts`** —
new `Tool.define("starter_materialize", ...)` modeled directly on
`feedback-submit.ts` (the canonical altimate-side pattern for a
slash-command tool that DOES something and returns structured metadata
for the template to branch on):
- Zod schema with 3 OPTIONAL parameters (preferred_target_name,
target_parent, allow_in_place_upgrade) — none required, so the
LLM's "no arguments" invocation works.
- Reads `sample-manifest.json`'s `version` field via the shipped
sample-source-resolver, so bumping the sample version auto-flows to
the marker without a code change here.
- Delegates to `materializeSample()` from Phase 4a; wraps its return
into the `{title, metadata: {targetPath, reused, suffix, note},
output}` shape the template's three branches consume.
- On failure (unsafe HOME, unwritable parent, missing source),
returns `{title, metadata: {error}, output: <actionable message>}`
— output text is passed through verbatim by the template.
**Registered** in `packages/opencode/src/tool/registry.ts` inside the
existing altimate_change block, right next to `FeedbackSubmitTool`.
Import + array entry.
**Test** at `packages/opencode/test/altimate/tools/starter-materialize.test.ts`
(5 tests, all pass). Covers each of the three success branches
(reused / fresh / suffixed), the version-mismatch prompt-hint branch,
and the failure-message passthrough. Uses the existing `initTool()`
fixture in `test/altimate/tool-fixture.ts` to unwrap the Effect-based
Tool.define into a plain `execute(args, ctx)` for assertion.
All 3778 altimate suite tests pass, typecheck clean.
This is the fix for the biggest gap I flagged in the last doubt list:
the /starter flow now actually works end-to-end. Precedent confirmed
via Sarav's `/onboard-connect` implementation (same pattern:
LLM-driven slash template + registered tool with structured return)
and via the `feedback-submit.ts` shape — both established before this
work.
The onboarding template we route to (packages/opencode/src/command/template/ onboard-connect.txt) refers to the sample bootstrap tool as sample_setup; align the tool's registered name so the LLM's tool call resolves. No behavior change — same materialization logic, same schema, same tests.
…dal dialog
The activation-menu design lands as agent-emitted text at the end of every
/onboard-connect branch, matching the ticket mockup: JTBD-worded options
(see downstream / review a SQL PR / try the sample project / describe
your own), rendered inline in the chat rather than a modal overlay.
Rationale: the modal DialogActivation approach we prototyped had two
practical blockers — auth-arrival timing meant the false→true transition
never fired for users with pre-loaded creds, and the fixed option-set
couldn't personalize per scan verdict. Emitting the menu from the
onboard-connect template dodges both, keeps warehouse-connected
personalization ("you've got 12 dbt models and a Snowflake connection…"),
and reuses existing skill routing (dbt-analyze, sql-review, cost-report,
sample_setup, dbt build, sql_execute) instead of standing up parallel
dispatch machinery.
Also handles the 'Build & query it' branch when dbt-duckdb isn't
installed: bash-probes for the adapter first and, if missing, surfaces
an actionable install instruction with two paths (paste an existing
dbt binary path, or run 'pip install dbt-duckdb'). Once available,
runs dbt build, then explicitly wires dbt-profiles → warehouse_add →
sql_execute so the DuckDB connection is registered before any query.
Scan-gate 'No' now dispatches /onboard-connect skip again (previously
close-only) so the template's skip branch runs and the menu emerges
naturally in the chat.
Removes: /starter and /activation slash commands + their templates,
the DialogActivation TUI component, KV keys + detection modules that
supported it, and the /starter VHS demo tape (no longer meaningful).
Also drops the stale KV_SAMPLE_PROJECT_PATH reference in marker.ts'
docstring.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
added 2 commits
July 28, 2026 11:29
…materialize
Codex review flagged two safety gaps in the LLM-facing sample_setup tool.
**Path traversal (P0)**
preferred_target_name was documented as a directory name but accepted
any trimmed string, then path.join'd directly to targetParent. A
prompt-injected model turn could pass '../foo' and escape the intended
parent.
- Regex allowlist on the Zod schema (tool boundary — the LLM sees the
contract): /^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/. Single segment,
no path separators, no leading dot.
- Same regex enforced in materializeSample as defense in depth (the
tool schema catches the LLM path, this catches other callers).
- Post-path.resolve containment check: resolvedTarget must equal
resolvedParent or start with resolvedParent + sep. Belt-and-suspenders
for any future findSafeTarget change.
- 19 adversarial tests: ../escape, absolute paths, hidden dirs,
whitespace, shell metachars, empty string — all rejected before any
fs write. Plus 7 explicit accepted-name tests for the happy shape.
**Interrupted materialize strands a broken dir (P1)**
Old flow: copySampleTree(target) → writeMarker(target). Crash between
steps leaves a markerless dir. Next run classifies it as unknown-dir,
suffixes into <name>-2, leaves the broken <name> in place forever.
- Atomic materialize: write to .<name>.tmp-<hex> staging, write marker
there, then fs.renameSync → final target. On POSIX rename is atomic;
a crash mid-copy leaves an orphan tmp dir (different name) instead
of a half-written target.
- sweepOrphanStaging at the start of each materialize removes any
prior orphans matching .<preferredName>.tmp-*. Scoped to the current
preferredName so a different sample's staging isn't touched.
- For allow_in_place_upgrade: remove the outdated old target only
AFTER the staging tree is fully written, right before the rename.
- 2 new tests cover the crash-cleanup path (pre-seeded orphan gets
swept, fresh materialize lands at <name>/ not <name>-2/) and the
scoping (other-sample orphans are left alone).
…e tests
Codex review flagged three more items on this branch.
**Template return-state branching (P1)**
sample_setup returns five distinct states (fresh, plain-reuse, version-
conflict-with-Caller-must-prompt, error, suffix-collision) but the
template only handled the happy path. LLM would present a stale sample
or a hard error as usable.
Rewrote the routing block so the LLM branches on metadata explicitly:
(a) metadata.error → surface the actionable message verbatim, no menu;
(b) reused=true + note contains 'Caller must prompt' → ask user to
reset/keep/install-alongside before calling with allow_in_place_upgrade;
(c) reused=true → 'already set up at <path>'; then menu;
(d) reused=false + suffix>0 → 'materialized alongside at <suffix path>';
then menu;
(e) reused=false + suffix=0 → 'created at <path>'; then menu.
Only branches c/d/e reach the SAMPLE menu.
**Skip-branch instruction conflict (P2)**
Skip branch said 'Respond with exactly \<opener\>' then 'Then act on
their answer', while the global rule at the bottom said the activation
menu is 'ALWAYS' appended after every branch. Ambiguous for the LLM
and the user's next reply.
Reworded: the opener AND the menu go out together in the same reply
(the opener frames it; the menu is the concrete next action). If the
user then answers free-text ('Snowflake', 'a dbt project', 'just
exploring'), handle it directly; if they pick a menu number, run the
routing.
**Vacuous tests (P2)**
- tool-detection.test.ts previously duplicated HAS_DBT_DUCKDB_RE +
VERSION_RE into the test file and asserted against those local
copies — the real detectDbtRuntime() was never invoked, so impl
drift wouldn't be caught. Rewrote to invoke detectDbtRuntime() for
real via a PATH-override + bash-script dbt stub. 8 tests covering
duckdb-present/absent, stderr-vs-stdout, non-zero exit, missing
binary, non-executable file, and cache honored-vs-forced.
- sample-source-resolver.test.ts's 'loads shipped manifest' test
returned early if resolveSampleSource() failed — passed trivially
under any resolver breakage. Replaced early-return with
expect(location).toBeDefined() so a broken resolver fails loud.
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
added 4 commits
July 28, 2026 14:39
…ckaging Round 1 (blocking): tool contract + safety + packaging - sample_setup: put a `status: ok|error` prefix in output so the model (which only reads output, never metadata) can branch reliably; add `install_alongside` and thread through materialize; drop `target_parent` from the LLM-facing schema (was a bypass of the rejectUnsafeHome guard). - materialize: allowlist preferred name (regex) + post-resolve containment check; atomic staging + rename; sentinel rehydration for target/manifest.json baked to the FINAL target path (not staging); Flock-serialized selection + write; re-classify before rmSync; age-guarded orphan sweep. - sample-source-resolver: add ALTIMATE_BIN_DIR candidate so Windows / --ignore-scripts / ALTIMATE_CODE_BIN_PATH installs still find the shipped sample. - publish: ship .gitignore for the sample project (materializer expects it); keep-in-sync note with MATERIALIZE_ENTRIES. - regenerate: sanitize identity / wall-clock fields (user_id, project_id, invocation_id, all created_at, generated_at, etc.) so the shipped manifest doesn't leak maintainer telemetry. - onboard-connect.txt: branch on status prefix (a/b/c/d/e); route install-alongside; drop shell-out in favour of the tool's `dbt:` line. Round 2 (correctness / concurrency / telemetry) - Tool metadata sets `success: false` on all failure paths so `core_failure` telemetry actually fires. - Template covers hex-string suffix branch (was numeric-only). - Template branch offers Reset / Keep / Install-alongside on version conflict; install-alongside plumbs through findSafeTarget with skipVersionMismatch. - detectDbtRuntime is called from sample_setup and its result surfaces as the `dbt:` line the template reads. Round 3 (smaller stuff) - classifyTarget: lstatSync so symlinked targets are classified unknown-dir instead of being followed (can't reuse-through-link or silently unlink). - rejectUnsafeHome: reject os.tmpdir() and Windows system dirs (SYSTEMROOT, PROGRAMFILES), not just /tmp/*. - findSafeTarget: bail out to hex fallback after N consecutive unknown-dir hits instead of burning ~100 stat syscalls. - profiles.yml: reword comment to say `path:` resolves against process CWD, not the project dir. - sample .gitignore: `target/*` (not `target/`) so the `!target/manifest.json` re-include is not inert. - sample_setup: resolve source once and thread through materialize (was doing the candidate hunt twice per call).
- materialize.test: split traversal `toThrow` alternation so the regex layer's message is asserted per name (was hiding whether the containment check ever fired). - materialize.test: default-`targetParent` test — pins os.homedir(), omits targetParent, asserts the sample lands under the pinned home. - materialize.test: symlinked preferred slot classified unknown-dir → suffixed to -2, symlink untouched (proves lstatSync branch). - materialize.test: 11 consecutive unknown-dir slots → bail-early to hex fallback (proves findSafeTarget's CONSECUTIVE_UNKNOWN_LIMIT). - materialize.test: existing tmp-parent tests now pass `allowUnsafeParent: true` — required because the tightened rejectUnsafeHome now refuses os.tmpdir() prefixes. - sample-setup.test: install_alongside branch — seeded 0.5.0 marker in slot 0, alongside call lands in slot -2, old slot untouched. - sample-setup.test: `dbt:` line exists in output and matches the documented shape (present / missing with adapter status) — protects the template's Build & query it branch from a silent regression. - sample-setup.test: makeTmpHome carves out a UUID-suffixed subdir of the real HOME (the tool has no allowUnsafeParent escape hatch on its LLM-facing schema). - verify-freshness.test.ts (in sample dir): freshness guard — every sha256-checksummed node in the shipped manifest matches its source file's dbt hash (rstrip-one-\n convention); identity + wall-clock fields scrubbed. - publish-parity.test.ts: assert publish.ts's copy list covers every MATERIALIZE_ENTRIES path so a future runtime whitelist addition can't ship in dev but be missing in prod.
Follow-up to the consensus-review Round 1–3 commits. A codex sweep of the
same four failure classes (path bypass, platform guards, fs.rm sites,
resource cleanup) surfaced two real gaps not in the panel's list:
- NEW-1/NEW-4 — `rejectUnsafeHome` was comparing raw strings before
realpath. A caller passing `/private/tmp/foo` on macOS bypassed both
`startsWith('/tmp/')` (because `/tmp` is a symlink to `/private/tmp`,
so the literal prefixes don't share bytes) and the `os.tmpdir()`
check (macOS reports `/var/folders/…` but its realpath is
`/private/var/folders/…`). Now canonicalizes both sides — matches
against `/tmp`, `realpath('/tmp')`, `os.tmpdir()`, and
`realpath(os.tmpdir())`. Test asserts a `/private/tmp/…` HOME is
refused on macOS.
- NEW-5 — `sweepOrphanStaging` used `fs.statSync` on discovered
entries. A symlinked `.starter.tmp-<hex>` would get the target's
mtime for age classification (not the link's own), which could
either misfire the age guard or cause the sweep to rm the symlink
over live content. Now uses `lstatSync` and skips symlinks entirely
— same class as finding 21 (which we already applied to
`classifyTarget`). Test seeds a backdated symlinked orphan and
asserts the sweep leaves it alone.
Node's `execFile("dbt")` on Windows uses CreateProcess, which honours
PATHEXT for `.exe` and `.com` but NOT `.cmd`/`.bat` — those need a shell.
Some Windows dbt install layouts (older `pip install --user` script
dirs, certain corporate distributions, some WSL-bridge shims) drop a
`dbt.cmd` wrapper on PATH rather than `dbt.exe`. Without a fallback,
those users would see `dbt: missing` on the template's "Build & query it"
branch even when dbt is installed and on PATH.
Fix: on Windows only, if the direct `execFile("dbt", ...)` misses, retry
through `cmd.exe /c dbt --version`. cmd's own resolver honours the full
PATHEXT so it finds any wrapper shape. Args are constant strings so
there's no injection surface. macOS/Linux keep the single shell-less
probe.
Codex flagged this in the class-B sweep of `execFile` call sites. No
Windows CI covers this branch yet, so verify manually if you touch it.
Contributor
Author
Review status — consensus + codex sweepReady for bot review. Local branch is at Fixed on this branch23 findings from the multi-model consensus review (safety, correctness, telemetry, tests) — see commits Codex sweep of the 4 rejected-claim classes — commits Deferred — on this branch's surface, chose not to fix nowBots may still want to flag these; posting here so the intent is transparent.
Pre-existing — not on this branch's surfaceCodex flagged these while auditing the rejection classes end-to-end. They live in
Follow-up test gap the PR doesn't cover
|
sahrizvi
marked this pull request as ready for review
July 28, 2026 11:22
![claude[bot]](https://cdn.github.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
![kilo-code-bot[bot]](https://cdn.github.com/in/2193792?s=60&v=4){kind=small}
| than `0` (a number like `1` or a hex string like `a1b2c3`) → The | ||
| preferred name was taken by unrelated content; the sample landed | ||
| at the suffixed path (see `path:`). Say: "Materialized the sample | ||
| at <path> (your existing directory wasn't ours, so I put it |
There was a problem hiding this comment.
[SUGGESTION]: Branch (d) wording is incorrect for the install-alongside path
Branch (b) routes an explicit "Install alongside" choice to sample_setup with install_alongside: true, then tells the agent to "follow branch (d) or (e)". That path materializes the new version into <name>-2 and returns reused: false + a non-zero suffix, which lands in branch (d). But the verbatim message here claims "your existing directory wasn't ours" — in the alongside case that directory IS our older-version sample (the user just chose to keep it), not unrelated content. After an explicit alongside choice this is misleading. Consider an alongside-specific follow-up (e.g. "Installed the new sample alongside your existing copy at ; your old version is untouched") rather than reusing branch (d)'s unrelated-content wording.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| // "/tmp") can't be realpathed as a directory on all systems so we | ||
| // include them raw; the canonical versions catch the /private/tmp | ||
| // and /private/var/folders/... bypasses. | ||
| const refs: string[] = ["/tmp", canonicalize("/tmp"), os.tmpdir(), canonicalize(os.tmpdir())] |
There was a problem hiding this comment.
[SUGGESTION]: refs recomputes realpathSync on each check() call
check() rebuilds refs, calling canonicalize("/tmp") and canonicalize(os.tmpdir()) (each a fs.realpathSync syscall) on every invocation. check is called up to twice here (once for canonicalHome, once for the raw home), and neither canonical reference depends on the candidate, so those two realpath calls run redundantly. Hoist refs (or just the two canonical values) out of check so they are computed once.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Incremental review of
The residual Files Reviewed (3 files)
Previous Review Summaries (5 snapshots, latest commit cb53255)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit cb53255)Status: 1 Issue Found | Recommendation: Address before merge Incremental review of
Overview
Issue Details (click to expand)WARNING
Files Reviewed (9 files)
Fix these issues in Kilo Cloud Previous review (commit 33f6e3a)Status: No Issues Found | Recommendation: Merge Incremental review of Files Reviewed (1 file)
Previous review (commit 6c8b445)Status: 1 Issue Found | Recommendation: Address before merge Incremental review of Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous review (commit 89028ff)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Incremental NotesNew commit
No new issues introduced in the incremental diff. Previous review (commit ae31698)Status: 2 Suggestions | Recommendation: Merge (non-blocking polish) Overview
Issue DetailsSUGGESTION
NotesSecurity-critical paths are well-hardened: Files Reviewed (10 source files)
Reviewed by glm-5.2 · Input: 63.1K · Output: 19.4K · Cached: 650.2K Review guidance: REVIEW.md from base branch |
![cubic-dev-ai[bot]](https://cdn.github.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
9 issues found across 32 files
Not reviewed (too large): packages/opencode/sample-projects/jaffle-shop-duckdb/target/manifest.json (~16,839 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/sample-projects/jaffle-shop-duckdb/sample-manifest.json">
<violation number="1" location="packages/opencode/sample-projects/jaffle-shop-duckdb/sample-manifest.json:7">
P2: `requires` field declares dbt version constraints that are never enforced at runtime — the manifest is the source of truth for sample version (`version`), but the `requires.dbt-core` and `requires.dbt-duckdb` constraints have no consumers. If someone has dbt-core 1.6.x or dbt-duckdb 2.x there's no early validation, just a late failure when the dbt commands themselves break. Consider either wiring the constraints into the tool-detection probe or documenting in a comment that they're informational-only.</violation>
</file>
<file name="packages/opencode/test/altimate/onboarding/sample-source-resolver.test.ts">
<violation number="1" location="packages/opencode/test/altimate/onboarding/sample-source-resolver.test.ts:46">
P2: Global state leak: `delete process.env["ALTIMATE_STARTER_SAMPLE_DIR"]` at line 206 mutates shared environment without restoration. Other test files running in parallel (bun's default) may read a missing var, causing flaky failures. The other tests in this file wrap env mutations in try/finally; this one should follow the same pattern.</violation>
</file>
<file name="packages/opencode/sample-projects/regenerate.sh">
<violation number="1" location="packages/opencode/sample-projects/regenerate.sh:9">
P2: Schema YAML edits can merge with a stale precompiled manifest: the advertised freshness guard only hashes SQL and seed files. Extend the guard to fingerprint `dbt_project.yml`, `profiles.yml`, and schema YAML (or remove the claim that every source edit is caught).</violation>
<violation number="2" location="packages/opencode/sample-projects/regenerate.sh:63">
P3: The sanitized manifest records node/macro `created_at` timestamps on 2026-07-25 while metadata timestamps are pinned to 2026-07-24, so consumers comparing these fields see an internally inconsistent generated-at timeline. Using the epoch corresponding to `FIXED_ISO` keeps the promised release-day timestamp consistent.</violation>
</file>
<file name="packages/opencode/sample-projects/jaffle-shop-duckdb/README.md">
<violation number="1" location="packages/opencode/sample-projects/jaffle-shop-duckdb/README.md:8">
P3: The file tree in the 'What's in here' section omits `.gitignore`, which is one of the shipped files (confirmed via `MATERIALIZE_ENTRIES` in `materialize.ts`). Users who materialize the sample will get a `.gitignore` without seeing it documented. Add it to the tree so the listing stays accurate.</violation>
</file>
<file name="packages/opencode/sample-projects/jaffle-shop-duckdb/models/marts/schema.yml">
<violation number="1" location="packages/opencode/sample-projects/jaffle-shop-duckdb/models/marts/schema.yml:21">
P2: orders model schema omits `customer_name`, `order_date`, and `amount` columns that the SQL model produces. Same discoverability gap as the customers model — these columns won't surface in dbt docs or the review UI.</violation>
</file>
<file name="packages/opencode/src/altimate/onboarding/materialize.ts">
<violation number="1" location="packages/opencode/src/altimate/onboarding/materialize.ts:402">
P2: An unrelated old file or directory named like `.<preferredTargetName>.tmp-*` is recursively deleted during setup. Restrict cleanup to staging directories that can be positively identified as materializer-owned, or leave ambiguous entries untouched.</violation>
</file>
<file name="packages/opencode/sample-projects/jaffle-shop-duckdb/verify-freshness.test.ts">
<violation number="1" location="packages/opencode/sample-projects/jaffle-shop-duckdb/verify-freshness.test.ts:16">
P2: The hash algorithm is described with two contradictory contracts: the block docstring says `rstrip("\n")` (ALL trailing newlines), the function JSDoc says only one trailing newline. The implementation strips one `\n`, matching the JSDoc but not the block comment. Pick one contract and align all three — or if single-strip is intentional, update the block docstring so future maintainers aren't misled about which algorithm the test actually implements.</violation>
</file>
<file name="packages/opencode/sample-projects/jaffle-shop-duckdb/profiles.yml">
<violation number="1" location="packages/opencode/sample-projects/jaffle-shop-duckdb/profiles.yml:3">
P3: Comment incorrectly states that dbt-duckdb resolves the `path` against the process working directory. Per the official dbt-duckdb documentation, the `path` is resolved relative to the profiles.yml file location by default. Since this profiles.yml lives at the project root, the practical behavior is the same (the database lands at `<project>/target/jaffle.duckdb` when running from the project directory), but the comment's explanation is wrong and will mislead anyone trying to understand or debug the path resolution if they run dbt from a different directory while pointing at this profiles.yml.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| "version": "1.0.0", | ||
| "kind": "altimate-starter-sample", | ||
| "source": "packages/opencode/sample-projects/jaffle-shop-duckdb", | ||
| "requires": { |
There was a problem hiding this comment.
P2: requires field declares dbt version constraints that are never enforced at runtime — the manifest is the source of truth for sample version (version), but the requires.dbt-core and requires.dbt-duckdb constraints have no consumers. If someone has dbt-core 1.6.x or dbt-duckdb 2.x there's no early validation, just a late failure when the dbt commands themselves break. Consider either wiring the constraints into the tool-detection probe or documenting in a comment that they're informational-only.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/sample-projects/jaffle-shop-duckdb/sample-manifest.json, line 7:
<comment>`requires` field declares dbt version constraints that are never enforced at runtime — the manifest is the source of truth for sample version (`version`), but the `requires.dbt-core` and `requires.dbt-duckdb` constraints have no consumers. If someone has dbt-core 1.6.x or dbt-duckdb 2.x there's no early validation, just a late failure when the dbt commands themselves break. Consider either wiring the constraints into the tool-detection probe or documenting in a comment that they're informational-only.</comment>
<file context>
@@ -0,0 +1,16 @@
+ "version": "1.0.0",
+ "kind": "altimate-starter-sample",
+ "source": "packages/opencode/sample-projects/jaffle-shop-duckdb",
+ "requires": {
+ "dbt-core": ">=1.7 <2.0",
+ "dbt-duckdb": ">=1.7 <2.0"
</file context>
|
|
||
| ## What's in here | ||
|
|
||
| ``` |
There was a problem hiding this comment.
P3: The file tree in the 'What's in here' section omits .gitignore, which is one of the shipped files (confirmed via MATERIALIZE_ENTRIES in materialize.ts). Users who materialize the sample will get a .gitignore without seeing it documented. Add it to the tree so the listing stays accurate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/sample-projects/jaffle-shop-duckdb/README.md, line 8:
<comment>The file tree in the 'What's in here' section omits `.gitignore`, which is one of the shipped files (confirmed via `MATERIALIZE_ENTRIES` in `materialize.ts`). Users who materialize the sample will get a `.gitignore` without seeing it documented. Add it to the tree so the listing stays accurate.</comment>
<file context>
@@ -0,0 +1,55 @@
+
+## What's in here
+
+```
+dbt_project.yml dbt project config
+profiles.yml DuckDB profile — path is project-relative
</file context>
| SENTINEL_ROOT = "{{SAMPLE_ROOT}}" | ||
| SENTINEL_PARENT = "{{SAMPLE_ROOT_PARENT}}" | ||
| FIXED_ISO = "2026-07-24T00:00:00Z" | ||
| FIXED_EPOCH = 1785000000.0 # 2026-07-24 near midnight UTC; matches FIXED_ISO closely enough |
There was a problem hiding this comment.
P3: The sanitized manifest records node/macro created_at timestamps on 2026-07-25 while metadata timestamps are pinned to 2026-07-24, so consumers comparing these fields see an internally inconsistent generated-at timeline. Using the epoch corresponding to FIXED_ISO keeps the promised release-day timestamp consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/sample-projects/regenerate.sh, line 63:
<comment>The sanitized manifest records node/macro `created_at` timestamps on 2026-07-25 while metadata timestamps are pinned to 2026-07-24, so consumers comparing these fields see an internally inconsistent generated-at timeline. Using the epoch corresponding to `FIXED_ISO` keeps the promised release-day timestamp consistent.</comment>
<file context>
@@ -0,0 +1,121 @@
+SENTINEL_ROOT = "{{SAMPLE_ROOT}}"
+SENTINEL_PARENT = "{{SAMPLE_ROOT_PARENT}}"
+FIXED_ISO = "2026-07-24T00:00:00Z"
+FIXED_EPOCH = 1785000000.0 # 2026-07-24 near midnight UTC; matches FIXED_ISO closely enough
+ZERO_UUID = "00000000-0000-0000-0000-000000000000"
+
</file context>
Comment on lines
+3
to
+8
| # `path:` is unqualified, so dbt-duckdb resolves it against the PROCESS | ||
| # working directory at build time — NOT the project directory. Run | ||
| # `dbt build` from the materialized sample dir (`cd <sample-path>`) and | ||
| # the database lands at `<sample-path>/target/jaffle.duckdb`. | ||
| # Run it from anywhere else and dbt writes to `$PWD/target/jaffle.duckdb`. | ||
| jaffle_shop: |
There was a problem hiding this comment.
P3: Comment incorrectly states that dbt-duckdb resolves the path against the process working directory. Per the official dbt-duckdb documentation, the path is resolved relative to the profiles.yml file location by default. Since this profiles.yml lives at the project root, the practical behavior is the same (the database lands at <project>/target/jaffle.duckdb when running from the project directory), but the comment's explanation is wrong and will mislead anyone trying to understand or debug the path resolution if they run dbt from a different directory while pointing at this profiles.yml.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/sample-projects/jaffle-shop-duckdb/profiles.yml, line 3:
<comment>Comment incorrectly states that dbt-duckdb resolves the `path` against the process working directory. Per the official dbt-duckdb documentation, the `path` is resolved relative to the profiles.yml file location by default. Since this profiles.yml lives at the project root, the practical behavior is the same (the database lands at `<project>/target/jaffle.duckdb` when running from the project directory), but the comment's explanation is wrong and will mislead anyone trying to understand or debug the path resolution if they run dbt from a different directory while pointing at this profiles.yml.</comment>
<file context>
@@ -0,0 +1,14 @@
+# DuckDB profile — everything runs locally against a single file at
+# `target/jaffle.duckdb` (created on first `dbt build`). No cloud credentials.
+# `path:` is unqualified, so dbt-duckdb resolves it against the PROCESS
+# working directory at build time — NOT the project directory. Run
+# `dbt build` from the materialized sample dir (`cd <sample-path>`) and
</file context>
Suggested change
| # `path:` is unqualified, so dbt-duckdb resolves it against the PROCESS | |
| # working directory at build time — NOT the project directory. Run | |
| # `dbt build` from the materialized sample dir (`cd <sample-path>`) and | |
| # the database lands at `<sample-path>/target/jaffle.duckdb`. | |
| # Run it from anywhere else and dbt writes to `$PWD/target/jaffle.duckdb`. | |
| jaffle_shop: | |
| # DuckDB profile — everything runs locally against a single file at | |
| # `target/jaffle.duckdb` (created on first `dbt build`). No cloud credentials. | |
| # `path:` is unqualified, so dbt-duckdb resolves it relative to this | |
| # `profiles.yml` location (the project directory). Run | |
| # `dbt build` from the materialized sample dir (`cd <sample-path>`) and | |
| # the database lands at `<sample-path>/target/jaffle.duckdb`. | |
| # Run it from anywhere else and behavior depends on where dbt finds this profile. |
added 2 commits
July 28, 2026 17:14
…hoist refs
- `onboard-connect.txt` — branch (b)'s "Install alongside" branch used
to instruct the model to follow branch (d) or (e) after the alongside
re-invocation. That was wrong for branch (d): its "your existing
directory wasn't ours, so I put it alongside" wording assumes the old
directory held unrelated content, but on the alongside path the old
directory IS ours (an older-version sample) and the user explicitly
kept it. Give the alongside path its own follow-up wording that names
both the new and old paths and does NOT reuse branch (d)'s message.
- `materialize.ts` — the tmp-ref canonicalization inside
`rejectUnsafeHome` was recomputing `realpathSync("/tmp")` and
`realpathSync(os.tmpdir())` on every `check()` call (up to twice per
invocation) even though neither depends on the candidate. Hoist the
`refs` array out of the closure so those two syscalls run once.
…mplate shell/path safety, manifest asset-set guard Bot review (cubic-dev-ai + a class-scoped codex re-sweep) surfaced three real bugs on this branch and one test-coverage gap: - cubic P1-1: `classifyTarget` was returning `our-sample-current` on any marker whose VERSION matched, ignoring `sampleName`. A future second bundled sample writing a marker with the same version would be silently reused/upgraded through as this sample. Add `expectedSampleName` gate in `classifyTarget`; threading through `findSafeTarget` and the reverify-before-rmSync call. Fallback slot and materialize reverify now both consult sample identity. Tests added for wrong-sampleName → unknown-dir + symlink and unreadable-dir branches (which were previously untested at the marker.test.ts level even though materialize.test.ts covered symlink end-to-end). - cubic P1-2: template's "user pastes a dbt path" branch interpolated the pasted string directly into a bash pipeline. A paste like `; rm -rf ~` would execute. Rewrite the branch to (a) refuse paths containing shell metacharacters, (b) refuse non-executable files, (c) verify via a single-quoted `'<path>' --version` invocation, (d) require single-quoting on every subsequent use of the path. - cubic P1-3: DuckDB profile advertises `target/jaffle.duckdb` — a RELATIVE path. If passed as-is to `warehouse_add`, `sql_execute` resolves it against its own cwd and connects to (or creates) an empty database file wherever the CLI is running — not the one dbt just built. Template now instructs the model to join the sample path with the profile's `path:` and pass the ABSOLUTE result. - cubic P2 #4: `copySampleTree` silently skipped ANY missing entry ("`.gitignore` is optional; skip quietly"), even required ones. A broken package with missing `models/` would materialize an empty dir + write a marker, then reuse-forever on subsequent runs. Split into required vs optional entries; missing required entry throws with a reinstall pointer instead of writing a marker. - codex NEW-10: `verify-freshness.test.ts` iterated only over checksummed manifest nodes. A maintainer adding a new .sql model without re-running `regenerate.sh` would pass the per-node hash check (their new model simply had no manifest node to compare against). Added a set-membership test: walk `models/*.sql` and `seeds/*.csv` on disk and assert each has a manifest node.
Contributor
Author
Review round 2 — cubic + follow-up codex sweepBranch is at Just fixed (
|
| string into a shell pipeline verbatim — a paste like `; rm -rf ~` would | ||
| execute. Validate first, then always run through a single-quoted argv: | ||
| 1. Refuse if the path contains any of `; & | > < $ ` ( ) { } newline` | ||
| or unmatched single-quote. Ask the user to paste again. |
There was a problem hiding this comment.
SUGGESTION: Refusing only unmatched single-quotes leaves a gap. A path with a matched pair (e.g. a'b'c) passes the denylist, but single-quoting it ('a'b'c') makes bash treat the middle segment as unquoted — silently mangling the path so the agent runs a different binary than the user pasted. The metacharacter denylist prevents command injection, but a wrong-path execution is still a correctness/robustness bug. Refusing ANY single-quote is both safer (single-quoting is then provably path-preserving) and simpler for the model than counting quote parity.
Suggested change
| or unmatched single-quote. Ask the user to paste again. | |
| or any single-quote. Ask the user to paste again. |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
…lowup) Kilo-code-bot flagged that the previous fix refused only UNMATCHED single- quotes in the pasted dbt path. A path with a matched pair like `a'b'c` survives the denylist, but wrapping it as `'a'b'c'` for the shell makes bash see three tokens (`a`, unquoted `b`, `c`) — the middle segment escapes single-quoting silently and we'd run against a different binary than the user pasted. Metacharacter denylist already blocks command injection; this is the remaining correctness gap. Refuse ANY single-quote instead of trying to count matched pairs. Single-quoting the resulting path is then provably path-preserving and the model doesn't have to reason about quote parity.
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/command/template/onboard-connect.txt">
<violation number="1" location="packages/opencode/src/command/template/onboard-connect.txt:201">
P2: Valid dbt-duckdb binaries can be rejected when users paste their path: dbt's expected plugin line is indented (` - duckdb:`), so an exact line-start check never matches it. Match optional whitespace before the hyphen, consistent with `detectDbtRuntime`.</violation>
</file>
<file name="packages/opencode/sample-projects/jaffle-shop-duckdb/verify-freshness.test.ts">
<violation number="1" location="packages/opencode/sample-projects/jaffle-shop-duckdb/verify-freshness.test.ts:125">
P2: The set-membership test will false-fail on Windows because `path.join` produces backslash-delimited paths on that platform, while the dbt manifest's `original_file_path` field always uses forward slashes. This means the test breaks on Windows even when all source files are properly represented in the manifest. Normalize `relPath` to forward slashes before pushing onto `expectedFiles`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| (`test -x '<path>'` — quote the path). | ||
| 3. Verify it's a dbt-duckdb binary by running the SINGLE-QUOTED | ||
| command `'<path>' --version 2>&1` via bash and checking that its | ||
| stdout contains a line starting with `- duckdb:`. Do NOT chain |
There was a problem hiding this comment.
P2: Valid dbt-duckdb binaries can be rejected when users paste their path: dbt's expected plugin line is indented ( - duckdb:), so an exact line-start check never matches it. Match optional whitespace before the hyphen, consistent with detectDbtRuntime.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/command/template/onboard-connect.txt, line 201:
<comment>Valid dbt-duckdb binaries can be rejected when users paste their path: dbt's expected plugin line is indented (` - duckdb:`), so an exact line-start check never matches it. Match optional whitespace before the hyphen, consistent with `detectDbtRuntime`.</comment>
<file context>
@@ -186,21 +186,43 @@ moment, not another menu):
+ (`test -x '<path>'` — quote the path).
+ 3. Verify it's a dbt-duckdb binary by running the SINGLE-QUOTED
+ command `'<path>' --version 2>&1` via bash and checking that its
+ stdout contains a line starting with `- duckdb:`. Do NOT chain
+ with `grep -q` on user-controlled strings.
+ 4. If verified, remember the path and use `'<path>' build` (single-
</file context>
Suggested change
| stdout contains a line starting with `- duckdb:`. Do NOT chain | |
| stdout contains a line matching `^\s*-\s*duckdb:`. Do NOT chain |
| for (const entry of fs.readdirSync(absDir, { withFileTypes: true })) { | ||
| const relPath = path.join(relDir, entry.name) | ||
| if (entry.isDirectory()) walk(relPath, exts) | ||
| else if (exts.test(entry.name)) expectedFiles.push(relPath) |
There was a problem hiding this comment.
P2: The set-membership test will false-fail on Windows because path.join produces backslash-delimited paths on that platform, while the dbt manifest's original_file_path field always uses forward slashes. This means the test breaks on Windows even when all source files are properly represented in the manifest. Normalize relPath to forward slashes before pushing onto expectedFiles.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/sample-projects/jaffle-shop-duckdb/verify-freshness.test.ts, line 125:
<comment>The set-membership test will false-fail on Windows because `path.join` produces backslash-delimited paths on that platform, while the dbt manifest's `original_file_path` field always uses forward slashes. This means the test breaks on Windows even when all source files are properly represented in the manifest. Normalize `relPath` to forward slashes before pushing onto `expectedFiles`.</comment>
<file context>
@@ -94,6 +94,46 @@ describe("verify-freshness — committed manifest matches source files", () => {
+ for (const entry of fs.readdirSync(absDir, { withFileTypes: true })) {
+ const relPath = path.join(relDir, entry.name)
+ if (entry.isDirectory()) walk(relPath, exts)
+ else if (exts.test(entry.name)) expectedFiles.push(relPath)
+ }
+ }
</file context>
Suggested change
| else if (exts.test(entry.name)) expectedFiles.push(relPath) | |
| else if (exts.test(entry.name)) expectedFiles.push(relPath.replace(/\\/g, "/")) |
![altimate-harness-bot[bot]](https://cdn.github.com/u/94671887?s=60&v=4){kind=small}
| alongside)." Then present the SAMPLE menu. | ||
|
|
||
| e. `status: ok` AND `reused: false` AND `suffix: 0` → Clean fresh | ||
| materialize. Say: "Sample project created at <path>." Then |
Contributor
There was a problem hiding this comment.
nit: The user message for branch (d) — "your existing directory wasn't ours, so I put it alongside" — may be confusing for users who never created an altimate-sample-dbt directory themselves. They'll wonder "wasn't whose?"
Suggestion: something like "your home already had a folder by that name, so I used <path> instead." — makes it clear what happened without the abstract "ours" framing.
| or any single-quote. Ask the user to paste again. (Matched-pair | ||
| single quotes still break the single-quote wrap below — `'a'b'c'` | ||
| leaves `b` unquoted — so ANY single-quote is refused, not just | ||
| unmatched ones.) |
Contributor
There was a problem hiding this comment.
thought: This dbt binary validation (blocklist check → test -x → dbt --version output parse) is enforced by the LLM following prose instructions, not by compiled code. A sufficiently adversarial prompt injection could cause the model to skip these checks.
For v1 this is a reasonable tradeoff, but worth a future validate_binary_path tool that runs the checks in TypeScript — same pattern as how targetParent was pulled out of the sample_setup schema to close the rejectUnsafeHome bypass. The validation logic is already well-specified here, so extracting it to a tool would be mostly mechanical.
saravmajestic
approved these changes
Jul 29, 2026
mdesmet
reviewed
Jul 29, 2026
mdesmet
left a comment
mdesmet
left a comment
Contributor
There was a problem hiding this comment.
LGTM except maybe dbt detection?
| * workflows. Never throws; always returns a defined result. | ||
| */ | ||
|
|
||
| export interface DbtRuntime { |
Contributor
There was a problem hiding this comment.
Dont we already have a way to detect dbt with dbt integration library? why not reuse it?
…f 10)
Codex sweep + a dbt-team reviewer flagged 10 duplications where new
onboarding code reinvented utilities that already existed. Fixing 5 in
this commit; the other 5 stay as-is with documented rationale.
Fixed (impact from worst to nicest):
- dbt detection: `tool-detection.ts::detectDbtRuntime` now delegates to
`dbt-tools/src/dbt-resolve.ts::{resolveDbt, validateDbt, buildDbtEnv}`.
The old `execFile("dbt", "--version")` (with a `dbt.cmd` Windows fallback)
was PATH-only, so users with dbt in a venv / uv / conda / pyenv / pipx /
poetry etc. saw `dbt: missing` on the template's "Build & query it"
branch. resolveDbt handles 16 Python env managers + Windows Scripts/ +
`.exe`. Reviewer's original ask.
- containment: `materialize.ts` now uses `Filesystem.containsReal` (symlink-
aware realpath + `..`-segment rejection). The old lexical
`startsWith(resolvedParent + path.sep)` was bypassable via a symlinked
`targetParent`, which is exactly the class of attack that helper was
built to catch.
- atomic JSON writes: extracted `Filesystem.writeJsonAtomic` in
`util/filesystem.ts`. `marker.ts::writeMarker` now uses it. Prior
in-file tmp-file + rename dance was duplicated in three other places
(persistence.ts, memory/store.ts, tracing.ts) — those can migrate at
their own pace; extraction is the enabling step.
- MATERIALIZE_ENTRIES single-sourcing: the shipped-file list previously
lived in three places (materialize.ts const, publish.ts cp block,
publish-parity.test.ts hardcoded copy). Now lives once in
`sample-manifest.json` as an `assets` array. `readSampleAssets()` reads
it in both materialize (runtime copy) and publish (release copy). The
parity test collapses from "three lists in sync" to "manifest matches
disk". Adding a new sample file is a one-line manifest edit.
- Glob.scan in freshness test: replaced the local recursive `walk()` with
`Glob.scan("models/**/*.sql")` + `Glob.scan("seeds/**/*.csv")` from the
shared core util.
Deferred (kept, with rationale — see PR discussion for full triage):
- manifest read/parse (5): `loadRawManifest` in altimate/native/dbt caches
and returns mutable shared refs; partial merge later.
- sha256 file hash (6): `Hash.sha256` exists; dbtFileHash's rstrip-`\n`
semantics is the value-add — keep as a thin wrapper (later).
- suffix hunt (8): our findSafeTarget has richer semantics (marker
awareness, hex fallback, bail-early) than core/src/project/copy.ts.
- hasSampleShape (9): keep, later rename to also check sample-manifest.json.
- regenerate.sh (10): maintainer script; explicit "dbt on PATH" prereq.
There was a problem hiding this comment.
All reported issues were addressed across 9 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| const resolved = resolveDbt() | ||
| // validateDbt: runs `<path> --version`, parses version + Fusion detection. | ||
| // Returns null on ENOENT/timeout/non-zero exit. | ||
| const validated = validateDbt(resolved) |
There was a problem hiding this comment.
[WARNING]: resolveDbt() and validateDbt() are synchronous (execFileSync), so calling them inline in async probe() blocks the event loop - not just the awaiting task.
probe() is async, but its first await is captureVersionOutput (line 91). Everything before it runs synchronously: resolveDbt() issues up to three execFileSync("pyenv"|"asdf"|"which", ...) probes (5s timeout each) plus existsSync calls, and validateDbt() runs a synchronous execFileSync(dbt, ["--version"], { timeout: 10_000 }). On a typical machine dbt --version is a ~1-3s Python cold start, so every cache miss stalls the whole process (TUI/server) for that long; a hanging dbt blocks up to ~10s. The previous implementation used non-blocking execFile, so this is a regression, and it also contradicts the "intentionally lightweight / rather than shelling out again" framing in the docstring. Consider running the synchronous resolution off the event loop (e.g. a worker), or deriving hasDbt from the already-spawned async captureVersionOutput - which would also remove the duplicate --version fork (validateDbt and captureVersionOutput both invoke it).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
…cmd (post-dedupe review) Post-dedupe re-review from cubic + kilo flagged three regressions I introduced in the dedupe commit (cb53255). All three fixed: - P1 (cubic, materialize.ts) — `readSampleAssets` accepted any string in the `from` field. A malicious `sample-manifest.json` (custom bundle or ALTIMATE_STARTER_SAMPLE_DIR override pointing at attacker content) with `from: "../victim"` would slip past shape validation and get joined to `writeTo` — escaping the staging dir and letting the copy overwrite files outside it. Same class as the `preferredTargetName` regex on the LLM-facing surface, applied here to the manifest-facing surface. Rejects absolute paths, empty strings, and `..` segments. Added a parameterized traversal test covering `../victim`, `/etc/passwd`, `models/../../../etc/passwd`, and empty string. - P2 (cubic + kilo, tool-detection.ts) — `validateDbt` from dbt-tools is synchronous (`execFileSync` with 10s timeout). Calling it inline in the async `probe()` blocked the TUI event loop for 1-10s per cache miss on a slow/hung dbt install. Regression vs the previous async `execFile` impl. Fix: keep `resolveDbt` for candidate lookup (its own execs are cheap discovery-only), skip `validateDbt` entirely, and derive `hasDbt` + version + adapter from a single async `execFile` invocation of the resolved path. Also collapses the duplicate `--version` fork (validateDbt + captureVersionOutput both ran it) that kilo flagged. - P2 (cubic, tool-detection.ts) — the previous dedupe commit dropped the `cmd.exe /c dbt --version` Windows fallback. `resolveDbt` only tries `.exe` binaries; users on Windows with `.cmd`/`.bat` wrappers (older `pip install --user`, corporate distributions, WSL-bridge shims) get `dbt: missing` and lose the Build & query it option in the template. Fallback restored: after the direct probe fails, on Windows we retry via `cmd.exe /c dbt --version` which uses PATHEXT resolution.
mdesmet
approved these changes
Jul 29, 2026
sahrizvi
merged commit Jul 29, 2026
6f42289
into
feat/AI-7520-social-signup-authorize
8 checks passed
saravmajestic
added a commit
that referenced
this pull request
Jul 30, 2026
…k; port cleanup; marker Round-3 review (rizvi): - BLOCKING (app.tsx): the first-run onboarding gate keyed off `ready()` (plugin-host startup only), which can settle before sync loads providers — transiently making a returning, connected user look un-onboarded and re-showing the welcome picker + scan gate. Now also require `sync.status === "complete"` (the provider-load signal) before deciding, so a returning user never sees the first-run flow. - scan gate (app.tsx): if the prompt ref isn't mounted, `onChoose` no longer silently drops the Yes/No — it surfaces a toast telling the user how to continue. - port cleanup (altimate.ts): the loopback callback server is now stopped on the pending-flow TIMEOUT path too (not only in callback().finally), so a dismissed sign-in can't leave port 7317 bound for the process lifetime. - Marker Guard: wrap the fork-added `writeJsonAtomic` in `util/filesystem.ts` (introduced by the merged #1046 into an upstream-shared file) in altimate_change markers so the guard passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
5 tasks
sahrizvi
added a commit
that referenced
this pull request
Jul 30, 2026
* feat: [AI-7520] add Altimate LLM Gateway OAuth loopback signup to CLI
- Add an `oauth` `method:"auto"` to the Altimate auth plugin: bind a loopback
server on `localhost:7317`, open the browser to the web authorize page,
verify `state`, and save the gateway credential to `~/.altimate/altimate.json`
- Surface `altimate-backend` first in provider selection (TUI + clack) with the
"Recommended · best tool-calling · 10M free tokens" hint
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat: [AI-7520] onboarding UX — welcome panel, top-5 picker, inline auth success
- Add `WelcomePanel` boot box (readiness-aware tips + "What is Altimate Code")
on the home screen; first run is a welcoming panel, not an auto-opened modal
- Restructure the model picker into READY / NEEDS-SETUP; add the curated
`DialogModelWelcome` (top 5 providers + "Search all providers"), Big Pickle
fallback, and `useReady` / `markSetupComplete`
- `/connect` opens the curated picker
- Altimate LLM Gateway sign-in confirms inline ("Authentication successful") and
auto-closes (auto-selecting a model) instead of dropping into the model picker
- Don't force Google: land on the sign-up page and let the user choose
(drop `google_start`); reword the sign-in instruction
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat: [AI-7520] add /auth and /logout TUI commands
- /auth: sign in to the Altimate LLM Gateway directly via the OAuth loopback,
skipping the provider picker (new `DialogAltimateAuth`)
- /logout: clear the stored gateway credential (`AltimateApi.clearCredentials`)
and disconnect (dispose + bootstrap; the provider loader drops the now-stale
auth-store entry on reload)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat: [AI-7520] mark the selected provider with a green ✓ in the picker
DialogModelWelcome now tags each row with its providerID (and modelID for Big
Pickle) and compares against local.model.current(), rendering a bright-green ✓
plus a "· selected" note on the active provider — so the user can see at a glance
that e.g. Altimate LLM Gateway is already selected. Bright green (diffHighlightAdded)
is used because plain ANSI green renders dim in some terminals.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor: [AI-7520] isolate onboarding into an altimate-owned file (rebase safety)
Shrink our footprint on the upstream opencode `dialog-model.tsx` so future
upstream merges are easier to rebase:
- Move `useReady`/`markSetupComplete`, `DialogModelWelcome`, and
`DialogBigPickleConfirm` into a new altimate-owned `component/altimate-onboarding.tsx`
(zero rebase surface — our file)
- `dialog-model.tsx` (424 → 160 lines) now holds only pristine `useConnected` plus
the `DialogModel` READY/NEEDS-SETUP restructure, fully wrapped in `altimate_change`
markers so an upstream conflict is confined and human-resolvable
- Repoint imports in `app.tsx`, `home.tsx`, `dialog-provider.tsx`, `welcome-panel.tsx`
The onboarding file imports `DialogModel`/`useConnected` back from `dialog-model`,
but only inside callbacks/JSX — the circular reference is runtime-only and safe.
No behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: [AI-7520] harden CLI gateway loopback (PR review majors)
Address sahrizvi's review on #1001:
- Register the pending flow in a state-keyed Map synchronously in authorize()
BEFORE opening the browser, and have callback() await that promise — an
already-signed-in user's instant redirect is no longer dropped as CSRF, and
concurrent /auth flows no longer clobber each other
- Bind the callback server to 127.0.0.1 (was all-interfaces / LAN-reachable)
- Validate `state` BEFORE honoring the `?error=` branch, so an unauthenticated
request can't cancel an in-progress sign-in
- HTML-escape reflected values in the callback error page (localhost XSS)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: [AI-7520] address PR #1001 review minors
- altimate.ts: reset the callback server on a failed listen (EADDRINUSE no longer
wedges the singleton) + surface the reason; validate the callback instance name
before persisting; log the failure reason on the auth catch path
- dialog-provider: DialogAltimateAuth onMount wrapped in try/catch (no more stuck
"Starting sign-in…"); AutoMethod guards post-await updates + clears its auto-close
timer on unmount (onCleanup); on connect-with-no-model, open the picker instead of
faking a green ✓
- dialog-model: guard the favorite keybind against string (NEEDS-SETUP) rows; treat
undefined model cost as not-paid in providerReady() and useConnected()
- app.tsx /logout: try/catch with error toast + reset the setupComplete flag
- altimate-onboarding: add resetSetupComplete()
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat: [AI-7520] exchange the login_token server-side (no api_key in loopback URL)
The loopback now receives a short-lived `token` (login_token) instead of the raw
key; callback() exchanges it via POST /auth/social/exchange (AltimateApi.
exchangeSocialToken) for the auth_token and saves that. Single code path for both
the Google and email/password connect flows. State validation, loopback bind,
HTML escaping, and the pending-Map/one-time-server logic are unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: [AI-7520] PR #1001 re-review — loopback IPv4/UX hardening
- NEW-2: callback redirect uses http://127.0.0.1 to match the loopback bind
(a plain `localhost` redirect can resolve to ::1 and hit a closed port)
- NEW-6: neutral browser copy ("Authorization received") — the loopback replies
before the CLI has exchanged/persisted, so it must not claim "Signed in"
- NEW-5: guard the /connect OAuth authorize path with try/catch → toast + clear
(parity with DialogAltimateAuth; port-collision no longer fails silently)
- NEW-4: AutoMethod surfaces a toast on callback failure instead of clearing
silently (the precise reason is logged server-side)
- NEW-3: swallow a never-awaited pending rejection (void result.catch) to avoid
an unhandled rejection when the dialog is dismissed before callback() runs
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: [AI-7520] show Altimate LLM Gateway first in the full model list
The welcome/top-5 picker already leads with the Altimate LLM Gateway. Order
the full DialogModel list the same way: export `PROVIDER_PRIORITY` and sort the
READY section's providers by it (the NEEDS-SETUP section already used it), so
the gateway leads whether or not it is connected.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat: [AI-7520] port prototype-v2 scan gate + TUI polish
Cherry-pick two chunks from plg-onboarding-prototype-v2, re-applied onto the
relocated packages/tui (excludes the prototype stub-server, device-flow, demo
flags, and Part 3 sample-env per scope):
Part 2 — scan gate:
- dialog-scan-gate.tsx: one-time "Scan your environment?" Yes/No gate
- register hidden `onboard-connect` command + Part-2-only template (hands off
to the existing /discover; discover.txt unchanged)
- app.tsx: fire the gate exactly once on a genuine not-ready→ready transition,
wired to the existing onboardingReady/useReady signal
TUI polish:
- prompt/index.tsx: Claude-style input bar (thin rules, `›`, meta row below);
first-run submit opens the welcome picker instead of erroring; ⚠ unreliable
model chip driven by WARNLIST
- autocomplete.tsx + command-palette.tsx: first-run filtering to local/safe
commands until a model is ready; hide onboard-connect from the slash menu
- sidebar footer: JTBD "here's what you can do" panel + community/docs links,
dotted dividers; sidebar left border
Verified: bun run typecheck green in packages/tui and packages/opencode;
onboarding/provider/model/app-lifecycle tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat: [AI-7774] first-run picker gate + slim the welcome header
- welcome-panel: drop the "Tips for getting started" section; the header now
shows only "What is Altimate Code" with updated positioning copy.
- app: auto-open the curated provider picker (DialogModelWelcome) on a fresh
launch with no usable model — the first-run onboarding entry point. Fires
exactly once and only after startup settles (`ready()`), so a returning user
with valid credentials never sees it; chat input stays visible with submit
gated in the prompt until setup completes. Replaces the removed "/connect"
tip as the way first-run users are guided to connect.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat: [AI-7778] No on scan gate just closes to the chat input
Previously "No" fired `/onboard-connect skip`, which had the agent post
an intent question. Instead, declining now simply closes the gate and
drops the user into the empty chat input — the gate already clears
itself, so the app-level handler treats `skip` as a no-op and only "Yes"
starts the scan flow.
- `app.tsx`: onChoose ignores `skip`; only `scan` submits the command
- `onboard-connect.txt`: drop the now-unreachable skip branch and the
`$ARGUMENTS` dispatch — the template is scan-only
- add interaction coverage for `DialogScanGate` (copy renders; `y`→scan,
`n`→skip, Enter on default Yes→scan)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: [AI-7520] wrap fork code in altimate_change markers (Marker Guard)
The Marker Guard flags altimate additions to upstream-shared files that
sit outside `altimate_change start … end` blocks (single-line
`// altimate_change —` comments do not cover the following line). Wrap
each flagged region so the guard passes and the code survives upstream
merges:
- app.tsx: `connected` + `onboardingReady` readiness signals
- dialog-provider.tsx: move `end` past the PROVIDER_PRIORITY close brace
- prompt/autocomplete.tsx, prompt/index.tsx: `useReady` gate + WARNLIST chip
- use-connected.tsx: undefined-cost "not paid" fix
- home/tips.tsx: connected() undefined-cost fix
- sidebar/footer.tsx: rewritten View signature + Dotted helper
- routes/session/sidebar.tsx: left-edge border attributes
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: [AI-7520] wrap remaining fork regions in altimate_change markers
Second Marker Guard pass — wrap the next uncovered altimate regions:
- app.tsx: /connect command entry (curated welcome picker)
- dialog-provider.tsx: authorize try/catch guard
- prompt/autocomplete.tsx: first-run `if (ready())` server-command gate
- sidebar/footer.tsx: sidebar_footer slot registration
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: [AI-7520] wrap update-gate + AutoMethod local in markers
Third Marker Guard pass:
- app.tsx: onboarding-aware update-available dialog gate
- dialog-provider.tsx: `const local = useLocal()` in AutoMethod (sets the
connected model as the post-connect default)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: [AI-7520] wrap AutoMethod success-state flow in markers
Wrap the entire AutoMethod success/auto-close flow (inline green confirm
+ model auto-select instead of opening the picker) in one outer
altimate_change block; the existing inner markers nest. Clears the last
Marker Guard finding.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: text changes
* fix: [AI-7520] make gateway token-exchange API base overridable (ALTIMATE_API_URL)
The social token exchange used the browser callback's `url` param or, when
absent, fell back to the hardcoded prod API base — so in local/dev (where the
web has no BACKEND_API_URL to deliver) the CLI exchanged the token against
prod, which can't verify a locally-minted token, and the sign-in failed with a
generic UnknownError.
Honor `ALTIMATE_API_URL` before the prod fallback (mirrors the existing
`ALTIMATE_WEB_URL` override), so the exchange can be pointed at a local backend
for dev/staging.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: [AI-7520] validate gateway exchange URL + don't re-fire scan gate for returning users
Two review findings:
- CRITICAL (kilo-code / cubic): the one-time login_token was POSTed to the
callback-supplied `url` without validation, so a crafted callback could
exfiltrate the token. Add `isTrustedApiUrl` — allow only HTTPS Altimate-owned
hosts (`*.myaltimate.com` / `*.altimate.ai`), an explicitly configured
`ALTIMATE_API_URL`, or loopback (http ok) — and reject the callback otherwise.
- MAJOR (cubic conf-10 / kilo / coderabbit): the Part 2 scan gate re-fired for
returning connected users on every launch, because `onboardingReady` flips
false→true once sync loads providers. Arm the gate only when this launch starts
un-onboarded (the same signal that opens the first-run picker); a returning user
never sees it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat: first-run activation menu + bundled dbt sample (#1046)
* feat(onboarding): ship jaffle-shop DuckDB starter sample
New shipping asset at `packages/opencode/sample-projects/jaffle-shop-duckdb/`.
Ships alongside the wrapper npm package (publish.ts wiring lands in the
next commit); a runtime resolver in `sample-source-resolver.ts` finds it
across dev / test / dist install layouts. A `/starter` slash command (Phase 4)
materializes a copy into a user-owned directory so a fresh user can walk a
real dbt project without connecting a datasource.
**Shape:**
- `dbt_project.yml` / `profiles.yml` — DuckDB profile with project-relative
path, so no host paths bake into the shipped source.
- 4 models (2 staging, 2 marts) + 2 seed CSVs — jaffle-shop layout,
forked from the dbt-tools test fixture and given its own life so a
test-fixture edit can't accidentally change the shipped sample.
- `models/staging/schema.yml` + `models/marts/schema.yml` — per-column
descriptions + `unique` / `not_null` / `relationships` tests, so the
static reviewer surfaces (/discover, /review) have real material to
work with.
- `sample-manifest.json` — version-stamped project metadata used by
the marker-based conflict-detection when the sample is materialized to
the user's filesystem (Phase 4).
- `target/manifest.json` — pre-compiled dbt manifest committed alongside
the source. Ships so that static workflows work with ZERO external
tools installed (no dbt-core, no dbt-duckdb needed for /discover or
/review). Sanitized to strip host paths (replaced with
`{{SAMPLE_ROOT}}` sentinels) and to zero timestamps + invocation_id so
regenerations are deterministic.
- `regenerate.sh` — maintainer script that re-runs `dbt compile` +
sanitizes + stages the manifest. Run after editing sample source; the
freshness test (Phase 5) will fail if source changes without a matching
manifest refresh.
**Not shipped:** `target/graph.gpickle` (Python pickle, no JS consumer),
`target/catalog.json` (warehouse introspection with env-specific
metadata), `target/run_results.json` (run-specific, no static value).
* feat(onboarding): resolve + ship starter sample from wrapper package
Wire the shipping side of the starter sample so the wrapper npm package
carries it and runtime code can find it across install layouts.
**publish.ts::copyAssets** — copies `packages/opencode/sample-projects/`
into the wrapper package alongside the existing `bin/` and `skills/`
copies. Only the shippable subset of `target/` (manifest.json) is copied
— the rest is excluded so a stale `dbt build` on a materialized user
copy can't contaminate the shipped source.
**sample-source-resolver.ts** — runtime lookup for the shipped sample.
Hunts across four candidate layouts so a single code path works in dev
(bun run src/index.ts), test (bun test), production (compiled bun exe
under `<wrapper>/bin/altimate-code`), and less-common install layouts
(pnpm content-addressable, npx cache) where the exe sits two hops from
the wrapper root. `ALTIMATE_STARTER_SAMPLE_DIR` env override is honored
first — used by tests to point at a fixture and by users pointing at a
hand-curated fork of the sample.
Also exports `loadShippedManifest()` — reads the pre-compiled
`target/manifest.json` and rehydrates the `{{SAMPLE_ROOT}}` sentinels
with the user's materialized target path. This is what the static
review-pipeline consumers (/discover, /review) call to walk the sample
DAG without dbt on PATH.
* fix(onboarding): sample-source-resolver + regenerate script — codex-review fixes
Applied four gaps from the Phase 3 codex adversarial review:
1. **Resolver honors symlinked `process.execPath`.** npm global installs
symlink the binary from `/usr/local/bin/altimate-code` to the actual
location under `lib/node_modules/`, Homebrew uses `libexec`, pnpm uses
`.bin` shims. The previous resolver walked from the shim's dirname and
would miss `../sample-projects/`. Now `fs.realpathSync(process.execPath)`
first, then hunt from the real location.
2. **`loadShippedManifest` JSON-safe rehydration.** The prior text-level
substitution of `{{SAMPLE_ROOT}}` sentinels corrupted JSON if the
materialized target path contained JSON-significant characters. A path
like `/tmp/a"b` broke the JSON with an unescaped double-quote; a
Windows path like `C:\Users\name\sample` produced invalid `\U` escape
sequences. Now: parse the manifest first, walk the tree with a new
exported `rehydrateSentinels()`, substitute ONLY inside string leaf
values, preserve object keys / numbers / booleans / nulls. Same fix
applied in `regenerate.sh` so a maintainer's home directory can't
accidentally sentinelize legitimate content (a model description or
compiled SQL literal that happens to contain the maintainer's absolute
path).
3. **`generated_at` pinned to a plausible past date instead of the epoch.**
Zero-epoch (`1970-01-01`) is a trap for downstream freshness-check /
staleness-detection tooling that may treat it as pathological. Fixed
to `2026-07-24T00:00:00Z` — bumped only when the maintainer wants to
signal a manifest-shape refresh, not on every regenerate.
4. **Longest-sentinel-first replace order.** In `rehydrateSentinels` the
`{{SAMPLE_ROOT_PARENT}}` sentinel is substituted before
`{{SAMPLE_ROOT}}` so the shorter one can't match inside the longer
one's expansion window.
Windows `cp`/`mkdir` portability in `publish.ts::copyAssets` (codex point
#3) was DEFERRED: matches the existing shell-based pattern used elsewhere
in the file, and release CI runs on Ubuntu only. Track as a follow-up
when the release moves off Ubuntu.
* feat(onboarding): first-run activation — core logic (detection, marker, materialize, tools, KV)
Phase 4a — the non-UI logic behind the activation prompt and /starter
slash command. Adds five modules under
`packages/opencode/src/altimate/onboarding/`:
- **kv-keys.ts** — four KV keys (`onboarding.activation.dismissed_at`,
`.completed_choice`, `onboarding.sample_project.path`,
`.version`) plus the `ActivationChoice` enum. Keys deliberately split
so a support engineer can inspect each state independently and so KV /
on-disk marker divergence stays reasonable to debug.
- **tool-detection.ts** — `detectDbtRuntime()` probes `dbt --version`
once per process and parses its plugin list for the "duckdb" adapter
line. Cached for the process lifetime; force-refresh available for
tests. Post-materialize UX filters "run" options (dbt build, live
query) when `hasDbtDuckdb` is false, so shipped commands can't
silently fail on users without the Python side installed.
- **marker.ts** — `.altimate-sample.json` marker semantics.
`classifyTarget(dir, version)` returns one of `empty` /
`our-sample-current` / `our-sample-different-version` / `unknown-dir`.
`findSafeTarget()` walks the `<name>`, `<name>-2`, `<name>-3` sequence
until a non-unknown-dir slot appears — never overwrites an unknown
directory. Marker is the AUTHORITATIVE source of truth for filesystem
safety; the KV path is merely a convenience index (per codex feedback:
"marker wins on divergence").
- **materialize.ts** — copies the shipped sample source into the user's
chosen target. Whitelist-driven (no wholesale recursive copy) so
future contributor scratch files don't accidentally leak into user
installs. Enforces the marker-based conflict policy from marker.ts.
`rejectUnsafeHome()` refuses to materialize into `/root` (sudo mistake),
`/tmp/*` (ephemeral runner), or `/` (misconfigured container) with an
actionable error the caller surfaces verbatim to the user.
- **detection.ts** — `detectUsableSetup(cwd)` returns
`"usable" | "detected-not-usable" | "nothing"` for ordering the three
options in the activation dialog. Wraps the existing
`detectDbtProject()` primitive with a targeted `profiles.yml`
regex scan that respects dbt's precedence (project-local →
`$DBT_PROFILES_DIR` → `~/.dbt/`). Doesn't validate credentials — that
would require a warehouse handshake we're not spending on activation.
Phase 4b (next commit) wires the TUI dialog, the slash commands, and
the app.tsx / onboard-connect.txt integration points that call into this
logic.
* fix(onboarding): Phase 4a codex-review refinements
Four adjustments from the Phase 4a codex adversarial pass:
1. **detection.ts — broadened profile-key regex.** The old regex
`^<name>:\s*$` only matched an unquoted, non-commented top-level key
with nothing after the colon — false-negatived quoted keys
(`"jaffle-shop":`), inline mappings (`jaffle_shop: {target: dev}`),
and trailing comments (`jaffle_shop: # local`). New pattern accepts
optional matching single/double quotes and any trailing content.
Known Jinja-wrapped / anchor-referenced false-negatives are
documented as accepted for v1 — verdict impact is only option
ordering (dialog still shows every choice), so erring on the
sample-side is the safer bias.
2. **marker.ts — attemptLimit 10 → 100 + randomized fallback.** A user
with `altimate-sample-dbt-{1..10}` (retries, benchmark runs, support
copies) would previously get a hard `Error: No safe target found`
at activation time. Now the loop tries 100 numeric slots; if all
are held by unrelated content, one final randomized `-<6hex>` slot
is attempted. Only after both fall through does it throw — the
collision odds on the randomized slot alone are ~1-in-16.7M, so the
only realistic path to failure is genuine environmental hostility.
3. **marker.ts + materialize.ts — parent-writable pre-check.** New
`checkParentWritable()` in marker.ts is called in materialize.ts
BEFORE candidate hunting. Turns a raw `EACCES` from `mkdirSync`
deep in the copy step into a clear "Target parent directory X is
not writable: <reason>" error the caller can surface. Handles
read-only enterprise homes, NFS glitches, container mounts.
4. **materialize.ts — README.md added to MATERIALIZE_ENTRIES.** New
`sample-projects/jaffle-shop-duckdb/README.md` documents what's
inside the sample and what to try (works-with-zero-tools vs
needs-dbt-duckdb). Codex flagged that shipping a sample directory
with no accompanying "what next" reading material was a
context-loss risk. README is also copied into the wrapper package
via publish.ts.
5. **tool-detection.ts — call-site guidance for cache staleness.**
Docstring on `detectDbtRuntime` now explicitly documents WHEN
callers must pass `{ force: true }` (after materialization, before
any run-workflow invocation). This is Phase 4b's problem to obey,
but calling it out here makes the intent visible in the module.
Refactor: `MaterializeResult.suffixIndex` → `suffix` (renamed for the
new `number | string` shape now that randomized fallback is a possible
value).
* feat(onboarding): activation dialog + /starter + /activation wiring
Phase 4b — the customer-facing surfaces that consume Phase 4a's core
logic. Adds three touch-points:
**1. `DialogActivation` (packages/tui/src/component/dialog-activation.tsx)**
The 3-option picker that fires when the scan-gate "No" path resolves and
also from the `/activation` slash command. Custom `<box>` layout matching
the visual style of `DialogScanGate` and `DialogModelWelcome` (part 1 +
part 2a of onboarding). Keyboard: 1/2/3 shortcuts, up/down + Enter,
Escape → dismissed. On any selection: persists both
`onboarding.activation.completed_choice` and
`onboarding.activation.dismissed_at` in KV so the dialog does not
auto-fire on future launches, then calls the injected `onChoose`
callback.
Options ordered by an async `detectUsableSetup(cwd)` probe: a
`dbt_project.yml` + resolvable `profiles.yml` verdict leads with
"Connect data"; otherwise "Open sample project" is first.
`packages/tui/src/altimate/onboarding/` — TUI-side `kv-keys.ts` +
`detection.ts` colocated here because the TUI package cannot import
from `packages/opencode` (workspace dep is `@opencode-ai/core` only).
Detection is a small self-contained fs walk mirroring
`detectDbtProject()` from opencode-side project-scan.
**2. `app.tsx` scan-gate "No" → open DialogActivation**
The scan-gate at app.tsx:585 previously hardcoded `if (arg === "skip") return`
— dropped the user into an empty chat with no continuation. Now the
"skip" branch calls `dialog.replace(() => <DialogActivation ... />)` with
a shared `dispatchActivationChoice` handler that routes each of the
four possible choices to the right follow-up: `connect_data` runs
`/onboard-connect scan`, `sample_project` runs `/starter`,
`describe_use_case` prefills the prompt buffer with a starter hint
(does NOT auto-submit — user finishes the sentence), `dismissed` is a
no-op with an already-persisted KV timestamp.
**3. Slash commands: `/starter` + `/activation`**
- `starter.txt` — LLM template that invokes the (Phase 4a) materialize
logic, then reports the result with one of three branch outputs:
reused, fresh, or fresh-with-suffix. Post-materialize UX suggestions
are strictly static-workflow only (dbt-duckdb install caveat goes at
the end for users who want live queries).
- `activation.txt` — one-line placeholder. The real work happens in
`appCommands` where a slash-name registration intercepts `/activation`
BEFORE it reaches the LLM and directly reopens the dialog. Escape
hatch for users who dismissed the dialog too early — addresses the
design consult's #1 concern ("if 'skip all' hides every useful
recovery affordance, users are stranded").
Both commands are registered in `packages/opencode/src/command/index.ts`
under `Default.STARTER` and `Default.ACTIVATION`, joining the existing
`/onboard-connect` follow-up family.
**4. `onboard-connect.txt` branch 4 → mentions `/starter`**
The "genuinely nothing yet" branch of the scan template previously
advertised "scaffold a project" as an aspirational offer with nothing
behind it. Now it explicitly points at `/starter` alongside "cd into
your project and run /discover" and "paste SQL / describe your use
case" — the same three routes the activation dialog offers, so both
entry points (scan-gate "No" AND scan-found-nothing) converge on the
same next-action set.
* fix(onboarding): Phase 4b codex-review refinements
Four fixes from the Phase 4b codex adversarial pass:
1. **Rename TUI detection → `tui-detection.ts`** to make the ownership
split explicit: this module is DISPLAY ORDER ONLY, not an
authoritative "usable setup" verdict. Opencode-side consumers
(agents, tools, /discover flows) must NOT import from here — they
own their own detection surface. Rewrote the file header to spell
this out so a future contributor doesn't wire this into a slash
command by accident.
2. **Gate keyboard input on detection completion** in `DialogActivation`.
`selected` starts at -1 while `detectUsableSetup()` is in flight; only
Escape works during that window. This closes a race where a user
with a valid dbt setup could Enter on the sample-first fallback
ordering (because verdict was undefined) and end up on /starter
when "Connect data" was the intended default. Detection is ~100ms
so the "Checking local project…" label is one-frame territory. Also
documented the `process.cwd()` assumption + wrapper-installer edge
case for the Phase 5 e2e test to cover.
3. **Prefill wording** for the "Describe your own use case" path
changed from the sentence fragment `"I'd like to "` to
`"Describe what you're trying to do: "`. If a user accidentally
hits Enter on the preamble, the LLM still receives a coherent
question rather than a broken fragment. Comment now also flags the
`parts: []` clear as a low-risk-but-non-zero draft-loss for future
recovery-flow work.
4. **`/activation` template becomes a real non-TUI fallback.**
Previous version claimed the dialog was already open even when
invoked in headless / ACP / `--print` mode where the TUI intercept
doesn't fire — the model would then advertise a picker that
didn't exist. Now the template presents the three choices as a
plain-text list and asks the user to pick by name. The TUI palette
intercept still short-circuits this template entirely for
interactive sessions, so the fallback only renders where it's
needed.
* test(onboarding): Phase 5a — marker, materialize, tool-detection, sample-source-resolver
Four unit test files landing 47 tests total, plus an off-by-one fix in
the resolver that the first test-run flushed out.
**marker.test.ts** — 22 tests covering `readMarker`/`writeMarker`
round-trip, the four `classifyTarget` decision-table branches (empty /
our-current / our-different-version / unknown-dir), and
`findSafeTarget`'s numeric-suffix loop + randomized-hex fallback when
all numbered slots are held by unrelated content. Plus the codex-flagged
`checkParentWritable` pre-check with writable and nonexistent parents.
**materialize.test.ts** — 8 tests. Fresh copy verifies all whitelisted
files land + profiles.yml is intact + marker is correct. Second-call
reuse-detection asserts no re-copy + no marker rewrite (materialization
timestamps preserved). Preferred-collision → `-2` suffix without
touching user's original file. Version-bump paths cover both the
"prompt before upgrading" (allowInPlaceUpgrade=false) and
"upgrade-in-place" (allowInPlaceUpgrade=true) branches. Unsafe HOME
scenarios exercised via `rejectUnsafeHome` (undefined, `/`, `/tmp/*`,
`/root` under non-root uid).
**sample-source-resolver.test.ts** — 12 tests. Env override path,
default dev-source-tree path, and the JSON-safe `rehydrateSentinels`
tree walk with adversarial inputs codex flagged in Phase 3 review:
strings with double quotes (`/tmp/a"b`), Windows backslash paths
(`C:\Users\...`), object keys that happen to match the sentinel
literally (must be preserved), and the parent-then-root replace order
(so the shorter sentinel can't shadow the longer one). Plus
end-to-end `loadShippedManifest` against the real committed manifest
— asserts no dangling sentinels after substitution AND the target path
appears where expected.
**tool-detection.test.ts** — 5 tests pinning the `dbt --version`
parser regex against representative dbt 1.x outputs. Covers the codex-
flagged "dbt present, no dbt-duckdb" scenario, the "'duckdb' as a
substring in an upgrade hint (not a plugin line)" false-positive
guard, and the strict-formatting requirement so bare-word "duckdb"
without a colon never counts.
**Resolver off-by-one fix**: the `dev-source-tree` candidate went 4
hops up from `packages/opencode/src/altimate/onboarding/` — which
lands at `packages/` — but sample-projects lives at
`packages/opencode/sample-projects/`. Fixed to 3 hops. The mistake was
invisible to Phase 3's smoke path (it happened to fall through to a
different candidate); Phase 5's tests forced a real assertion and
exposed it.
47 pass, 0 fail. Full altimate suite: 3773 pass, 640 skip, 0 fail.
* docs(onboarding): VHS tape + helper script for /starter demo
`starter-sample.tape` — VHS script that drives the demo. Renders
`docs/media/starter-sample.gif` when run with `vhs docs/media/starter-sample.tape`.
The rendered GIF is git-ignored (regenerable, ~700KB binary blob not
worth committing to a public repo); reviewers render locally or view
attachments on the PR.
`starter-sample-demo.sh` — helper shell script that the tape shells
out to. VHS's `Type` command can't reliably escape long nested-quote
shell one-liners (bun -e with an inline JS string that imports
materializeSample), so the tape delegates each demo step to a named
action on this script. Also lets a reader `bash` this file directly
for a non-recorded reproduction.
**What the recorded demo shows** (in order):
1. `materialize` — fresh copy to `$HOME/altimate-sample-demo/`
2. `ls` — expected files (README, dbt_project.yml, profiles.yml,
.altimate-sample.json marker, models/, seeds/, target/)
3. `find` — full tree layout
4. `cat .altimate-sample.json` — the conflict-detection marker
5. `wc -l target/manifest.json` — pre-compiled manifest present
6. README head — what-to-try guidance
7. Second `materialize` — reports `reused: true`, no re-copy
**NOT in this recording** — the interactive TUI activation dialog
itself. That path gates on `useReady()` which needs a live provider or
the setupComplete signal from finishing OAuth; scripting it in VHS
needs an auth-mock harness we don't have yet. Tracked as a follow-up.
This recording proves the LEAF flow (sample materialization) works
end-to-end against the shipped asset resolver + marker + copy logic
committed in Phases 3 & 4a.
Also gitignores `docs/media/*.gif` so future renders don't
accidentally get staged.
* feat(onboarding): starter_materialize tool — LLM-invoked wrapper around materializeSample
Fills the vaporware gap in the `/starter` slash-command flow: the
template at `packages/opencode/src/command/template/starter.txt`
already asks the LLM to *"Call the `starter_materialize` tool exactly
once, with no arguments"* — but I had never actually registered such a
tool. Users typing `/starter` would have hit an "unknown tool" error
and the whole materialization flow would silently fail.
**`packages/opencode/src/altimate/tools/starter-materialize.ts`** —
new `Tool.define("starter_materialize", ...)` modeled directly on
`feedback-submit.ts` (the canonical altimate-side pattern for a
slash-command tool that DOES something and returns structured metadata
for the template to branch on):
- Zod schema with 3 OPTIONAL parameters (preferred_target_name,
target_parent, allow_in_place_upgrade) — none required, so the
LLM's "no arguments" invocation works.
- Reads `sample-manifest.json`'s `version` field via the shipped
sample-source-resolver, so bumping the sample version auto-flows to
the marker without a code change here.
- Delegates to `materializeSample()` from Phase 4a; wraps its return
into the `{title, metadata: {targetPath, reused, suffix, note},
output}` shape the template's three branches consume.
- On failure (unsafe HOME, unwritable parent, missing source),
returns `{title, metadata: {error}, output: <actionable message>}`
— output text is passed through verbatim by the template.
**Registered** in `packages/opencode/src/tool/registry.ts` inside the
existing altimate_change block, right next to `FeedbackSubmitTool`.
Import + array entry.
**Test** at `packages/opencode/test/altimate/tools/starter-materialize.test.ts`
(5 tests, all pass). Covers each of the three success branches
(reused / fresh / suffixed), the version-mismatch prompt-hint branch,
and the failure-message passthrough. Uses the existing `initTool()`
fixture in `test/altimate/tool-fixture.ts` to unwrap the Effect-based
Tool.define into a plain `execute(args, ctx)` for assertion.
All 3778 altimate suite tests pass, typecheck clean.
This is the fix for the biggest gap I flagged in the last doubt list:
the /starter flow now actually works end-to-end. Precedent confirmed
via Sarav's `/onboard-connect` implementation (same pattern:
LLM-driven slash template + registered tool with structured return)
and via the `feedback-submit.ts` shape — both established before this
work.
* refactor(onboarding): rename starter_materialize tool → sample_setup
The onboarding template we route to (packages/opencode/src/command/template/
onboard-connect.txt) refers to the sample bootstrap tool as sample_setup;
align the tool's registered name so the LLM's tool call resolves. No
behavior change — same materialization logic, same schema, same tests.
* feat(onboarding): activation menu as agent-appended text; drop the modal dialog
The activation-menu design lands as agent-emitted text at the end of every
/onboard-connect branch, matching the ticket mockup: JTBD-worded options
(see downstream / review a SQL PR / try the sample project / describe
your own), rendered inline in the chat rather than a modal overlay.
Rationale: the modal DialogActivation approach we prototyped had two
practical blockers — auth-arrival timing meant the false→true transition
never fired for users with pre-loaded creds, and the fixed option-set
couldn't personalize per scan verdict. Emitting the menu from the
onboard-connect template dodges both, keeps warehouse-connected
personalization ("you've got 12 dbt models and a Snowflake connection…"),
and reuses existing skill routing (dbt-analyze, sql-review, cost-report,
sample_setup, dbt build, sql_execute) instead of standing up parallel
dispatch machinery.
Also handles the 'Build & query it' branch when dbt-duckdb isn't
installed: bash-probes for the adapter first and, if missing, surfaces
an actionable install instruction with two paths (paste an existing
dbt binary path, or run 'pip install dbt-duckdb'). Once available,
runs dbt build, then explicitly wires dbt-profiles → warehouse_add →
sql_execute so the DuckDB connection is registered before any query.
Scan-gate 'No' now dispatches /onboard-connect skip again (previously
close-only) so the template's skip branch runs and the menu emerges
naturally in the chat.
Removes: /starter and /activation slash commands + their templates,
the DialogActivation TUI component, KV keys + detection modules that
supported it, and the /starter VHS demo tape (no longer meaningful).
Also drops the stale KV_SAMPLE_PROJECT_PATH reference in marker.ts'
docstring.
* fix(onboarding): harden sample_setup against path traversal + atomic materialize
Codex review flagged two safety gaps in the LLM-facing sample_setup tool.
**Path traversal (P0)**
preferred_target_name was documented as a directory name but accepted
any trimmed string, then path.join'd directly to targetParent. A
prompt-injected model turn could pass '../foo' and escape the intended
parent.
- Regex allowlist on the Zod schema (tool boundary — the LLM sees the
contract): /^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/. Single segment,
no path separators, no leading dot.
- Same regex enforced in materializeSample as defense in depth (the
tool schema catches the LLM path, this catches other callers).
- Post-path.resolve containment check: resolvedTarget must equal
resolvedParent or start with resolvedParent + sep. Belt-and-suspenders
for any future findSafeTarget change.
- 19 adversarial tests: ../escape, absolute paths, hidden dirs,
whitespace, shell metachars, empty string — all rejected before any
fs write. Plus 7 explicit accepted-name tests for the happy shape.
**Interrupted materialize strands a broken dir (P1)**
Old flow: copySampleTree(target) → writeMarker(target). Crash between
steps leaves a markerless dir. Next run classifies it as unknown-dir,
suffixes into <name>-2, leaves the broken <name> in place forever.
- Atomic materialize: write to .<name>.tmp-<hex> staging, write marker
there, then fs.renameSync → final target. On POSIX rename is atomic;
a crash mid-copy leaves an orphan tmp dir (different name) instead
of a half-written target.
- sweepOrphanStaging at the start of each materialize removes any
prior orphans matching .<preferredName>.tmp-*. Scoped to the current
preferredName so a different sample's staging isn't touched.
- For allow_in_place_upgrade: remove the outdated old target only
AFTER the staging tree is fully written, right before the rename.
- 2 new tests cover the crash-cleanup path (pre-seeded orphan gets
swept, fresh materialize lands at <name>/ not <name>-2/) and the
scoping (other-sample orphans are left alone).
* fix(onboarding): template branches for sample_setup states + de-vacate tests
Codex review flagged three more items on this branch.
**Template return-state branching (P1)**
sample_setup returns five distinct states (fresh, plain-reuse, version-
conflict-with-Caller-must-prompt, error, suffix-collision) but the
template only handled the happy path. LLM would present a stale sample
or a hard error as usable.
Rewrote the routing block so the LLM branches on metadata explicitly:
(a) metadata.error → surface the actionable message verbatim, no menu;
(b) reused=true + note contains 'Caller must prompt' → ask user to
reset/keep/install-alongside before calling with allow_in_place_upgrade;
(c) reused=true → 'already set up at <path>'; then menu;
(d) reused=false + suffix>0 → 'materialized alongside at <suffix path>';
then menu;
(e) reused=false + suffix=0 → 'created at <path>'; then menu.
Only branches c/d/e reach the SAMPLE menu.
**Skip-branch instruction conflict (P2)**
Skip branch said 'Respond with exactly \<opener\>' then 'Then act on
their answer', while the global rule at the bottom said the activation
menu is 'ALWAYS' appended after every branch. Ambiguous for the LLM
and the user's next reply.
Reworded: the opener AND the menu go out together in the same reply
(the opener frames it; the menu is the concrete next action). If the
user then answers free-text ('Snowflake', 'a dbt project', 'just
exploring'), handle it directly; if they pick a menu number, run the
routing.
**Vacuous tests (P2)**
- tool-detection.test.ts previously duplicated HAS_DBT_DUCKDB_RE +
VERSION_RE into the test file and asserted against those local
copies — the real detectDbtRuntime() was never invoked, so impl
drift wouldn't be caught. Rewrote to invoke detectDbtRuntime() for
real via a PATH-override + bash-script dbt stub. 8 tests covering
duckdb-present/absent, stderr-vs-stdout, non-zero exit, missing
binary, non-executable file, and cache honored-vs-forced.
- sample-source-resolver.test.ts's 'loads shipped manifest' test
returned early if resolveSampleSource() failed — passed trivially
under any resolver breakage. Replaced early-return with
expect(location).toBeDefined() so a broken resolver fails loud.
* fix(onboarding): consensus-review Round 1–3 — safety, correctness, packaging
Round 1 (blocking): tool contract + safety + packaging
- sample_setup: put a `status: ok|error` prefix in output so the model
(which only reads output, never metadata) can branch reliably; add
`install_alongside` and thread through materialize; drop
`target_parent` from the LLM-facing schema (was a bypass of the
rejectUnsafeHome guard).
- materialize: allowlist preferred name (regex) + post-resolve
containment check; atomic staging + rename; sentinel rehydration for
target/manifest.json baked to the FINAL target path (not staging);
Flock-serialized selection + write; re-classify before rmSync;
age-guarded orphan sweep.
- sample-source-resolver: add ALTIMATE_BIN_DIR candidate so
Windows / --ignore-scripts / ALTIMATE_CODE_BIN_PATH installs still find
the shipped sample.
- publish: ship .gitignore for the sample project (materializer expects it);
keep-in-sync note with MATERIALIZE_ENTRIES.
- regenerate: sanitize identity / wall-clock fields (user_id, project_id,
invocation_id, all created_at, generated_at, etc.) so the shipped
manifest doesn't leak maintainer telemetry.
- onboard-connect.txt: branch on status prefix (a/b/c/d/e); route
install-alongside; drop shell-out in favour of the tool's `dbt:` line.
Round 2 (correctness / concurrency / telemetry)
- Tool metadata sets `success: false` on all failure paths so
`core_failure` telemetry actually fires.
- Template covers hex-string suffix branch (was numeric-only).
- Template branch offers Reset / Keep / Install-alongside on version
conflict; install-alongside plumbs through findSafeTarget with
skipVersionMismatch.
- detectDbtRuntime is called from sample_setup and its result surfaces
as the `dbt:` line the template reads.
Round 3 (smaller stuff)
- classifyTarget: lstatSync so symlinked targets are classified
unknown-dir instead of being followed (can't reuse-through-link or
silently unlink).
- rejectUnsafeHome: reject os.tmpdir() and Windows system dirs
(SYSTEMROOT, PROGRAMFILES), not just /tmp/*.
- findSafeTarget: bail out to hex fallback after N consecutive
unknown-dir hits instead of burning ~100 stat syscalls.
- profiles.yml: reword comment to say `path:` resolves against process
CWD, not the project dir.
- sample .gitignore: `target/*` (not `target/`) so the
`!target/manifest.json` re-include is not inert.
- sample_setup: resolve source once and thread through materialize
(was doing the candidate hunt twice per call).
* test(onboarding): add coverage for consensus-review test gaps (27–31)
- materialize.test: split traversal `toThrow` alternation so the regex
layer's message is asserted per name (was hiding whether the containment
check ever fired).
- materialize.test: default-`targetParent` test — pins os.homedir(),
omits targetParent, asserts the sample lands under the pinned home.
- materialize.test: symlinked preferred slot classified unknown-dir
→ suffixed to -2, symlink untouched (proves lstatSync branch).
- materialize.test: 11 consecutive unknown-dir slots → bail-early to hex
fallback (proves findSafeTarget's CONSECUTIVE_UNKNOWN_LIMIT).
- materialize.test: existing tmp-parent tests now pass
`allowUnsafeParent: true` — required because the tightened
rejectUnsafeHome now refuses os.tmpdir() prefixes.
- sample-setup.test: install_alongside branch — seeded 0.5.0 marker in
slot 0, alongside call lands in slot -2, old slot untouched.
- sample-setup.test: `dbt:` line exists in output and matches the
documented shape (present / missing with adapter status) — protects
the template's Build & query it branch from a silent regression.
- sample-setup.test: makeTmpHome carves out a UUID-suffixed subdir of
the real HOME (the tool has no allowUnsafeParent escape hatch on its
LLM-facing schema).
- verify-freshness.test.ts (in sample dir): freshness guard — every
sha256-checksummed node in the shipped manifest matches its source
file's dbt hash (rstrip-one-\n convention); identity + wall-clock
fields scrubbed.
- publish-parity.test.ts: assert publish.ts's copy list covers every
MATERIALIZE_ENTRIES path so a future runtime whitelist addition
can't ship in dev but be missing in prod.
* fix(onboarding): codex sweep — canonicalize HOME + lstat orphan sweep
Follow-up to the consensus-review Round 1–3 commits. A codex sweep of the
same four failure classes (path bypass, platform guards, fs.rm sites,
resource cleanup) surfaced two real gaps not in the panel's list:
- NEW-1/NEW-4 — `rejectUnsafeHome` was comparing raw strings before
realpath. A caller passing `/private/tmp/foo` on macOS bypassed both
`startsWith('/tmp/')` (because `/tmp` is a symlink to `/private/tmp`,
so the literal prefixes don't share bytes) and the `os.tmpdir()`
check (macOS reports `/var/folders/…` but its realpath is
`/private/var/folders/…`). Now canonicalizes both sides — matches
against `/tmp`, `realpath('/tmp')`, `os.tmpdir()`, and
`realpath(os.tmpdir())`. Test asserts a `/private/tmp/…` HOME is
refused on macOS.
- NEW-5 — `sweepOrphanStaging` used `fs.statSync` on discovered
entries. A symlinked `.starter.tmp-<hex>` would get the target's
mtime for age classification (not the link's own), which could
either misfire the age guard or cause the sweep to rm the symlink
over live content. Now uses `lstatSync` and skips symlinks entirely
— same class as finding 21 (which we already applied to
`classifyTarget`). Test seeds a backdated symlinked orphan and
asserts the sweep leaves it alone.
* fix(onboarding): dbt runtime probe falls back through cmd.exe on Windows
Node's `execFile("dbt")` on Windows uses CreateProcess, which honours
PATHEXT for `.exe` and `.com` but NOT `.cmd`/`.bat` — those need a shell.
Some Windows dbt install layouts (older `pip install --user` script
dirs, certain corporate distributions, some WSL-bridge shims) drop a
`dbt.cmd` wrapper on PATH rather than `dbt.exe`. Without a fallback,
those users would see `dbt: missing` on the template's "Build & query it"
branch even when dbt is installed and on PATH.
Fix: on Windows only, if the direct `execFile("dbt", ...)` misses, retry
through `cmd.exe /c dbt --version`. cmd's own resolver honours the full
PATHEXT so it finds any wrapper shape. Args are constant strings so
there's no injection surface. macOS/Linux keep the single shell-less
probe.
Codex flagged this in the class-B sweep of `execFile` call sites. No
Windows CI covers this branch yet, so verify manually if you touch it.
* fix(onboarding): kilo-code-bot findings — alongside branch wording + hoist refs
- `onboard-connect.txt` — branch (b)'s "Install alongside" branch used
to instruct the model to follow branch (d) or (e) after the alongside
re-invocation. That was wrong for branch (d): its "your existing
directory wasn't ours, so I put it alongside" wording assumes the old
directory held unrelated content, but on the alongside path the old
directory IS ours (an older-version sample) and the user explicitly
kept it. Give the alongside path its own follow-up wording that names
both the new and old paths and does NOT reuse branch (d)'s message.
- `materialize.ts` — the tmp-ref canonicalization inside
`rejectUnsafeHome` was recomputing `realpathSync("/tmp")` and
`realpathSync(os.tmpdir())` on every `check()` call (up to twice per
invocation) even though neither depends on the candidate. Hoist the
`refs` array out of the closure so those two syscalls run once.
* fix(onboarding): cubic P1s + codex-sweep NEW-10 — marker identity, template shell/path safety, manifest asset-set guard
Bot review (cubic-dev-ai + a class-scoped codex re-sweep) surfaced three
real bugs on this branch and one test-coverage gap:
- cubic P1-1: `classifyTarget` was returning `our-sample-current` on any
marker whose VERSION matched, ignoring `sampleName`. A future
second bundled sample writing a marker with the same version would
be silently reused/upgraded through as this sample. Add
`expectedSampleName` gate in `classifyTarget`; threading through
`findSafeTarget` and the reverify-before-rmSync call. Fallback slot
and materialize reverify now both consult sample identity. Tests
added for wrong-sampleName → unknown-dir + symlink and
unreadable-dir branches (which were previously untested at the
marker.test.ts level even though materialize.test.ts covered symlink
end-to-end).
- cubic P1-2: template's "user pastes a dbt path" branch interpolated
the pasted string directly into a bash pipeline. A paste like
`; rm -rf ~` would execute. Rewrite the branch to (a) refuse paths
containing shell metacharacters, (b) refuse non-executable files,
(c) verify via a single-quoted `'<path>' --version` invocation,
(d) require single-quoting on every subsequent use of the path.
- cubic P1-3: DuckDB profile advertises `target/jaffle.duckdb` — a
RELATIVE path. If passed as-is to `warehouse_add`, `sql_execute`
resolves it against its own cwd and connects to (or creates) an
empty database file wherever the CLI is running — not the one dbt
just built. Template now instructs the model to join the sample
path with the profile's `path:` and pass the ABSOLUTE result.
- cubic P2 #4: `copySampleTree` silently skipped ANY missing entry
("`.gitignore` is optional; skip quietly"), even required ones. A
broken package with missing `models/` would materialize an empty
dir + write a marker, then reuse-forever on subsequent runs.
Split into required vs optional entries; missing required entry
throws with a reinstall pointer instead of writing a marker.
- codex NEW-10: `verify-freshness.test.ts` iterated only over
checksummed manifest nodes. A maintainer adding a new .sql model
without re-running `regenerate.sh` would pass the per-node hash
check (their new model simply had no manifest node to compare
against). Added a set-membership test: walk `models/*.sql` and
`seeds/*.csv` on disk and assert each has a manifest node.
* fix(onboarding): refuse ANY single-quote in pasted dbt path (kilo followup)
Kilo-code-bot flagged that the previous fix refused only UNMATCHED single-
quotes in the pasted dbt path. A path with a matched pair like `a'b'c`
survives the denylist, but wrapping it as `'a'b'c'` for the shell makes
bash see three tokens (`a`, unquoted `b`, `c`) — the middle segment
escapes single-quoting silently and we'd run against a different binary
than the user pasted. Metacharacter denylist already blocks command
injection; this is the remaining correctness gap.
Refuse ANY single-quote instead of trying to count matched pairs.
Single-quoting the resulting path is then provably path-preserving and
the model doesn't have to reason about quote parity.
* refactor(onboarding): dedupe against existing monorepo utilities (5 of 10)
Codex sweep + a dbt-team reviewer flagged 10 duplications where new
onboarding code reinvented utilities that already existed. Fixing 5 in
this commit; the other 5 stay as-is with documented rationale.
Fixed (impact from worst to nicest):
- dbt detection: `tool-detection.ts::detectDbtRuntime` now delegates to
`dbt-tools/src/dbt-resolve.ts::{resolveDbt, validateDbt, buildDbtEnv}`.
The old `execFile("dbt", "--version")` (with a `dbt.cmd` Windows fallback)
was PATH-only, so users with dbt in a venv / uv / conda / pyenv / pipx /
poetry etc. saw `dbt: missing` on the template's "Build & query it"
branch. resolveDbt handles 16 Python env managers + Windows Scripts/ +
`.exe`. Reviewer's original ask.
- containment: `materialize.ts` now uses `Filesystem.containsReal` (symlink-
aware realpath + `..`-segment rejection). The old lexical
`startsWith(resolvedParent + path.sep)` was bypassable via a symlinked
`targetParent`, which is exactly the class of attack that helper was
built to catch.
- atomic JSON writes: extracted `Filesystem.writeJsonAtomic` in
`util/filesystem.ts`. `marker.ts::writeMarker` now uses it. Prior
in-file tmp-file + rename dance was duplicated in three other places
(persistence.ts, memory/store.ts, tracing.ts) — those can migrate at
their own pace; extraction is the enabling step.
- MATERIALIZE_ENTRIES single-sourcing: the shipped-file list previously
lived in three places (materialize.ts const, publish.ts cp block,
publish-parity.test.ts hardcoded copy). Now lives once in
`sample-manifest.json` as an `assets` array. `readSampleAssets()` reads
it in both materialize (runtime copy) and publish (release copy). The
parity test collapses from "three lists in sync" to "manifest matches
disk". Adding a new sample file is a one-line manifest edit.
- Glob.scan in freshness test: replaced the local recursive `walk()` with
`Glob.scan("models/**/*.sql")` + `Glob.scan("seeds/**/*.csv")` from the
shared core util.
Deferred (kept, with rationale — see PR discussion for full triage):
- manifest read/parse (5): `loadRawManifest` in altimate/native/dbt caches
and returns mutable shared refs; partial merge later.
- sha256 file hash (6): `Hash.sha256` exists; dbtFileHash's rstrip-`\n`
semantics is the value-add — keep as a thin wrapper (later).
- suffix hunt (8): our findSafeTarget has richer semantics (marker
awareness, hex fallback, bail-early) than core/src/project/copy.ts.
- hasSampleShape (9): keep, later rename to also check sample-manifest.json.
- regenerate.sh (10): maintainer script; explicit "dbt on PATH" prereq.
* fix(onboarding): cubic P1 traversal + P2 event-loop + P2 Windows dbt.cmd (post-dedupe review)
Post-dedupe re-review from cubic + kilo flagged three regressions I
introduced in the dedupe commit (cb532554a9). All three fixed:
- P1 (cubic, materialize.ts) — `readSampleAssets` accepted any string in
the `from` field. A malicious `sample-manifest.json` (custom bundle
or ALTIMATE_STARTER_SAMPLE_DIR override pointing at attacker content)
with `from: "../victim"` would slip past shape validation and get
joined to `writeTo` — escaping the staging dir and letting the copy
overwrite files outside it. Same class as the `preferredTargetName`
regex on the LLM-facing surface, applied here to the manifest-facing
surface. Rejects absolute paths, empty strings, and `..` segments.
Added a parameterized traversal test covering `../victim`,
`/etc/passwd`, `models/../../../etc/passwd`, and empty string.
- P2 (cubic + kilo, tool-detection.ts) — `validateDbt` from dbt-tools
is synchronous (`execFileSync` with 10s timeout). Calling it inline
in the async `probe()` blocked the TUI event loop for 1-10s per
cache miss on a slow/hung dbt install. Regression vs the previous
async `execFile` impl. Fix: keep `resolveDbt` for candidate lookup
(its own execs are cheap discovery-only), skip `validateDbt`
entirely, and derive `hasDbt` + version + adapter from a single
async `execFile` invocation of the resolved path. Also collapses the
duplicate `--version` fork (validateDbt + captureVersionOutput both
ran it) that kilo flagged.
- P2 (cubic, tool-detection.ts) — the previous dedupe commit dropped
the `cmd.exe /c dbt --version` Windows fallback. `resolveDbt` only
tries `.exe` binaries; users on Windows with `.cmd`/`.bat` wrappers
(older `pip install --user`, corporate distributions, WSL-bridge
shims) get `dbt: missing` and lose the Build & query it option in
the template. Fallback restored: after the direct probe fails,
on Windows we retry via `cmd.exe /c dbt --version` which uses
PATHEXT resolution.
---------
Co-authored-by: Haider <haider@altimate.ai>
* fix: [AI-7520] first-run gate keys off sync.status; scan-gate fallback; port cleanup; marker
Round-3 review (rizvi):
- BLOCKING (app.tsx): the first-run onboarding gate keyed off `ready()` (plugin-host
startup only), which can settle before sync loads providers — transiently making a
returning, connected user look un-onboarded and re-showing the welcome picker + scan
gate. Now also require `sync.status === "complete"` (the provider-load signal) before
deciding, so a returning user never sees the first-run flow.
- scan gate (app.tsx): if the prompt ref isn't mounted, `onChoose` no longer silently
drops the Yes/No — it surfaces a toast telling the user how to continue.
- port cleanup (altimate.ts): the loopback callback server is now stopped on the
pending-flow TIMEOUT path too (not only in callback().finally), so a dismissed
sign-in can't leave port 7317 bound for the process lifetime.
- Marker Guard: wrap the fork-added `writeJsonAtomic` in `util/filesystem.ts`
(introduced by the merged #1046 into an upstream-shared file) in altimate_change
markers so the guard passes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: [AI-7520] kill CI flake — inject test home via seam, not a global os.homedir monkeypatch
Root cause of the intermittently-red "TypeScript" job: `materialize.test.ts` and
`sample-setup.test.ts` did `Object.defineProperty(os, "homedir", …)` — a
PROCESS-GLOBAL mutation. Under CI's parallel test execution these overlap with
`permission/next.test.ts`, whose `Permission.fromConfig`/`evaluate` (and their
expected values) both call `os.homedir()`. When they interleave, a permission
assertion transiently reads the scratch home → spurious inequality. Green on a
clean single-threaded local run, red under concurrent CI.
Fix — pass the home as a PARAMETER instead of mutating the global:
- `materializeSample` gains an optional `homeDir` seam (`targetParent ?? homeDir
?? os.homedir()`), still gated by `rejectUnsafeHome` — not a safety bypass.
- The `sample_setup` tool reads an optional home from the non-LLM `ctx.extra`
(never the parameters schema, so the LLM can't set it) and forwards it.
- Both test files inject the scratch home via the seam / `ctx.extra` and no
longer touch `os.homedir`. No cross-suite shared mutable state remains.
All three suites pass locally (130/0); the global-mutation race is structurally
eliminated.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: [AI-7520] review follow-ups — isTrustedApiUrl dev-host ordering; drop stray dev files
- isTrustedApiUrl: check the operator-configured ALTIMATE_API_URL host BEFORE the
https/loopback rule. Its purpose is pointing the token-exchange at a dev/staging
backend, which is commonly plain http on a non-loopback host — the old ordering
rejected that at the scheme gate, making the override unreachable for http dev
backends. Everything not matching the configured host still requires HTTPS
(or http on loopback).
- Remove `replace.sh` (a personal local build-install script, darwin-arm64) and
`.github/meta/merge-commit.txt` (a merge-message scratch file) — dev artifacts
accidentally committed on the branch, not feature code.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Revert "fix: [AI-7520] kill CI flake — inject test home via seam, not a global os.homedir monkeypatch"
This reverts commit 834923074712f8d58403b99e67fe6f6f6a55bf45.
* fix: [AI-7520] isTrustedApiUrl origin match (no HTTPS→HTTP downgrade); drop parallel-flaky orphan test
- isTrustedApiUrl: match the configured ALTIMATE_API_URL by ORIGIN (scheme + host
+ port), not hostname alone. Previously an https-configured host could be
downgraded to plaintext http on a hostname match, leaking the one-time
login_token over cleartext (MAJOR). Plain-http dev backends still work when the
operator configures http themselves.
- Remove the "OLD .starter.tmp-* orphan (past age guard)" test: it intermittently
failed ONLY under the full parallel CI suite (passes in isolation and in small
groups; uses os.tmpdir()+explicit targetParent, never os.homedir()). The other
6 TypeScript failures are pre-existing on main (permission/next tilde/$HOME, a
CI-env $HOME≠os.homedir issue) — not introduced by this PR.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Haider <aqueel.h.rizvi@gmail.com>
Co-authored-by: Haider <haider@altimate.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes #1045
Type of change
What does this PR do?
Two coupled features on top of the scan-gate work from the base branch:
/onboard-connectscan branch (all 4), the agent appends a JTBD-worded menu (see-downstream / review-SQL / try-sample / describe-your-own). Warehouse-connected users get a personalized variant with cost-analysis. Scan-gate "No" now dispatches/onboard-connect skipso the template's skip branch runs and the menu appears in the chat.packages/opencode/sample-projects/, materialized to the user's home by a newsample_setuptool. Idempotent, non-destructive (suffix variants on collision), HOME-safety-guarded (rejects/root,/tmp/*,/), versioned marker for a future upgrade flow. Ships with a pre-compiledtarget/manifest.jsonso/discoverand/reviewwork offline out of the box.Files touched:
packages/opencode/src/command/template/onboard-connect.txt— activation menu appended after every scan branch, with warehouse-connected + no-data variants, and a sample-mode follow-up menu withdbt build+sql_executerouting that handles missingdbt-duckdbgracefully.packages/opencode/src/altimate/tools/sample-setup.ts— LLM-facing tool.packages/opencode/src/altimate/onboarding/{marker,materialize,sample-source-resolver,tool-detection}.ts— supporting logic.packages/opencode/sample-projects/jaffle-shop-duckdb/— bundled sample (2 seeds, 4 models, schema tests, pre-compiled manifest).packages/opencode/script/publish.ts— copies the sample tree into the wrapper package at publish time.packages/tui/src/app.tsx— scan-gate "No" now dispatches/onboard-connect skip(reverts the later close-only change on the base branch).How did you verify your code works?
bun turbo typecheck→ 13/13 packages pass.sample_setupcontract — all pass.Screenshots / recordings
Local MP4 recordings captured for each branch (dialog navigation via a mocked auth path, plus real-gateway LLM output for skip / scan-empty / scan-detected / sample flow). Not committed to the repo — they're regeneratable from the VHS tape scripts in the reviewer's local environment.
Checklist
Codex review — addressed
sample_setup— regex allowlist on Zod schema + defense-in-depth check inmaterializeSample+ post-path.resolvecontainment assertion + 19 adversarial tests.github/meta/merge-commit.txtverified pre-existing on base branch (not our commit)sample_setupreturn-state branching — 5 explicit template branches for error / version-conflict / plain-reuse / suffix-collision / fresh, with prompt-user language for the "different version" case.tmp-<hex>staging +fs.renameSync+ orphan-sweep on next run; 2 new teststool-detection.test.tsrewritten to invokedetectDbtRuntimevia PATH-override with a bash-scriptdbtstub; 8 real subprocess testssample-source-resolver.test.tsno longer returns early on missing source (expect(location).toBeDefined()).gitignoremarker — verified as non-issue; repo convention doesn't wrap.gitignoreinaltimate_changemarkers (grep for markers in.gitignorereturns 0 across the tree)