Post-release cleanup: deferred items from v0.9.4 multi-persona review

[sahrizvi]

Consolidated tracking issue for items surfaced by the v0.9.4 pre-release multi-persona review (CTO, PM, Data Engineer, Tech Lead, Support Engineer) and deliberately deferred out of the release because each requires more than a 30-min patch (new design work, performance experiment, cross-platform testing, or org-level infra).

All items were re-verified against the release/v0.9.4 tree; the review's original list of 9 was reduced to 8 after empirical checks (packages/tui typecheck runs green — that item was a false positive).

---

D1 — Interactive activation-menu picker (arrow-key selectable rows in the chat)

What: The activation menu that ends every /onboard-connect branch is currently plain LLM-emitted text — the user types the number instead of arrow-selecting a row.

Why deferred: Design decision, not a 30-min fix. The plain-text design was chosen in commit e89955fe3e because (a) an earlier DialogActivation modal never rendered for users with pre-loaded creds (auth-arrival timing meant the false → true transition never fired), and (b) a fixed option-set can't personalize per scan verdict, whereas the LLM branches on warehouse/dbt discovery. Reverting to a picker needs to solve both problems — either intercept LLM-emitted numbered messages in the TUI (fragile heuristic) or introduce a structured tool-call schema and route selection back through the LLM.

Attribution: Design decision — @haider (e89955fe3e).

---

D2 — Retire the globalThis bandaid on the Question pending-registry

What: packages/opencode/src/question/index.ts:165 anchors the pendingByDir registry on globalThis["__altimateQuestionPending"] so every duplicated module copy shares one Map. Root cause is dual module-graph loading in the runtime (compiled bundle vs. dev-mode source imports), not the registry itself.

Why deferred: Correct fix is fixing the module-graph duplication so there is only one question module. That means auditing every import path in packages/opencode, packages/tui, and the plugin surface for both dev and compiled builds — bigger than a release patch.

Attribution: @saravanan-altimate (8c9a9a893b7, 2026-07-24).

---

D3 — Server-side shell-arg validation for user-pasted dbt paths (move defense out of the template)

What: packages/opencode/src/command/template/onboard-connect.txt:197-213 instructs the LLM to refuse paths containing ; & | > < $ \ ( ) { } newlineor single-quotes, verify executability withtest -x`, and always wrap in single quotes. That defense-in-depth is real but template-level — a model that ignores the instructions could still shell-inject.

Why deferred: Requires designing a validation layer at the tool boundary (probably a new dbt_binary_validate server tool or a bash-wrapper allowlist) and rewriting the template to route through it. Not mechanical.

Attribution: @saravanan-altimate (4f12e7d08ef, PR #1001 landing).

---

D4 — Slim the bundled target/manifest.json (gzip + inflate on materialize)

What: packages/opencode/sample-projects/jaffle-shop-duckdb/target/manifest.json ships at 684KB uncompressed. Every npm install -g altimate-code downloads it; every sample_setup copies it byte-for-byte (with {{SAMPLE_ROOT}} sentinel rehydration).

Why deferred: Needs (a) build-step change to gzip the shipped copy, (b) inflate step in copySampleTree, (c) size measurement + package-size regression guard. Cheap on wall-clock, but touches packaging.

Attribution: @saravanan-altimate (4f12e7d08e, PR #1001).

---

D5 — Windows long-path handling in copySampleTree

What: packages/opencode/src/altimate/onboarding/materialize.ts:471 uses stock fs.cpSync / fs.copyFileSync / fs.mkdirSync. On Windows, paths longer than 260 chars need the \\?\ prefix unless the process opts in to long-path support. A user materializing into a deeply-nested directory would hit ENAMETOOLONG.

Why deferred: Fix is a Windows CI matrix + long-path guard. Bun's fs may or may not paper over this; needs empirical test on a Windows runner before choosing a solution.

Attribution: @saravanan-altimate (4f12e7d08ef, PR #1001).

---

D6 — Dedup question events at the /event boundary (or formalize the dual-emit contract)

What: packages/opencode/src/question/index.ts:100-130 deliberately mirrors question.asked / question.replied / question.rejected via Bus.publish in addition to EventV2 — the webview subscribes to the wildcard PubSub feed and otherwise never sees the events. The EventV2Bridge.listen forwarder ALSO republishes to GlobalBus, so /global/event consumers see each event twice (different top-level ids). Verified in-repo consumers (TUI sync.tsx, notifications.ts, trace consumer) all dedupe or ignore — external subscribers to /event that don't dedupe would double-count.

Why deferred: Two viable fixes: (a) dedupe in the /event handler itself (server-side, safe for external subscribers), or (b) drop one of the two publish paths and migrate every internal consumer. Both require SDK contract review — this is public API.

Attribution: @saravanan-altimate (8c9a9a893b7, 2026-07-24).

---

D8 — Sitewide internal-tracker-ref scrubber + pre-push git hook

What: altimate-code is a public repo; the org rule is that internal-tracker references (project prefix + numeric id, and the internal Atlassian host) must never land in branch names, commits, code, or docs on this repo. Enforcement today is manual (author discipline + release-time grep). No .githooks/pre-push or CI step blocks a leak automatically.

Why deferred: Needs a script that scans staged content + branch name + commit message trailer, a .githooks entry that runs it, and a repo-level git config core.hooksPath .githooks bootstrap (documented in CONTRIBUTING.md for new clones). Also needs a CI mirror (contributors who skip local hooks) — probably a lightweight GitHub Action on push. Not urgent, but the class of bug is real (already almost leaked twice this quarter).

Attribution: Org infra gap — no single author.

---

D9 — useConnected() regression detector: check against user's prior configured state, not just current

What: packages/tui/src/component/use-connected.tsx returns whether any registered provider counts as connected — pure point-in-time check. The v0.9.4 grandfather fix (in-flight on release/v0.9.4) trusts non-OpenCode registrations without cost metadata to avoid force-feeding the welcome picker to legitimate BYOK / self-hosted / custom-registration users. That's the right call for upgrades from v0.9.3, but it doesn't defend against future regressions the same way — the check has no memory of what the user had connected yesterday.

Why deferred: Proper detector needs persisted "last-known-connected" state (probably stashed in the auth manifest) and a comparison at bootstrap. Small feature, but crosses TUI, auth, and manifest storage — bigger than the release-window fix.

Attribution: Original file @anandgupta42 (c985718c4df, 2026-06-23); current grandfather fix @haider (release/v0.9.4, uncommitted at time of writing).

---

Summary

ID	Item	Attribution
D1	Interactive activation-menu picker	Haider (e89955fe3e design decision)
D2	Retire globalThis Question bandaid	Saravanan (8c9a9a893b7)
D3	Server-side shell-arg validation for dbt paths	Saravanan (4f12e7d08ef, PR #1001)
D4	Slim bundled target/manifest.json	Saravanan (4f12e7d08e, PR #1001)
D5	Windows long-path in copySampleTree	Saravanan (4f12e7d08ef, PR #1001)
D6	Dedup question events at /event boundary	Saravanan (8c9a9a893b7)
D8	Sitewide internal-tracker-ref scrubber + pre-push hook	Org infra gap
D9	useConnected() regression detector vs. prior state	Original @anandgupta42; current fix @haider

Milestone: v0.9.4
Label: post-release-cleanup

[sahrizvi]

Additional deferrals from PR #1053 review rounds

Four new follow-ups surfaced during the bot review (CodeRabbit + cubic) and the 6-reviewer consensus review of PR #1053. All are legitimate but bigger-than-release-window; adding them here so we don't lose track.

D10 — Build-manifest fingerprint for the smoke-test staleness guard

What: PR #1053 landed a two-tier staleness guard for test/install/smoke-test-binary.test.ts (initially against script/build.ts, later widened in 4caaee08d8 to walk src/ + script/). Neither approach covers everything the binary actually embeds — CHANGELOG, migrations, .opencode/skills/SKILL.md, src/provider/models-snapshot.ts, parser/worker output, and the pinned @altimateai/altimate-core prebuild are still outside the guard's window.

Why deferred: Proper fix is a build-time stamp file that records a hash of the actual embedded inputs, and the guard compares against that hash. Small feature but touches script/build.ts, package publishing, and the smoke test — bigger than a release patch.

Attribution: Reviewer @coderabbitai on PR #1053 (with the specific "changes to CHANGELOG/migrations/skills won't trigger the guard" analysis).

---

D11 — Non-idempotent retry semantics in cli-process.run()

What: The spawn retry-on-database is locked in packages/opencode/test/lib/cli-process.ts retries the entire opencode run invocation. That command writes session state + tracing on the first attempt before it could hit the lock, so a retry can double-write. Today the narrow error-string match (/database is locked/i) limits blast radius — only failures whose stderr contains that exact string retry, which in practice fires only before session-state write completes. But that's a fragile promise; a legitimate refactor could move the SQLite open earlier and turn the retry into a silent duplication.

Why deferred: Proper fix is either (a) gate the retry to command-only invocations that don't write session state, or (b) reset the per-test fixture (makeTempDirectoryScoped) before the second attempt. Neither is a 30-min patch — both need the test harness's withCliFixture reshape.

Attribution: Reviewer @coderabbitai on PR #1053.

---

D12 — Deterministic replacement for the deleted phase-label TUI e2e

What: PR #1053 deleted packages/opencode/test/cli/tui/phase-label.tui-e2e.test.ts — the file was test.skip when CI=true (so CI coverage was already zero), but it was the only local runtime check on the publishPhase → Bus.publish → sync.tsx handler → store update → render chain. The flake was diagnosed (span duration vs 50ms PTY poll interval) rather than fixed.

Why deferred: Right replacement is a synthetic-event unit test that injects a long-lived session.phase event into a mounted TUI and asserts the rendered text, without the PTY timing dependency. That's a real test-harness build-out — not a delete-and-replace.

Attribution: Consensus reviewers on PR #1053 (Claude, GPT 5.4 Codex, MiMo — reduced from MAJOR to MINOR in convergence given no CI coverage delta).

---

D13 — SSH port-forwarding / device-code flow for headless LLM Gateway sign-in

What: PR #1053's A3+A4 improved the loopback OAuth flow (port walk 7317-7325, 15-min window) but users on SSH / tmux / VS Code Remote-SSH sessions still can't complete sign-in unless they've separately forwarded the bound port from the remote workstation to their local browser. The dialog renders the URL and offers copy-to-clipboard, but the URL points at the REMOTE loopback which the local browser can't reach.

Why deferred: Right long-term answer is a device-code flow (or a non-loopback intermediate) that doesn't require the browser and the CLI to share a network namespace. That's a gateway-protocol change, not a CLI patch.

Attribution: Reviewer @cubic-dev-ai on PR #1053 (P2, confidence 8).

---

Updated summary table

ID	Item	Origin	Attribution
D1	Interactive activation-menu picker	v0.9.4 persona	Haider (design decision)
D2	Retire globalThis Question bandaid	v0.9.4 persona	Saravanan
D3	Server-side shell-arg validation for dbt paths	v0.9.4 persona	Saravanan
D4	Slim bundled target/manifest.json	v0.9.4 persona	Saravanan
D5	Windows long-path in copySampleTree	v0.9.4 persona	Saravanan
D6	Dedup question events at /event boundary	v0.9.4 persona	Saravanan
D8	Sitewide internal-tracker-ref scrubber + pre-push hook	v0.9.4 persona	Org infra gap
D9	useConnected() regression detector vs. prior state	v0.9.4 persona	Original @anandgupta42; current fix @haider
D10	Build-manifest fingerprint for smoke staleness guard	PR #1053 review	@coderabbitai
D11	Non-idempotent retry semantics in cli-process.run()	PR #1053 review	@coderabbitai
D12	Deterministic replacement for phase-label TUI e2e	PR #1053 review	Consensus (Claude/GPT/MiMo)
D13	SSH port-forwarding / device-code flow for headless auth	PR #1053 review	@cubic-dev-ai

[coderabbitai]

@sahrizvi, acknowledged.

The issue now records the deferred scope, rationale, and attribution for D10 through D13.

D10 and D11 correctly preserve the review concerns:

• The smoke-test guard must track all binary-embedded build inputs.
• The SQLite-lock retry must not repeat a state-writing opencode run invocation without isolation or reset.

The proposed long-term directions are appropriate.

_{You are interacting with an AI system.}

[sahrizvi]

Additional deferral surfaced during v0.9.4 release

D14 — [HIGH PRIORITY] Cold-start module-load hazard blocks non-interactive run under Linux network isolation (blocked v0.9.4 sanity → had to ship around it)

What surfaces: The Verdaccio sanity suite's Phase 3 "No internet graceful handling" test failed deterministically on the v0.9.4 tag (2 workflow runs, same failure). The failing shell test:

timeout 15 unshare --net altimate run --max-turns 1 --yolo "hello" 2>&1

captured zero bytes to stdout+stderr on Ubuntu 24 x86_64 GitHub runners under unshare --net. The 15-second timeout sent SIGTERM before any bytes flushed. That gated Publish to npm and Create GitHub Release for v0.9.4 (both needs: sanity).

Not reproducible in the ways I tried: Docker on macOS ARM64 with the linux-arm64 or linux-x64 v0.9.4 binary produces non-empty output in the same 15s window (either fast ConnectionRefused on --network none or a proper models.dev error under unshare --net inside a container). Sanity Phase 3 on the same commit passed on v0.9.3's CI run — same test file, same assertion, same runner OS. So something in the delta v0.9.3 → v0.9.4 tipped the CI-specific timing without any code in the suspected hazard's path changing.

Likely mechanism (not proven with high confidence):

• packages/opencode/src/provider/models.ts:154 fires ModelsDev.refresh() at module import via setTimeout(0), unawaited. The outer timer is not .unref()'d; the fetch's AbortSignal.timeout(10000) can't cancel a synchronous getaddrinfo if the kernel resolver is unreachable but not refused.
• On Linux under unshare --net, getaddrinfo("models.dev") can block far longer than under Docker's --network none (which returns fast ConnectionRefused because there's no resolver at all).
• The pending unresolved fetch keeps the Bun event loop alive after the main flow errors on "no API key" (the run command throws to yargs, which prints an error and returns without an explicit process.exit()).
• 15s wall-clock hits → SIGTERM → block-buffered stdio has no bytes to flush.

What blame says: the setTimeout(0) wrapping ModelsDev.refresh() was written by @anandgupta42 on 2026-03-04 (8451ad13ace) and re-added on 2026-05-02 after a bridge merge dropped it (3cc3d4cb68d). Months-old pattern, unchanged between v0.9.3 and v0.9.4.

Why deferred: to fix properly this needs:

1. Either skip the import-time refresh() when the shipped snapshot is present (which it always is in release binaries — src/provider/models-snapshot.ts is 2.4MB embedded), OR
2. Bound the fetch with a proper cancellation path that survives DNS-blocking (hard — getaddrinfo isn't cancellable), OR
3. Force an early process.exit() in the run command's error path so pending I/O can't hold the process, OR
4. Add an early flushed stderr line at CLI entry (defensive — makes the sanity test see SOMETHING before any potential blocking await, but doesn't fix the underlying hang).

Choice between these depends on how we want cold-boot to behave when the model catalog is unreachable — none of them is a drop-in patch, and the CI-specific delta of "same code passes on v0.9.3, fails on v0.9.4" is still not explained.

Impact on real users: none observed. TUI mode loads and renders correctly offline; interactive altimate works. The failure only manifests for non-interactive altimate run "prompt" under bare-Linux unshare --net with stdout captured to a shell variable — a synthetic combination the Verdaccio sanity suite tests but end users don't hit.

How v0.9.4 shipped anyway: the failing check was worked around on the release path — see the release commit(s) that unblocked publish. This issue captures the underlying hazard that needs a proper fix before the guard can be re-enabled.

Attribution: original hazard @anandgupta42 (2026-03-04); surfaced during v0.9.4 sanity run.