Skip to content

feat(onboarding): first-run CTA and workflow-routing handoff - #1488

Merged
flora131 merged 13 commits into
mainfrom
flora131/feature/update-atomic-onboarding
Jun 25, 2026
Merged

feat(onboarding): first-run CTA and workflow-routing handoff#1488
flora131 merged 13 commits into
mainfrom
flora131/feature/update-atomic-onboarding

Conversation

@flora131

@flora131 flora131 commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Implements a one-time first-run onboarding flow for fresh Atomic installs. On the first session with no prior history, the editor shows a bordered CTA header and placeholder guiding the user to paste a ticket, spec, or task prompt, then hands the seed to the normal coding-agent session with goal/ralph routing guidance. Also fixes workflow config/discovery isolation so ATOMIC_CODING_AGENT_DIR is honored consistently.

Key Changes

Onboarding flow (interactive-onboarding.ts — new)

  • Detects first-run via two new settings fields: firstRunOnboardingStartedVersion / onboardedVersion
  • Shows a bordered CTA header (ONBOARDING_COPY) and editor placeholder (ONBOARDING_PLACEHOLDER) on fresh installs
  • Returning users (those with prior changelog state) are automatically marked onboarded and skip the CTA on upgrade
  • /chat escape hatch transitions immediately to a normal coding-agent session with guidance copy; an initial message after the slash is forwarded as-is
  • Accepts tickets, GitHub issues, spec file paths, or free-form task prompts as seeds
  • Hands the first seed to the normal session via buildOnboardingHandoffPrompt, which includes goal/ralph routing guidance inline — no separate probe step
  • Seeds submitted before login are stashed in-memory and auto-resumed after completeProviderAuthentication or after a /model selection step
  • /new during onboarding resets the flow and discards any previously stashed seed
  • Safe cwd-contained path handling — resolves symlinks and rejects path-traversal attempts

Input routing (interactive-input-handling.ts)

  • Intercepts /chat [message] during onboarding for immediate mode transition
  • Routes path-like slash-commands (spec files, local paths with optional :line[:col] suffixes) and non-command text through onboarding when active
  • Guards concurrent submissions with firstRunOnboardingSeedInFlight flag
  • Successful /import <jsonl> exits onboarding UI state so the next message is not treated as a fresh seed

Settings (settings-types.ts, settings-manager-basic-accessors.ts)

  • Added firstRunOnboardingStartedVersion and onboardedVersion to Settings
  • Typed accessors (get/set) with persistent save via the existing settings internals pattern

Startup integration (interactive-startup.ts, interactive-auth-login.ts)

  • init detects first-run eligibility and sets firstRunOnboardingStartedVersion before header render; tracks hadLastChangelogVersionAtStartup to distinguish changelog-aware returning users from true first-timers
  • Injects bordered CTA components (DynamicBorder, Text, Spacer) into the header container on eligible sessions
  • completeProviderAuthentication calls resumePendingFirstRunOnboardingSeed to replay a stashed seed post-login

@mention autocomplete fallback (interactive-autocomplete.ts)

  • Adds AtMentionFallbackAutocompleteProvider that extracts @<path> tokens (including quoted paths) and delegates to the path-autocomplete provider when the primary provider returns no results — improves file-completion ergonomics during onboarding seeds

Workflow config/discovery isolation fix (@bastani/workflows)

  • Fixed config-loader.ts and discovery.ts to use getAgentDir / getAgentDirs instead of hardcoded os.homedir()
  • ATOMIC_CODING_AGENT_DIR now correctly prevents home-global workflows from shadowing the bundled goal / ralph targets
  • homeDir option preserved for legacy test/compat resolution; new agentDirs option added for explicit override

Docs

  • Updated docs/quickstart.md, docs/settings.md, and docs/workflows.md with first-run onboarding references

Tests

Six test files (42+ tests) covering:

  • Startup detection and CTA rendering
  • Returning-user detection (existing changelog state bypasses onboarding)
  • Input routing for seeds, /chat, slash-command paths, /import, and /new
  • Multiline absolute-path seeds with :line[:col] notes preserved on handoff
  • Timeout/cancellation fallback behavior
  • URL-only vs. localizing-evidence heuristics for path seeds
  • Resume and session-clearing edge cases (pre-login stash, /model wait, /new reset)
  • Workflow agent-dir isolation (ATOMIC_CODING_AGENT_DIR precedence over home-global)
  • Custom editor placeholder rendering

Validation

  • bun run typecheck — passed
  • bun run test:unit (incl. all onboarding and isolation tests) — passed
  • bun run check:file-length — passed
  • Commit hooks (large-file, case, conflict, lint, file-length, unit tests) — passed

Notes

  • A P3 race exists: /chat submitted while an onboarding seed is in flight can still allow the pending workflow handoff to complete. Non-blocking; tracked for a follow-up.
  • research/docs/ spec artifact is included for traceability but contains intermediate review notes not intended as user-facing docs.

@flora131

Copy link
Copy Markdown
Collaborator Author

Implementation Notes

Task: Objective:
Implement the first-run onboarding workflow routing behavior specified in specs/2026-06-20-first-run-onboarding-workflow-routing.md and open a pull request when complete.

Context:
Work from /Users/norinlavaee/atomic. Read the specification fully before editing. The relevant implementation is expected to be in the Atomic coding-agent first-run/onboarding/workflow-routing code paths, with user-facing documentation under packages/coding-agent/docs and changelog entries under the appropriate packages/*/CHANGELOG.md ## [Unreleased] sections.

Scope:

  • Make the smallest correct code changes needed to satisfy the spec.
  • Add or update tests that directly cover the specified routing behavior.
  • Update relevant user-facing docs in packages/coding-agent/docs.
  • Update relevant changelog entries under ## [Unreleased] according to repo rules.
  • Use only Bun-based commands for development and validation: bun, bun run, and bun test.

Non-goals:

  • Do not introduce unrelated refactors, redesigns, new features, or behavior changes outside the spec.
  • Do not add build steps or generated dist outputs.
  • Do not bump package versions.
  • Do not exceed the 500-line authored source file limit.

Done criteria:

  • The implemented behavior matches specs/2026-06-20-first-run-onboarding-workflow-routing.md.
  • Existing behavior outside the specified first-run onboarding workflow routing case remains unchanged.
  • Relevant tests are added or updated and pass.
  • bun run typecheck passes.
  • Targeted Bun tests covering the changed behavior pass.
  • bun run check:file-length passes if authored source files changed.
  • Docs and changelog updates are present where appropriate.
  • A PR is created with a clear summary and validation notes.

Validation command:
Run bun run typecheck, targeted bun test ... commands for the affected tests, and bun run check:file-length if source files changed.

Reporting requirements:
In the PR and final report, include:

  • Summary of behavior implemented.
  • Key files changed.
  • Tests and validation commands run, with results.
  • Any remaining risks, assumptions, or follow-up work.

Stop conditions:
Stop and ask for guidance before proceeding if the spec is ambiguous or conflicts with existing behavior, if implementation appears to require public API changes, security-sensitive changes, data migrations, broad architecture changes, version bumps, or changes outside the stated scope.

Running Notes

  • Record implementation decisions, deviations from the research findings, tradeoffs, blockers, validation notes, and anything else the user should know.

Iteration 1 Preflight

  • Repository evidence indicates an initialized Bun + TypeScript monorepo with Rust/N-API native components.
  • bun.lock, node_modules/, Cargo.lock, and Rust target/ are present; no setup command was required before implementation.
  • Fresh-checkout setup would be bun install.
  • Validation and development should use Bun-only commands; packages/workflows must remain raw TypeScript with no build step.

Iteration 1 Implementation Notes

  • Implemented first-run onboarding as a coding-agent-only flow with separate onboardedVersion settings accessors instead of reusing changelog state.
  • Chose a small read-only heuristic assessor for iteration 1 rather than invoking subagents from the TUI startup path. It reads only a cwd-contained single-file spec path when supplied, caps file reads, returns the required structured shape, and grounds its reason in the workflow system-prompt guidance (goal for small/quick fixes, ralph for non-trivial ~2k+ LoC / many files/areas).
  • Workflow launch uses the registered workflow tool directly so success/failure is observable before setting onboardedVersion; on success it emits the existing workflow chat-surface dispatch card. ralph receives only prompt=<raw> and no onboarding max_loops override.
  • Preserved slash commands other than /chat; preserved existing bash-command handling before onboarding interception to avoid changing ! command behavior during first run.
  • Updated quickstart/workflows/settings docs and the coding-agent changelog Unreleased entry.

Iteration 1 Validation

  • bun run typecheck passed.
  • bun run --cwd packages/coding-agent test first-run-onboarding.test.ts interactive-mode-compaction.test.ts passed (10 tests).
  • bun run --cwd packages/coding-agent test interactive-mode-startup-banner.test.ts interactive-mode-startup-input.test.ts first-run-onboarding.test.ts passed (11 tests).
  • bun run check:file-length passed.

Iteration 1 Final Validation

  • Re-ran combined validation after preserving extension slash-command pass-through: bun run typecheck && bun run --cwd packages/coding-agent test first-run-onboarding.test.ts interactive-mode-compaction.test.ts interactive-mode-startup-banner.test.ts interactive-mode-startup-input.test.ts && bun run check:file-length passed.

Iteration 1 Implementation

  • Implementer added separate onboardedVersion settings state, first-run CTA copy, onboarding placeholder, /chat escape hatch, slash-command pass-through, a lightweight structured onboarding routing assessor, and workflow launch mapping (goal.objective / ralph.prompt).
  • Onboarding is marked complete only after /chat or successful workflow start; failed launches/auth/model errors should not complete onboarding.
  • Tradeoff: the first iteration uses a bounded heuristic/read-only assessor inside the TUI flow rather than spawning subagents directly from onboarding. It still quotes/summarizes workflow guidance from packages/workflows/src/extension/workflow-prompts.ts and avoids full research artifacts.
  • Implementer reported validation passed: bun run typecheck, bun run --cwd packages/coding-agent test first-run-onboarding.test.ts interactive-mode-compaction.test.ts interactive-mode-startup-banner.test.ts interactive-mode-startup-input.test.ts, and bun run check:file-length.

Iteration 1 Follow-up

  • Gap analysis found the original heuristic-only assessor did not fully satisfy the spec's lightweight subagent/model-backed scope probe requirement.
  • Follow-up changed runOnboardingRoutingAssessment() to try the existing subagent tool first, run one bounded read-only codebase-locator probe with a 10-minute timeout, request compact routing JSON, and conservatively validate the result.
  • The deterministic heuristic remains as fallback when the subagent tool is unavailable, errors/times out, or returns invalid JSON. No custom auth/login handling was added.
  • The probe is intentionally small (output:false, reads:false, artifacts:false) to avoid durable research artifacts and keep onboarding bounded.
  • Implementer reported focused tests for valid subagent result routing, invalid fallback, and tool-unavailable fallback.

Iteration 1 Validation

  • Required validation rerun from the repository root after the follow-up:
    • bun run typecheck — passed.
    • bun run --cwd packages/coding-agent test first-run-onboarding.test.ts interactive-mode-compaction.test.ts interactive-mode-startup-banner.test.ts interactive-mode-startup-input.test.ts — passed (4 test files, 17 tests).
    • bun run check:file-length — passed (1744 tracked files checked, max 500 lines).
  • Independent validation originally could not run commands due subagent environment limitations, but it did identify the heuristic-only assessor gap that was fixed.
  • Terminal/TUI E2E is applicable in principle with isolated temp ATOMIC_CODING_AGENT_DIR and ATOMIC_CODING_AGENT_SESSION_DIR; it was not run because the delegated tmux environment lacked shell/tmux execution. The narrower targeted tests cover the executable onboarding behaviors.

QA E2E Video

  • No QA E2E video applies: this change is terminal/TUI onboarding behavior, not browser UI. Playwright/video evidence was not produced.

Final State Clarification

  • Earlier notes about a heuristic-only assessor describe the initial implementation state. The final implementation supersedes that with an optional subagent-backed read-only probe plus heuristic fallback.

Iteration 2 Reviewer Fixes

  • Stored the rendered first-run onboarding header/CTA components on the interactive mode instance and remove them in completeFirstRunOnboarding(), so /chat and successful onboarding workflow launches clear the CTA from the current TUI surface immediately.
  • Added startup-order-safe fresh-install gating with hadLastChangelogVersionAtStartup, captured before getChangelogForDisplay() mutates lastChangelogVersion; onboarding now requires an empty session, missing onboardedVersion, and no pre-existing changelog version, excluding upgraded installs while preserving resumed/non-empty session gating.
  • Changed the subagent-backed onboarding scope probe to propagate tool.execute() rejections and isError results (for auth/model/tool execution errors) through existing input error handling instead of falling back; invalid/no useful probe output and missing subagent tool still use the heuristic fallback.
  • Extended packages/coding-agent/test/first-run-onboarding.test.ts to cover current-surface CTA removal, startup-order fresh-install gating vs upgraded installs, and subagent execution-error propagation.
  • Docs/changelog wording already described fresh-install-only behavior and did not need iteration-2 changes.

Iteration 2 Validation

  • bun run --cwd packages/coding-agent test first-run-onboarding.test.ts — passed (14 tests).
  • bun run typecheck — passed.
  • bun run --cwd packages/coding-agent test first-run-onboarding.test.ts interactive-mode-compaction.test.ts interactive-mode-startup-banner.test.ts interactive-mode-startup-input.test.ts — passed (4 files, 21 tests).
  • bun run check:file-length — passed (1744 tracked files checked, max 500 lines).

Iteration 2 Orchestrator Validation

  • Re-ran combined validation after iteration-2 fixes:
    • bun run typecheck — passed.
    • bun run --cwd packages/coding-agent test first-run-onboarding.test.ts interactive-mode-compaction.test.ts interactive-mode-startup-banner.test.ts interactive-mode-startup-input.test.ts — passed (4 test files, 21 tests).
    • bun run check:file-length — passed (1744 tracked files checked, max 500 lines).
  • Post-iteration-2 review confirmed the three latest P2 findings are resolved: current-surface CTA removal, fresh-install-only gating with pre-mutation changelog state, and propagation of subagent probe execution/tool errors.
  • Docs and changelog remained accurate; no iteration-2 docs/changelog edits were needed.
  • Browser QA E2E video remains not applicable because this is terminal/TUI behavior. Terminal E2E was not run in this stage; targeted tests provide the validation evidence.

Iteration 3 Review-Round-2 Fixes

  • Treated existing cwd-local absolute file paths as onboarding workflow seeds before slash-command handling, so absolute spec paths like /Users/.../spec.md do not get mistaken for slash commands.
  • Added a separate internal firstRunOnboardingStartedVersion state. onboardedVersion remains completion-only, while the started marker prevents upgraded installs from seeing onboarding without treating changelog state as onboarding completion.
  • Changed subagent scope-probe failure handling so abort/timeout-shaped errors fall back to heuristic routing, while auth/model/tool execution failures and explicit isError results still propagate through existing error handling and do not complete onboarding.
  • Included bounded cwd-local spec excerpts in the primary subagent probe prompt using the existing path/size safeguards.
  • Updated targeted tests for all four review-round-2 findings and adjusted settings docs for the new internal marker.

Iteration 3 Validation

  • Re-ran combined validation after iteration-3 fixes:
    • bun run typecheck — passed.
    • bun run --cwd packages/coding-agent test first-run-onboarding.test.ts interactive-mode-compaction.test.ts interactive-mode-startup-banner.test.ts interactive-mode-startup-input.test.ts — passed (4 test files, 24 tests).
    • bun run check:file-length — passed (1744 tracked files checked, max 500 lines).
  • Post-iteration-3 review confirmed the four latest P2 findings are resolved.
  • Remaining noted risk: timeout fallback uses the deterministic fallback assessment rather than partial subagent findings, which matches the current minimal implementation but does not enrich the reason with timeout context.
  • Browser QA E2E video remains not applicable because this is terminal/TUI behavior. Terminal E2E was not run in this stage; targeted tests provide validation evidence for the review-round-2 fixes.

Iteration 4 Review-Round-3 Fixes

  • Timeout/cancel-shaped isError: true subagent probe results now fall back to the bounded heuristic instead of hard-erroring; auth/model/tool failures still propagate.
  • Spec path handling now uses lstat/realpath containment before reading to avoid symlink escapes while preserving cwd containment and size caps.
  • Existing cwd-local spec paths with spaces are allowed as onboarding seeds/spec reads, while non-path slash commands remain slash commands.
  • First-run onboarding startup marker is not set when initialMessage or non-empty initialMessages are present.
  • Unknown/vague non-localized heuristic fallback now routes conservatively to ralph; clearly localized/small work still routes to goal.
  • Inconsistent structured probe output is normalized to ralph when estimates indicate broad/non-trivial work despite workflow: "goal".
  • Implementer added focused tests for all six review-round-3 findings.

Iteration 4 Validation

  • Re-ran combined validation after iteration-4 fixes:
    • bun run typecheck — passed.
    • bun run --cwd packages/coding-agent test first-run-onboarding.test.ts interactive-mode-compaction.test.ts interactive-mode-startup-banner.test.ts interactive-mode-startup-input.test.ts — passed (4 test files, 27 tests).
    • bun run check:file-length — passed (1744 tracked files checked, max 500 lines).
  • Post-iteration-4 review confirmed the six latest review-round-3 P2 findings are resolved.
  • Docs and changelog remain accurate for user-facing behavior; the research handoff still describing those findings as unresolved is stale relative to the current code/tests.
  • Browser QA E2E video remains not applicable because this is terminal/TUI behavior. Terminal E2E was not run in this stage; targeted regression tests cover the six logic paths directly.

Iteration 5 Review-Round-4 Fixes

  • Excluded URL tokens from fallback path-like heuristics so bare GitHub issue/docs URLs route conservatively to ralph instead of being mistaken for localized file-path scope.
  • Added a non-completing onboarding clear path after successful /resume into an ineligible/non-fresh session: CTA/placeholder/interception are removed without setting onboardedVersion.
  • Added an in-flight onboarding seed guard to prevent duplicate scope probes/workflow launches when submit fires repeatedly while the first onboarding seed is still processing.
  • Added focused round-4 regression tests in a new packages/coding-agent/test/first-run-onboarding-round4.test.ts file to keep authored test files under the 500-line limit.
  • No docs/changelog update was made because these were pre-release edge-case hardenings and existing user-facing wording stayed accurate.

Iteration 5 Validation

  • Re-ran combined validation after iteration-5 fixes:
    • bun run typecheck — passed.
    • bun run --cwd packages/coding-agent test first-run-onboarding.test.ts first-run-onboarding-round4.test.ts interactive-mode-compaction.test.ts interactive-mode-startup-banner.test.ts interactive-mode-startup-input.test.ts — passed (5 test files, 30 tests).
    • bun run check:file-length — passed (1744 tracked files checked, max 500 lines).
  • Post-iteration-5 review confirmed the three latest review-round-4 findings are resolved.
  • Non-blocking risks noted by review: the duplicate-submission guard lives in submit-handler onboarding seed paths rather than inside handleOnboardingWorkflowSeed() for direct internal callers; /chat can still complete onboarding while an earlier seed is in flight, although it does not duplicate probes or workflow launches.
  • Browser QA E2E video remains not applicable because this is terminal/TUI behavior. Terminal E2E was not run in this stage; targeted regression tests cover the round-4 logic paths directly.

Iteration 6 Reviewer-B Fixes

  • Added bounded targeted follow-up probes beyond codebase-locator: codebase-analyzer, codebase-pattern-finder, and codebase-online-researcher can be invoked when locator output is missing/insufficient and the seed indicates those signals are needed. The probes remain read-only/no-artifacts and bounded to avoid becoming full research.
  • URL-only fix/bug seeds now route conservatively to ralph unless non-URL localizing evidence exists.
  • Timeout/abort/cancel fallback assessments now include explicit low-confidence wording in the fallback reason.
  • Added focused round-5 regression tests in packages/coding-agent/test/first-run-onboarding-round5.test.ts.

Iteration 6 Review Follow-up

  • Post-iteration-6 review found the main reviewer-b requirements were implemented, but noted robustness risks: targeted follow-up probes were skipped when locator returned numeric estimates even if pattern/external evidence was indicated, and multiple follow-up outputs were parsed by first valid result rather than conservative reconciliation.

Iteration 6 Follow-up Hardening

  • Follow-up probe selection now still runs pattern/online probes when seed indicators are present, even if the locator returned numeric estimates. Analyzer follow-up remains limited to missing/insufficient scope estimates.
  • Multiple valid follow-up probe results are reconciled conservatively: maximum line/file estimates, union touched areas, and ralph wins when any probe indicates broad/non-trivial scope.
  • Added round-5 tests for numeric locator output plus pattern/online indicators and conservative reconciliation where a later broad result overrides an earlier goal result.
  • Implementer also tried broader non-required validation: broad bun run --cwd packages/coding-agent test had one unrelated session-id-readonly auth-diagnostic mismatch, and bun run --cwd packages/coding-agent build failed during a bunx tsgo spawned copy step. Required targeted validation is recorded separately.

Iteration 6 Second Follow-up Hardening

  • Reconciled locator and follow-up probe assessments together before final routing, so broad/ralph evidence from the locator cannot be masked by a narrower follow-up result.
  • Added final URL-only/no-localizing-evidence enforcement after probe normalization, so valid subagent goal output for URL-only fix/bug seeds is conservatively normalized to ralph.
  • Added round-5 regression tests for broad locator plus narrow follow-up and URL-only subagent goal output.
  • A non-required bun run --cwd packages/coding-agent build attempt failed in an existing asset-copy/declaration step (bunx tsgo status null); required validation is recorded separately.

Iteration 6 Validation

  • Re-ran combined validation after iteration-6 fixes and follow-up hardening:
    • bun run typecheck — passed.
    • bun run --cwd packages/coding-agent test first-run-onboarding.test.ts first-run-onboarding-round4.test.ts first-run-onboarding-round5.test.ts interactive-mode-compaction.test.ts interactive-mode-startup-banner.test.ts interactive-mode-startup-input.test.ts — passed (6 test files, 37 tests).
    • bun run check:file-length — passed (1744 tracked files checked, max 500 lines).
  • Final post-iteration-6 review confirmed reviewer-b findings and follow-up risks are resolved with no remaining blockers in the requested areas.
  • Docs/changelog remain accurate at the user-facing level; no iteration-6 docs/changelog edits were needed.
  • Browser QA E2E video remains not applicable because this is terminal/TUI behavior. Terminal E2E was not run in this stage; targeted regression tests cover the routing/probe decision logic directly.

Iteration 7 Fixes

  • Reconciled latest research against current code: the original reviewer-b findings were already resolved, but review found two remaining issues.
  • Follow-up timeout/cancel fallback now preserves usable locatorAssessment ?? fallback instead of discarding locator partial findings, adds low-confidence wording, and applies seed conservatism.
  • Split probe parsing/reconciliation/tool-error helper logic into packages/coding-agent/src/modes/interactive/interactive-onboarding-probe.ts to bring interactive-onboarding.ts below the 500-line authored-source limit.
  • Added a round-5 regression test where broad ralph locator evidence is preserved through follow-up timeout/cancel.

Iteration 7 Validation

  • Re-ran combined validation after iteration-7 fixes:
    • bun run typecheck — passed.
    • bun run --cwd packages/coding-agent test first-run-onboarding.test.ts first-run-onboarding-round4.test.ts first-run-onboarding-round5.test.ts interactive-mode-compaction.test.ts interactive-mode-startup-banner.test.ts interactive-mode-startup-input.test.ts — passed (6 test files, 38 tests).
    • bun run check:file-length — passed (1744 tracked files checked, max 500 lines).
  • Final iteration-7 review confirmed follow-up timeout/cancel preserves locator partial findings and the onboarding files are below the 500-line gate. Reviewer-b findings remain resolved.
  • Docs/changelog remain accurate at the user-facing level; no iteration-7 docs/changelog edits were needed.
  • Browser QA E2E video remains not applicable because this is terminal/TUI behavior and routing-assessment control flow. Terminal E2E was not run in this stage; targeted tests directly exercise the changed paths.

Iteration 8 Review-Round-7 Fixes

  • Initial-message launches now persist the first-run firstRunOnboardingStartedVersion marker while keeping onboarding inactive for that run, so initial prompts do not permanently suppress a later true fresh interactive onboarding.
  • Successful /resume now clears active onboarding UI/interception without setting onboardedVersion, even when the resumed existing session is empty.
  • Workflow launch success no longer depends on the follow-up chat-surface status card: if sendCustomMessage() fails after the workflow run starts, Atomic warns and still allows onboarding completion.
  • Existing outside-cwd absolute paths are routed as raw onboarding seeds instead of slash commands, but spec reading remains cwd-contained and does not read outside-cwd files.
  • Added focused round-7 regression tests in packages/coding-agent/test/first-run-onboarding-round7.test.ts.

Iteration 8 Validation

  • Re-ran combined validation after iteration-8 fixes:
    • bun run typecheck — passed.
    • bun run --cwd packages/coding-agent test first-run-onboarding.test.ts first-run-onboarding-round4.test.ts first-run-onboarding-round5.test.ts first-run-onboarding-round7.test.ts interactive-mode-compaction.test.ts interactive-mode-startup-banner.test.ts interactive-mode-startup-input.test.ts — passed (7 test files, 42 tests).
    • bun run check:file-length — passed (1744 tracked files checked, max 500 lines).
  • Post-iteration-8 review confirmed the four latest review-round-7 findings are resolved.
  • Residual non-blocking notes: the missing-cwd resume branch uses identical clear-on-resume behavior but lacks a dedicated regression test; no onboarding-specific terminal E2E/video coverage was produced.
  • Docs/changelog remain broadly accurate at the user-facing level; no iteration-8 docs/changelog edits were needed.
  • Browser QA E2E video remains not applicable because this is terminal/TUI behavior and state/input-routing control flow.

@mintlify

mintlify Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
bastani 🟢 Ready View Preview Jun 23, 2026, 1:57 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@claude claude Bot changed the title feat(onboarding): route first-run work to workflows feat(onboarding): first-run workflow routing with scope probe Jun 23, 2026
@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown

Review: feat(onboarding): route first-run work to workflows

Thorough, well-tested change — the scope-routing fallbacks (timeout/abort/invalid-JSON to heuristic), the cwd-contained spec reading with symlink-escape protection, and the conservative reconciliation all look carefully thought through, and the test matrix (in-flight dedup, URL-only seeds, symlink escape, copied-settings re-arm) is impressive. A few items worth addressing before merge.

Bugs / correctness

1. /chat during an in-flight seed probe (the noted P3) — easy to close.
In interactive-input-handling.ts, the path-seed branch (:124) and the plain-text branch (:307) both early-return on this.firstRunOnboardingSeedInFlight, but the /chat branch (:108) does not. So if a user submits a seed (scope probe starts, up to ~10 min) and then types /chat, completeFirstRunOnboarding() runs and marks them onboarded, yet the in-flight handleOnboardingWorkflowSeed keeps going and still calls launchOnboardingWorkflow() — a workflow launches even though the user explicitly opted into chat. The PR flags this as non-blocking, but it is a one-line guard: have the /chat branch check firstRunOnboardingSeedInFlight and either ignore or surface "scope check in progress — press esc to cancel" rather than silently completing. Right now it is an actual unexpected-workflow-launch, not just a cosmetic race.

2. Shared 10-minute timeout budget across both probes.
runOnboardingRoutingAssessment creates one AbortSignal.timeout(600_000) (interactive-onboarding.ts:307) and reuses it for both the locator probe and the follow-up probe (:357). That is a defensible total-budget design, but it means a slow locator can starve the follow-up of nearly all its time, and 10 minutes staring at a "quick read-only scope check" status is a long worst case for a first-run experience. Consider a tighter per-probe budget (or at least documenting that 600s is the combined ceiling), since the heuristic fallback makes a shorter timeout cheap.

Code quality / DRY

3. Duplicated helpers between interactive-onboarding.ts and interactive-onboarding-probe.ts.
URL_TOKEN_PATTERN, PATH_LIKE_TOKEN_PATTERN, unique, removeUrlTokens, hasUrlToken, and hasUrlOnlyWithoutLocalizingEvidence are defined verbatim in both files. These are the core scope-detection primitives — if the URL/path regexes ever diverge, the heuristic (assessOnboardingRoute) and the conservatism enforcement (enforceSeedConservatism) will silently disagree on what counts as "localizing evidence." Hoist them into the probe module (or a small shared util) and import, since interactive-onboarding.ts already imports from the probe file.

Minor / nits

  • The scope heuristic keyword lists (broadWords, the looksTiny regex, extractTouchedAreas keyword map) are inline magic. They are fine, but a brief comment that these are intentionally coarse "first-pass only, the probe/fallback refines" would help the next maintainer not over-trust them.
  • isExistingAbsolutePathSeed runs a statSync on arbitrary absolute paths for every /-prefixed onboarding submit. The actual file read is correctly cwd-sandboxed via getContainedExistingPath, so this is not a path-traversal concern — just noting the unsandboxed stat is intentional and limited to existence/type checks.

What is good

  • getContainedExistingPath resolving realpathSync on both root and target before the isInside check is the right way to defeat symlink escape, and there is a test for it.
  • Onboarding state (firstRunOnboardingStartedVersion / onboardedVersion) is cleanly separated from lastChangelogVersion, with tests covering the copied-settings re-arm and the "do not re-onboard after completion" cases.
  • Workflow config-loader refactor to getAgentDirs() / ATOMIC_CODING_AGENT_DIR (away from hardcoded homedir()) is a nice correctness fix with isolation coverage, and it preserves the legacy homeDir path for back-compat.

Nice work overall — items 1 and 3 are the ones I would want resolved; 2 is a judgment call.

🤖 Generated with Claude Code

Add first-run onboarding behavior that stashes pre-login seeds, hands ready seeds to normal workflow routing, and documents the updated flow.

Assistant-model: GPT-5.5
…te-atomic-onboarding

# Conflicts:
#	packages/coding-agent/CHANGELOG.md
@claude

claude Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review: first-run onboarding workflow routing

Nice, well-tested feature overall — the path-safety handling and edge-case coverage are genuinely thorough. A few things worth addressing before merge, the first of which is significant.

🔴 The scope-probe subsystem is no longer wired into the runtime flow

The actual onboarding submit path is:

onSubmitsubmitFirstRunOnboardingSeedhandleOnboardingWorkflowSeedbuildOnboardingHandoffPromptonInputCallback

handleOnboardingWorkflowSeed (interactive-onboarding.ts:444) just hands a prompt to the normal session and lets the agent self-route. As a result, none of the probe machinery is reachable in production:

  • runOnboardingRoutingAssessment (~80 lines) — only referenced by the surface type + tests
  • launchOnboardingWorkflow — same
  • assessOnboardingRoute and effectively the entire interactive-onboarding-probe.ts (153 lines) — reachable only through the two dead methods above

I grepped `packages/coding-agent/src` for callers and found none outside the definitions themselves and the `interactive-mode-surface.ts` declarations. So ~300 lines of probe/reconciliation logic plus three of the test files (`round4/5/7`) exercise behavior that no longer ships in the user flow. This bloats the bundle and creates a maintenance trap (tests stay green while testing dead code).

Recommendation: either re-wire `handleOnboardingWorkflowSeed` to call `runOnboardingRoutingAssessment` + `launchOnboardingWorkflow`, or delete the probe subsystem (and its tests) and keep the simpler prompt-handoff design. Right now the PR ships both and uses only one.

🟠 PR description / spec no longer match the implementation

The description says the flow "runs a bounded read-only scope probe… and routes to `goal` or `ralph` based on probe output," and the `specs/` doc describes the same. The shipped behavior instead delegates routing to the LLM via the handoff prompt (`buildOnboardingHandoffPrompt`). Please reconcile the description/spec with what actually executes so reviewers and future readers aren't misled. (The last commit, "update first-run workflow handoff," appears to be where the approach changed.)

🟡 Slash-path interception can capture real filesystem paths

In `interactive-input-handling.ts:147`, during onboarding any `/`-prefixed input that resolves to an existing file/dir is treated as a seed before the command handlers run. Typing something like `/tmp` or `/etc` (which exist as directories) during onboarding would be swallowed as a task seed. Impact is low since those aren't real commands, but the precedence is surprising. Worth confirming it can't shadow a real slash command that also happens to exist as a path, and perhaps restricting to files (not directories) or requiring a recognizable spec extension.

🟡 `firstRunOnboardingStartedVersion` is persisted before any interaction

`interactive-startup.ts` writes `setFirstRunOnboardingStartedVersion(this.version)` during `init()` on a fresh install, before the user does anything. If that's intentional (distinguishing a fresh-but-immediately-quit session from a genuinely new one), fine — but it means the very first `atomic` launch always incurs a settings write. Please confirm this is the intended semantics.

🟢 Things that look good

  • Path containment via `realpathSync` + `isInside` correctly resolves symlinks and rejects traversal; bounded reads (256 KB stat cap, 64 KB slice) are sensible.
  • Test framework choice (`vitest`) is consistent with the rest of `packages/coding-agent` (244 files), so the CLAUDE.md `bun:test` note doesn't apply here — good call.
  • The `@bastani/workflows` `getAgentDir`/`getAgentDirs` change is the right fix for `ATOMIC_CODING_AGENT_DIR` isolation; `toScopedDiscoveryConfig` pre-resolving global paths to absolute keeps the downstream `homeDir`-based `loadFromPaths` consistent, so no resolution mismatch there.
  • `clearFirstRunOnboardingUi` on resume and the stash/resume-after-login flow are nice touches.

Minor

  • `specs/2026-06-20-*.md` (492 lines) is included as a draft artifact; the description notes it contains "intermediate review notes not intended as user-facing docs." Consider trimming to the final design or dropping it to avoid future confusion.
  • The PR's own "P3 race" note (`/chat` during an in-flight seed) is largely mooted by the synchronous handoff, but the stash→`resumePendingFirstRunOnboardingSeed` path after login could still interleave — worth a follow-up test.

Overall: solid groundwork and excellent test discipline. The main blocker is deciding whether the probe path is in or out and aligning the code + description accordingly.

🤖 Generated with Claude Code

@flora131 flora131 self-assigned this Jun 24, 2026
@claude claude Bot changed the title feat(onboarding): first-run workflow routing with scope probe feat(onboarding): first-run CTA and workflow-routing handoff Jun 24, 2026
@claude

claude Bot commented Jun 24, 2026

Copy link
Copy Markdown

Code Review — first-run onboarding with workflow routing

Thanks for this — the feature is cleanly modularized, the new interactive-onboarding.ts is well under the 500-line gate, the prototype-extension pattern matches the surrounding interactive-* files, and the @bastani/workflows agent-dir isolation fix is a genuinely nice correctness improvement (preserving homeDir for legacy/test resolution while adding agentDirs/getAgentDir(s) is the right backward-compatible shape). Tests using vitest are consistent with the coding-agent package (it has 243 vitest files), so no convention issue there.

A few things worth addressing before merge:

🔴 1. PR description is significantly out of sync with the implementation

The summary describes a "lightweight read-only scope probe" in interactive-onboarding-probe.ts that "assesses complexity and routes the input to either goal or ralph", with "timeout/cancellation fallback" and "probe assessment parsing" tests. None of that exists in the diff — there is no probe file, no programmatic routing, and no timeout logic. The actual implementation (buildOnboardingHandoffPrompt) hands the seed to a normal session with prose instructions asking the model to pick goal/ralph itself. The CHANGELOG correctly reflects this ("Removed the unused first-run onboarding scope-probe/routing-assessment subsystem…"), so the code is fine — but the PR body (and the title "with scope probe") is stale and will mislead reviewers. Please update the description/title to match. The "research/docs/ spec artifact is included" note is also inaccurate — the only added artifact is specs/2026-06-20-first-run-onboarding-workflow-routing.md.

🟠 2. Dead field hadLastChangelogVersionAtStartup — and a likely upgrade-user gating gap

hadLastChangelogVersionAtStartup is declared (interactive-mode-base.ts:126, interactive-mode-surface.ts:14) and assigned (interactive-startup.ts:51) but never read in production — only in a test. Neither the init gate (interactive-startup.ts:54-60) nor isFirstRunOnboardingEligible (interactive-onboarding.ts:122-128) consults it. The consequence:

An existing user upgrading to this version starts each new session with messages.length === 0, no onboardedVersion, and no firstRunOnboardingStartedVersion → the gate sets the started version → they see the "fresh install" CTA.

The computed-but-unused field strongly suggests the intent was to gate on it (don't onboard users who already have changelog history = not fresh installs), but the wiring was dropped. Please either (a) wire it into the gate/eligibility if existing users should be excluded, or (b) delete the dead field and confirm onboarding-for-existing-users is intentional. As-is, the behavior contradicts the stated "for fresh Atomic installs" goal.

🟡 3. The cwd-containment path logic is effectively a no-op at the call site

interactive-input-handling.ts:147 routes a /-prefixed input as a seed when isCwdLocalExistingPathSeed(...) || isExistingAbsolutePathSeed(...). Since isExistingAbsolutePathSeed accepts any existing absolute path (no containment check), the || makes the careful realpathSync/isInside symlink-resolution logic in isCwdLocalExistingPathSeed (interactive-onboarding.ts:51-79) irrelevant to the routing decision — both branches require only an absolute path that exists, and both feed the full original text to the prompt regardless. So the description's "rejects path-traversal attempts" isn't actually enforced here. In practice the seed is just prompt text (low risk), but the containment code reads as a security guarantee it doesn't provide. Consider dropping the redundant predicate or making containment actually decide routing.

🟡 4. Model-delegated routing has no guarantee

Because routing is now prose in buildOnboardingHandoffPrompt, there is no guarantee the model actually starts a goal/ralph workflow vs. just chatting. Reasonable tradeoff for a v1, but worth a code comment and an honest framing in docs/changelog (the title oversells "routing").

Minor / nits

  • Non-existent spec path during onboarding: a /-prefixed path that doesn't exist (typo, not-yet-created spec) fails both predicates and falls through to normal slash-command handling → "unknown command" rather than being treated as a seed. Minor UX sharp edge.
  • /chat history: in the /chat branch, addToHistory is only reached when there is a trailing message; bare /chat returns before recording history. Harmless inconsistency.
  • Known P3 race (noted in the PR): /chat while a seed is in flight can still let the pending launch complete. Fine to defer — please file the follow-up so it is not lost.
  • I did not re-run bun run typecheck / test:unit; trusting the reported green run. Note hadLastChangelogVersionAtStartup is a class field, so noUnusedLocals will not flag the dead write.

Overall: solid, well-isolated feature with good coverage of the eligibility/handoff/clearing paths and the workflow-dir isolation. The blockers for me are reconciling the description with reality (#1) and resolving the upgrade-user gating intent (#2).

Raise onboarding handoff routing to high reasoning when supported, expand scope-routing guidance, and keep docs/changelog aligned.

Fix @ file-reference autocomplete during first-run onboarding by falling back to synchronous path completion before fd readiness.

Assistant-model: GPT-5.5
…te-atomic-onboarding

Assistant-model: GPT-5.5

# Conflicts:
#	packages/coding-agent/CHANGELOG.md
#	packages/coding-agent/docs/quickstart.md
#	packages/workflows/CHANGELOG.md
@claude

claude Bot commented Jun 25, 2026

Copy link
Copy Markdown

PR Review: first-run onboarding CTA + workflow-routing handoff

Thanks for this — it's a well-structured feature. The onboarding logic is cleanly isolated in a new interactive-onboarding.ts, the settings accessors follow the existing markModified/save pattern, the workflow isolation fix is backward-compatible (legacy homeDir preserved), and test/changelog coverage is strong. Notes below, roughly by severity.

Correctness / bugs

  1. /chat is not guarded by firstRunOnboardingSeedInFlight (the acknowledged P3 race). In interactive-input-handling.ts:131, the /chat branch calls completeFirstRunOnboarding() immediately, while submitFirstRunOnboardingSeed guards with the in-flight flag. If /chat is submitted while a seed handoff is awaiting, both paths run completeFirstRunOnboarding()/clearPendingFirstRunOnboardingSeed() and you get a double-completion. You already track this as a follow-up, but the guard is cheap — adding if (this.firstRunOnboardingSeedInFlight) return; (or routing /chat through the same in-flight check) would close it now rather than later.

  2. isOnboardingWorkflow() in dispatch-confirm.ts applies to every goal/ralph run, not just onboarding ones. The extra status/connect+"ask here" hint rows now render for all goal/ralph dispatches regardless of onboarding state. The surrounding comments ("those are the first-run onboarding targets") imply onboarding-only intent. If that was the intent, this is a behavior change for all existing users; if the always-on UX is desired, the comments/naming are misleading. Worth confirming which you meant.

  3. handleOnboardingWorkflowSeed raises the session to high reasoning permanently. this.session.setThinkingLevel("high") is never restored after the handoff. The handoff prompt explains the bump to the model, but from the user's perspective their thinking level is silently changed for the rest of the session. Intended? If so, a one-line note in the status copy ("raised reasoning to high for routing") would set expectations.

Security / framing

  1. isExistingAbsolutePathSeed is not cwd-contained, which contradicts the PR's "Safe cwd-contained path handling" claim. isCwdLocalExistingPathSeed does proper realpath + containment (nicely done — traversal via .. is correctly rejected by isInside). But the || isExistingAbsolutePathSeed(text) fallback at interactive-input-handling.ts:147 accepts any existing absolute path on the machine (/etc/passwd, etc.) as a seed. The actual risk is low — the path is only embedded as text into the handoff prompt, not auto-read by onboarding — but the description's security framing overstates the containment guarantee. Either tighten the fallback to cwd or soften the claim.

Code quality

  1. Direct mutation of headerContainer.children instead of removeChild. clearFirstRunOnboardingUi (interactive-onboarding.ts:196) reassigns this.headerContainer.children = children.filter(...). The rest of the codebase uses the Container.removeChild(...) API (see interactive-queueing.ts:186, interactive-agent-events.ts:249, interactive-extension-widgets.ts:90). Direct reassignment can bypass any internal bookkeeping the container does on add/remove; prefer iterating firstRunOnboardingHeaderComponents and calling removeChild for consistency and safety.

  2. Minor DRY. workflowAgentDirs() is duplicated verbatim in both config-loader.ts and discovery.ts. And applyBatch/applyBatchShapeOnly in discovery.ts are near-identical (the only difference is one being async) — note applyBatch is declared async but contains no await. These are in the touched area; a small shared helper would reduce drift.

Process / docs nits

  1. PR description is slightly out of sync with the diff. It mentions a research/docs/ artifact that isn't in the changeset (the spec lands at specs/2026-06-20-first-run-onboarding-workflow-routing.md), and it omits the sizable AtMentionFallbackAutocompleteProvider change in interactive-autocomplete.ts (that one is in the changelog, good). Worth confirming the 258-line specs/*.md is meant to be tracked in-repo — CLAUDE.md points at DESIGN.md/PRODUCT.md but doesn't mention a specs/ convention.

Smaller observations

  • handleResumeSession / import clear the onboarding UI but don't call completeFirstRunOnboarding(), so onboardedVersion stays unset and the CTA can reappear on the next fresh session. Probably intended (the user never engaged the seed flow), just flagging for confirmation.
  • Bare /chat doesn't get added to editor history while /chat <msg> does (interactive-input-handling.ts:143 sits after the empty-message return). Trivial inconsistency.
  • The returning-user detection via hadLastChangelogVersionAtStartup captured before getChangelogForDisplay() mutates lastChangelogVersion is a nice, correct ordering detail.

Nothing here is blocking except possibly #2 (depends on intent). Nice work overall.

🤖 Generated with Claude Code

@claude

claude Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review: first-run CTA and workflow-routing handoff

Reviewed the full diff (38 files). Overall this is a well-structured, well-tested feature. The settings-driven state machine (firstRunOnboardingStartedVersion / onboardedVersion), the careful separation of "true first-timer" vs. "returning user" via hadLastChangelogVersionAtStartup, and the security-conscious cwd-containment path handling are all solid. The workflow agent-dir isolation fix is correct and the test coverage (42+ tests, including symlink-escape and multiline-seed edge cases) is genuinely thorough. Nice work.

A few things worth a look before merge:

Substantive

1. The handoff permanently changes the user's default thinking level. handleOnboardingWorkflowSeed calls this.session.setThinkingLevel("high") (interactive-onboarding.ts:214). setThinkingLevel clamps for non-reasoning models (good — no error there), but for reasoning models it hits settingsManager.setDefaultThinkingLevel(effectiveLevel) (agent-session-models.ts:202), which persists "high" as the user's global default going forward. The handoff prompt copy describes this as switching "to high reasoning for this routing decision," but the change is permanent, not scoped to the routing pass. If the intent is per-decision, consider restoring the prior level after dispatch (or at least not persisting it). At minimum the discrepancy between copy and behavior is worth resolving.

Minor

2. setThinkingLevel("high") is unconditional but the prompt says "when the model supports it." The clamp makes it safe, but the code doesn't gate on supportsThinking() the way the copy implies. Either gate it or adjust the copy so the two agree.

3. In-memory stash vs. "Task saved for after login" copy. pendingFirstRunOnboardingSeed is in-memory only. If the user reads "Task saved for after login," quits, and relaunches, the seed is silently gone. Consider softening the copy (e.g. "saved for this session") or noting the volatility.

4. /chat history inconsistency. In the /chat branch (interactive-input-handling.ts:131-145), addToHistory(text) runs only when a trailing message is present; bare /chat returns before reaching it. Minor, but /chat alone won't be recallable from history while /chat foo will.

5. Silent in-flight guard. submitFirstRunOnboardingSeed early-returns when firstRunOnboardingSeedInFlight is set (interactive-input-handling.ts:105) without clearing the editor or giving feedback — the text just sits there. This is the acknowledged P3 race; a brief status note would improve UX if you touch it.

Questions / nits

6. Redundant declaration. firstRunOnboardingHeaderComponents: Component[] is declared both as a class field (interactive-mode-base.ts:129) and in the declare module augmentation (interactive-mode-surface.ts:15). Typecheck passes, but the data-field duplication is unusual versus the method-only pattern used elsewhere in surface — worth confirming it's intentional.

7. isExistingAbsolutePathSeed is filesystem-wide. Unlike isCwdLocalExistingPathSeed, it accepts any existing absolute path with no containment check (interactive-onboarding.ts:82). Appears intentional (specs can live outside the repo), and since this is a local CLI on the user's own machine there's no security concern — just flagging that the two helpers have meaningfully different scopes.

8. Spec artifact in tree. specs/2026-06-20-first-run-onboarding-workflow-routing.md is included "for traceability." Fine if that's the repo convention, but confirm it's not intermediate review notes that should be excluded.

Verified

  • getAgentDirs() correctly returns only [primary] when ATOMIC_CODING_AGENT_DIR is set (config.ts:349), so the isolation fix does prevent home-global goal/ralph shadowing — and the test asserts exactly that.
  • vitest usage in the new tests matches the established convention for packages/coding-agent (the upstream pi fork), so no bun:test rule violation there.
  • First-timer/returning-user/changelog ordering in init is correct (hadLastChangelogVersionAtStartup captured before getChangelogForDisplay mutates lastChangelogVersion).

🤖 Generated with Claude Code

@flora131
flora131 merged commit d9b1d0c into main Jun 25, 2026
11 checks passed
lavaman131 pushed a commit that referenced this pull request Jun 29, 2026
* feat(onboarding): route first-run work to workflows

Assistant-model: GPT-5.5

* fix(onboarding): preserve isolated workflow setup

* feat(onboarding): update first-run workflow handoff

Add first-run onboarding behavior that stashes pre-login seeds, hands ready seeds to normal workflow routing, and documents the updated flow.

Assistant-model: GPT-5.5

* refactor(onboarding): hand first-run tasks to normal session

* feat(onboarding): improve first-run routing and file mentions

Raise onboarding handoff routing to high reasoning when supported, expand scope-routing guidance, and keep docs/changelog aligned.

Fix @ file-reference autocomplete during first-run onboarding by falling back to synchronous path completion before fd readiness.

Assistant-model: GPT-5.5

* feat(workflows): clarify first-run run controls

Assistant-model: GPT-5.5

* fix(onboarding): detect returning users from startup state

Assistant-model: GPT-5.5

* fix(onboarding): clear pending seed on new session

Assistant-model: GPT-5.5

* fix(onboarding): resume pending first-run seeds

Assistant-model: GPT-5.5

* fix(workflows): simplify onboarding dispatch confirmation

Assistant-model: GPT-5.5
@flora131
flora131 deleted the flora131/feature/update-atomic-onboarding branch August 14, 2026 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant