Skip to content

feat(cli): setup overhaul + archon doctor + complete bundled skill#1566

Merged
Wirasm merged 5 commits intodevfrom
archon/task-fix-issue-1494
May 4, 2026
Merged

feat(cli): setup overhaul + archon doctor + complete bundled skill#1566
Wirasm merged 5 commits intodevfrom
archon/task-fix-issue-1494

Conversation

@Wirasm
Copy link
Copy Markdown
Collaborator

@Wirasm Wirasm commented May 4, 2026

Summary

  • Problem: bundled-skill.ts only embeds 18 of 21 skill files; no CI check catches regressions. The setup wizard presents a database prompt (irrelevant — SQLite only), Discord (community-gated), and all platform adapters as mandatory, inflating setup from ~2 required steps to 8+. No archon doctor command exists.
  • Why it matters: Binary users silently get an incomplete skill. New users abandon setup before finishing because the flow looks overwhelming. There's no canonical "is my setup healthy?" answer after setup.
  • What changed: Fixed bundled-skill.ts to embed all 21 files + added check:bundled-skill CI guard. Rewrote archon setup to remove DB step, remove Discord, make each adapter skippable with explicit "Archon works without this" messaging, validate Claude binary via spawn test, add gh auth status check for GitHub, add security note for Telegram, and show update instructions + doctor offer at the end. Added archon doctor command with a green/red checklist.
  • What did not change: Docs reframe (README, installation.md, overview.md) is scoped to a separate PR per the issue. PostgreSQL setup wizard, GitHub App support, and per-platform webhook automation are explicitly out of scope.

UX Journey

Before

archon setup
│
├─ Step 1: AI provider (Claude / Codex)           ← no spawn validation
├─ Step 2: Database (SQLite vs Postgres)           ← irrelevant, confusing
├─ Step 3: Platform multiselect (GitHub/Telegram/Slack/Discord)
│          └─ Feels mandatory; no "skip = OK" messaging
├─ Step 4: Per-platform credential collection
│          └─ Telegram: no security warning on empty allowlist
│          └─ GitHub: no gh auth status check
├─ Step 5: Skill copy (optional)
└─ outro "Setup complete!"

PAIN POINTS:
- CLAUDE_BIN_PATH saved unverified → fails silently at first workflow run
- Telegram bot is wide open to the public if user leaves allowlist empty
- No "did this work?" answer; no update path shown
- Binary users: bundled-skill.ts missing 3 files (good-practices.md,
  parameter-matrix.md, troubleshooting.md)
- No archon doctor command

After

archon setup
│
├─ Step 1: AI provider — validates binary via spawn test before saving
├─ Step 2: GitHub (skippable) — runs gh auth status, offers gh auth login
├─ Step 3: Slack (skippable) — "Archon works as CLI + skill without this"
├─ Step 4: Telegram (skippable) — security note + allowlist warning if empty
├─ Step 5: Skill copy (skippable)
├─ Step 6: [Update Instructions note — always shown]
├─ Step 7: Offer to run archon doctor
└─ outro "Setup complete!"

archon doctor  (new command, re-runnable at any time)
├─ ✓ Claude binary: ~/.local/bin/claude (spawns OK)
├─ ✓ gh CLI: authenticated (or ○ skipped — no GITHUB_TOKEN)
├─ ✓ Database: reachable (SQLite at ~/.archon/archon.db)
├─ ✓ Workspace: ~/.archon/ is writable
├─ ✓ Bundled defaults: workflows + commands loaded
└─ ○ Slack/Telegram: no tokens set (skipped)

bundled-skill.ts: 21/21 files embedded; CI check prevents regression

Architecture Diagram

Before

packages/cli/src/
├─ bundled-skill.ts        (18/21 files — missing 3)
├─ commands/
│   └─ setup.ts            (wizard with DB step + Discord + no spawn validation)
└─ cli.ts                  (no doctor case)

scripts/
└─ generate-bundled-defaults.ts   (check:bundled guard for defaults)
    (no equivalent for bundled-skill)

After

packages/cli/src/
├─ bundled-skill.ts        [~] (21/21 files)
├─ commands/
│   ├─ setup.ts            [~] (no DB, no Discord, spawn-validate, gh auth, Telegram note, doctor offer)
│   ├─ doctor.ts           [+] (new archon doctor command)
│   └─ doctor.test.ts      [+] (tests)
└─ cli.ts                  [~] (doctor case registered, noGitCommands updated)

scripts/
├─ generate-bundled-defaults.ts   (unchanged)
└─ check-bundled-skill.ts  [+] (mirrors check:bundled pattern for bundled-skill.ts)

package.json (root)        [~] (check:bundled-skill added, validate updated)
packages/cli/package.json  [~] (doctor.test.ts added to test batch)

Connection inventory:

From To Status Notes
cli.ts commands/doctor.ts new Dynamic import case 'doctor':
commands/setup.ts commands/doctor.ts new Dynamic import for doctor offer at wizard end
commands/doctor.ts @archon/paths new BUNDLED_IS_BINARY, getArchonHome
commands/doctor.ts @archon/git new execFileAsync for binary + gh probes
commands/doctor.ts @archon/core/db new Lazy import for DB reachability check
commands/setup.ts @archon/git new execFileAsync for spawn test + gh auth
scripts/check-bundled-skill.ts .claude/skills/archon/ new Reads skill files to verify coverage
package.json validate script check-bundled-skill new Added to pipeline

Label Snapshot

  • Risk: risk: low
  • Size: size: M
  • Scope: cli
  • Module: cli:setup, cli:doctor, cli:bundled-skill

Change Metadata

  • Change type: feature
  • Primary scope: cli

Linked Issue

Validation Evidence (required)

bun run validate

All five checks passed (EXIT=0):

Check Result
bun run check:bundled ✅ 36 commands, 20 workflows up to date
bun run check:bundled-skill ✅ 21 files up to date
bun run type-check (all 10 packages) ✅ no errors
bun run lint --max-warnings 0 ✅ 0 errors, 0 warnings
bun run format:check ✅ all files formatted
bun run test (all packages) ✅ 131 CLI tests pass, 0 failures

Smoke test:

bun --cwd packages/cli src/cli.ts doctor
# Output: 5 ✓ checks, 4 ○ skips, 0 ✗ failures
  • Evidence provided: full bun run validate run with exit 0; smoke test of archon doctor in dev mode
  • No commands skipped

Security Impact (required)

  • New permissions/capabilities? Yesarchon doctor makes optional network probes (Slack auth.test, Telegram getMe) using existing configured tokens. No new token collection.
  • New external network calls? Yesarchon doctor optionally pings Slack/Telegram APIs (best-effort, degrade to skip on failure; only run when tokens are already in env)
  • Secrets/tokens handling changed? No — tokens read from env only, never logged
  • File system access scope changed? No — doctor writes a temp file in ARCHON_HOME then immediately deletes it (writable check)
  • Mitigation: All adapter pings are guarded by Promise.allSettled — network failures produce skip not fail. gh auth login is TTY-gated (process.stdout.isTTY) to prevent hangs in CI.

Compatibility / Migration

  • Backward compatible? Yes — wizard changes are purely UX (fewer prompts, not different behavior). Existing .env files are untouched.
  • Config/env changes? No — no new env vars required. check:bundled-skill is a new script but not user-facing config.
  • Database migration needed? No

Human Verification (required)

  • Verified scenarios: bun run cli doctor in dev context (5 ✓ checks, 4 ○ skips); full bun run validate suite passing; type-check across all 10 packages
  • Edge cases checked: BUNDLED_IS_BINARY = false → Claude check skips cleanly; GITHUB_TOKEN not set → gh auth check skips; empty Telegram allowlist path in setup confirmed to show warning via code review
  • What was not verified: interactive archon setup end-to-end in a fresh install (requires a real binary build); gh auth login interactive flow (TTY-gated code path)

Side Effects / Blast Radius (required)

  • Affected subsystems: @archon/cli (setup, doctor, bundled-skill), root package.json, scripts/
  • Potential unintended effects: bun run validate now takes slightly longer due to check:bundled-skill step (negligible — file scan only). Removing collectDatabaseConfig from setup means any code path that relied on SetupConfig.database will fail to compile — all such paths were updated.
  • Guardrails: check:bundled-skill in CI catches any future bundled-skill.ts drift immediately

Rollback Plan (required)

  • Fast rollback: git revert b1c49a53 — reverts all changes in this PR atomically
  • Feature flags: none — no feature flags used
  • Observable failure symptoms: archon setup showing database prompt or Discord = rollback needed; check:bundled-skill exiting 2 = bundled-skill.ts out of sync

Risks and Mitigations

  • Risk: setup.ts type errors after removing database from SetupConfig cascade to generateEnvContent and checkExistingConfig

    • Mitigation: All downstream references updated; bun run type-check passes clean across all 10 packages
  • Risk: spawnSync('gh', ['auth', 'login']) hangs in non-TTY environments (CI, piped shells)

    • Mitigation: Offer gated on process.stdout.isTTY — the gh auth login call is never reached in non-TTY contexts
  • Risk: Adapter pings (Slack, Telegram) fail in environments with restricted outbound network access

    • Mitigation: All pings use Promise.allSettled; network errors produce skip not fail in doctor output

Summary by CodeRabbit

  • New Features

    • Added archon doctor command to diagnose environment setup (Claude binary, authentication, database, adapters).
    • Enhanced setup wizard with binary validation and GitHub authentication checks.
    • Setup wizard now auto-generates project configuration when missing.
    • Expanded bundled resources with additional reference guides.
  • Documentation

    • Updated CLI documentation with doctor command details.
    • Updated troubleshooting guide with diagnostic reference.

…1494)

Make binary installs the official primary path. Setup wizard becomes
a dead-simple "AI + skippable adapters" flow, the embedded skill ships
all 21 files, and `archon doctor` provides a canonical "is my setup OK?"
checklist.

Changes:
- bundled-skill.ts: embed all 21 .claude/skills/archon/ files (was 18,
  missing good-practices.md, parameter-matrix.md, troubleshooting.md)
- scripts/check-bundled-skill.ts: CI guard that fails when bundled-skill.ts
  drifts from .claude/skills/archon/; wired into `bun run validate`
- setup.ts: remove database prompt (SQLite implicit), remove Discord,
  validate Claude binary via spawn test, probe `gh auth status` and
  optionally run `gh auth login`, add Telegram security note + empty-
  allowlist warning, append update instructions and offer to run
  `archon doctor` at the end
- doctor.ts: new `archon doctor` command — green/red checklist for
  Claude binary, gh auth, database, workspace writability, bundled
  defaults, and adapter token pings (best-effort)
- cli.ts: register `doctor` and add to noGitCommands
- doctor.test.ts: spyOn-based tests for individual check functions
- setup.test.ts: drop SetupConfig.database and platforms.discord
  references, anchor DATABASE_URL assertion to line start

Closes #1494
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8f26d9c0-fbf8-4927-bd29-7b15fdb89820

📥 Commits

Reviewing files that changed from the base of the PR and between 5593498 and 9c6d4c3.

📒 Files selected for processing (13)
  • CLAUDE.md
  • package.json
  • packages/cli/package.json
  • packages/cli/src/bundled-skill.ts
  • packages/cli/src/cli.ts
  • packages/cli/src/commands/doctor.test.ts
  • packages/cli/src/commands/doctor.ts
  • packages/cli/src/commands/setup.test.ts
  • packages/cli/src/commands/setup.ts
  • packages/docs-web/src/content/docs/getting-started/overview.md
  • packages/docs-web/src/content/docs/reference/cli.md
  • packages/docs-web/src/content/docs/reference/troubleshooting.md
  • scripts/check-bundled-skill.ts

📝 Walkthrough

Walkthrough

This PR implements the setup overhaul from issue #1494, introducing archon doctor for end-to-end setup verification, simplifying the setup wizard (removing database/Discord steps, adding Claude binary spawn validation and gh auth checks), embedding three new reference files in the bundled skill, adding a CI safety script to verify bundled files, and updating documentation and tests accordingly.

Changes

Setup Overhaul & Doctor Command

Layer / File(s) Summary
Core Doctor Command
packages/cli/src/commands/doctor.ts
Implements eight verification checks: Claude binary spawn test, gh auth status, database reachability, workspace directory writability, bundled defaults load, Slack/Telegram token auth pings (best-effort). Uses Promise.allSettled to continue after failures; renders each check with pass/fail/skip icons; returns exit code 0 (all pass) or 1 (any fail).
Doctor Tests
packages/cli/src/commands/doctor.test.ts
Comprehensive test suite covering all eight check functions with mocked execFileAsync and fetch; validates pass/fail/skip outcomes, timeout behavior, error message content, and temporary workspace cleanup. Final doctorCommand tests verify exit codes and parallel execution completeness via console.log spy.
Setup Wizard Refactor
packages/cli/src/commands/setup.ts
Removes database and Discord steps (SQLite-only, GitHub/Telegram/Slack only). Adds probeClaudeBinarySpawns to validate binaries before save. Adds gh auth status check with optional TTY login flow. Improves Telegram allowlist messaging (no mandatory trap). Introduces bootstrapProjectConfig to create .archon/config.yaml for skill installation. Optionally invokes archon doctor at end.
Setup Tests
packages/cli/src/commands/setup.test.ts
Adds bootstrapProjectConfig test suite (creation, idempotency, unwritable-path handling). Refines checkExistingConfig to drop database/Discord assertions. Strengthens generateEnvContent SQLite invariant. Simplifies test inputs by removing redundant database/Discord fields while keeping same assertions.
CLI Wiring
packages/cli/src/cli.ts
Imports doctorCommand from ./commands/doctor. Adds doctor to printUsage() output and noGitCommands list. Adds case 'doctor': branch returning await doctorCommand().
Bundled Skill Files
packages/cli/src/bundled-skill.ts
Adds three new reference files to BUNDLED_SKILL_FILES: references/good-practices.md, references/parameter-matrix.md, references/troubleshooting.md. Updates file count header from 18 to 21.
Bundled Skill Validator
scripts/check-bundled-skill.ts
New Bun script that recursively walks .claude/skills/archon/ and verifies each file exists as a substring in packages/cli/src/bundled-skill.ts. Exits with code 2 (CI) or 1 (default) when files are missing; logs "up to date" on success.
CI Integration
package.json, packages/cli/package.json, CLAUDE.md
Adds check:bundled-skill script to root package.json; integrates into validate pipeline. Updates CLI test script to include doctor.test.ts. Updates CLAUDE.md to document new validation step and bun run cli doctor command.
Documentation
packages/docs-web/src/content/docs/getting-started/overview.md, packages/docs-web/src/content/docs/reference/cli.md, packages/docs-web/src/content/docs/reference/troubleshooting.md
Adds archon doctor command to CLI reference table with description. Documents doctor command purpose, usage, exit-code behavior, and adapter pings. Updates troubleshooting guide to recommend running archon doctor after setup to verify Claude binary spawns.

Sequence Diagram

sequenceDiagram
    actor User
    participant CLI as archon CLI
    participant Doctor as doctorCommand()
    participant Checks as Check Functions
    participant Claude as Claude Binary
    participant Git as gh CLI
    participant DB as Database
    participant FS as Filesystem
    participant Slack as Slack API
    participant Telegram as Telegram API

    User ->> CLI: archon doctor
    CLI ->> Doctor: dispatch to doctorCommand()
    
    Doctor ->> Checks: start Promise.allSettled([<br/>  checkClaudeBinary,<br/>  checkGhAuth,<br/>  checkDatabase,<br/>  checkWorkspaceWritable,<br/>  checkBundledDefaults,<br/>  checkSlack,<br/>  checkTelegram<br/>])
    
    par Parallel Check Execution
        Checks ->> Claude: execute --version (5s timeout)
        Claude -->> Checks: pass/fail result
    and
        Checks ->> Git: gh auth status (10s timeout)
        Git -->> Checks: pass/fail/skip result
    and
        Checks ->> DB: SELECT 1 query
        DB -->> Checks: pass/fail result
    and
        Checks ->> FS: write/delete temp file
        FS -->> Checks: pass/fail result
    and
        Checks ->> Slack: POST auth.test (5s timeout)
        Slack -->> Checks: pass/fail/skip result
    and
        Checks ->> Telegram: POST getMe (5s timeout)
        Telegram -->> Checks: pass/fail/skip result
    end
    
    Checks -->> Doctor: [CheckResult, CheckResult, ...]
    Doctor ->> Doctor: render all checks (✓/✗/○ icons)
    Doctor ->> Doctor: count failures + rejected promises
    Doctor ->> User: print summary + exit code
    User ->> User: exit 0 (pass) or 1 (fail)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 Hops with glee
Setup now simple—no database dance,
Doctor checks all at a glance!
Claude spawns, GitHub auth shines bright,
Bundled skills make binary right! 🎉
Archon's ready—you're all set tonight! 🌙

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch archon/task-fix-issue-1494

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@Wirasm
Copy link
Copy Markdown
Collaborator Author

Wirasm commented May 4, 2026

Comprehensive PR Review

PR: #1566 — feat(cli): setup overhaul + archon doctor + complete bundled skill
Reviewed by: 5 specialized agents (code-review, error-handling, test-coverage, comment-quality, docs-impact)
Date: 2026-05-04


Summary

Clean delivery of archon doctor, simplified archon setup (DB prompt and Discord removed), check:bundled-skill CI guard, and completed skill bundle. The Promise.allSettled + CheckResult pattern, network timeouts on every external call, and spyOn-only test isolation are all well-executed. Main concerns are two silent-failure paths in the GitHub auth flow of setup.ts, a misleading writability probe in doctor.ts, misplaced JSDoc blocks, and three stale entries in CLAUDE.md/docs-web.

Verdict: REQUEST_CHANGES

Severity Count
🔴 CRITICAL 0
🟠 HIGH 3
🟡 MEDIUM 4
🟢 LOW 6

🟠 High Issues (Should Fix Before Merge)

H1: Orphaned JSDoc Blocks — collectClaudeBinaryPath Lost Its Docs

📍 packages/cli/src/commands/setup.ts:398-421

Inserting probeClaudeBinarySpawns between an existing JSDoc and collectClaudeBinaryPath left three stacked JSDoc blocks with only the last attached to a function. collectClaudeBinaryPath now has no JSDoc (losing the important "Returns undefined if the user declines" contract note), and the "Collect Claude authentication method" block is orphaned before probeClaudeBinarySpawns.

View fix

Move probeClaudeBinarySpawns (with its own JSDoc) to before the collectClaudeBinaryPath JSDoc:

/**
 * Try to spawn the Claude binary with `--version` to confirm it actually runs.
 * Returns true on success, false on any spawn or non-zero exit. Bounded to 5s
 * so a hung process can't stall setup.
 */
async function probeClaudeBinarySpawns(path: string): Promise<boolean> { ... }

/**
 * Resolve the Claude Code executable path for CLAUDE_BIN_PATH.
 * Auto-detects common install locations and falls back to prompting the user.
 * Returns undefined if the user declines to configure (setup continues; the
 * compiled binary will error with clear instructions on first Claude query).
 */
async function collectClaudeBinaryPath(): Promise<string | undefined> { ... }

H2: CLAUDE.md validate Description Says "Five" — Now Runs Six Checks

📍 CLAUDE.md:155

The validate script now runs check:bundled, check:bundled-skill, type-check, lint, format check, and tests. CLAUDE.md still says "All five must pass."

View fix
This runs `check:bundled`, `check:bundled-skill`, type-check, lint, format check, and tests. All six must pass for CI to succeed.

H3: CLAUDE.md CLI Section Missing archon doctor

📍 CLAUDE.md (CLI section)

archon doctor is a user-facing, re-runnable health check command. The CLI reference lists every other subcommand but omits it.

View fix

Insert before the # Show version block in the CLI section:

# Verify your Archon setup (Claude binary, gh auth, DB, adapters)
bun run cli doctor

🟡 Medium Issues (Consider Fixing)

M1: spawnSync('gh', ...) Return Not Checked — Silent Failure If gh Not Installed

📍 packages/cli/src/commands/setup.ts:933

When the user confirms "Run gh auth login now?", spawnSync return is never inspected. If gh is not installed (ENOENT), spawnSync returns { error: Error('ENOENT') } — but with stdio: 'inherit' nothing reaches the terminal. The user says yes, nothing happens, they don't know why.

View fix
const ghLoginResult = spawnSync('gh', ['auth', 'login'], { stdio: 'inherit' });
if (ghLoginResult.error) {
  log.warning(
    `Could not run gh auth login: ${ghLoginResult.error.message}. ` +
      'Install the gh CLI from https://cli.github.com/ and run it manually.'
  );
}

M2: checkWorkspaceWritable Reports "Not Writable" Even When Write Succeeded

📍 packages/cli/src/commands/doctor.ts:100-108

Write and rmSync cleanup share the same try block. If rmSync throws (e.g. EPERM on Windows), the catch reports "${home} not writable" — but the write already succeeded. The status is factually wrong, and a stale .doctor-probe-* file is left behind.

View fix
export async function checkWorkspaceWritable(): Promise<CheckResult> {
  const label = 'Workspace';
  const home = getArchonHome();
  const probe = join(home, `.doctor-probe-${process.pid}-${Date.now()}`);
  try {
    mkdirSync(home, { recursive: true });
    writeFileSync(probe, 'ok');
  } catch (err) {
    return { label, status: 'fail', message: `${home} not writable: ${(err as Error).message}` };
  }
  try {
    rmSync(probe, { force: true });
  } catch {
    // Deletion failure is cosmetic — the write succeeded, so the dir is writable.
  }
  return { label, status: 'pass', message: `${home} is writable` };
}

M3: Empty Catch Swallows gh Error in Setup — Can't Distinguish "Not Installed" From "Not Logged In"

📍 packages/cli/src/commands/setup.ts:912-918

The empty catch {} in the gh auth probe means if gh is not installed at all, the user sees "gh CLI is not authenticated" — same as when it's installed but not logged in. These require different fixes (brew install gh vs gh auth login).

View fix
let ghAuthError: string | undefined;
try {
  await execFileAsync('gh', ['auth', 'status'], { timeout: 10_000 });
  ghAuthOk = true;
  ghSpin.stop('gh CLI is authenticated');
} catch (err) {
  const e = err as NodeJS.ErrnoException;
  ghAuthError = e.code === 'ENOENT'
    ? 'gh not found in PATH — install it first (https://cli.github.com)'
    : (e.message ?? 'unknown error');
  ghSpin.stop('gh CLI check failed');
}

if (!ghAuthOk) {
  log.warning(
    `gh auth check failed: ${ghAuthError}\n` +
    (ghAuthError?.includes('not found') ? '' : 'Run: gh auth login')
  );
}

M4: docs-web CLI Reference Missing doctor Command

📍 packages/docs-web/src/content/docs/reference/cli.md

reference/cli.md documents every CLI command but has no entry for doctor. Can be folded into the docs-reframe follow-up PR mentioned in the PR description.

View suggested section

Add after ### setup:

### `doctor`

Verify your Archon setup. Runs a checklist: Claude binary spawn, gh CLI auth, database reachability, workspace writability, bundled defaults, and adapter token pings (Slack/Telegram, best-effort).

```bash
archon doctor

Exit code 0 if all checks pass or skip; 1 if any critical check fails. Adapter pings degrade to skip on network errors.


</details>

---

## 🟢 Low Issues

<details>
<summary>View 6 low-priority suggestions</summary>

| Issue | Location | Suggestion |
|-------|----------|------------|
| `check-bundled-skill.ts` substring match passes comment-only references | `scripts/check-bundled-skill.ts:36` | Add comment documenting the limitation; acceptable as-is |
| `doctorCommand` rejection handler prints `undefined` for non-Error rejections | `doctor.ts:207-211` | `s.reason instanceof Error ? s.reason.message : String(s.reason)` |
| `checkSlack`/`checkTelegram` label JSON parse failures as "network error" | `doctor.ts:144-151, 173-178` | Change to `ping skipped (${err.message})` — drop "network error:" prefix |
| Test module comment implies spy on `BUNDLED_IS_BINARY` | `doctor.test.ts:1-8` | Clarify: BUNDLED_IS_BINARY is not spied — it's false in dev builds by default |
| "Skip silently" contradicts visible terminal output | `doctor.ts:66-67` | Change "Skip silently" → "Skip" |
| `checkBundledDefaults` pass/fail paths untested | `doctor.ts:84-125` | Can be tested directly in dev mode (no mocks needed); rating 6/10 |

</details>

---

## ✅ What's Good

- **`Promise.allSettled` + `CheckResult` pattern**: Each check returns a result rather than throwing; `allSettled` provides belt-and-braces protection even though checks already catch their own errors. Well-reasoned and documented.
- **Network timeouts on every external call**: `execFileAsync` gets `timeout: 5000`/`10_000`, fetch gets `AbortSignal.timeout(5000)`. No unbounded waits.
- **Network errors on adapter pings intentionally degrade to `skip`** with explicit comments — correctly documented fallback behavior.
- **`BUNDLED_IS_BINARY` guard in `checkClaudeBinary`** cleanly short-circuits in dev builds, preventing false failures.
- **`spyOn`-only test isolation** in `doctor.test.ts` — avoids `mock.module()` entirely, exactly per CLAUDE.md's Bun mock isolation rules.
- **`checkGhAuth` tests cover all three branches** (skip, pass, fail) — the most critical check is the most thoroughly tested.
- **Discord removal from setup wizard** is clean and consistent across all affected code paths.
- **`check:bundled-skill` in validate pipeline** closes a real CI gap for the hand-maintained `bundled-skill.ts`.
- **Telegram security note** surfaced as a separate note rather than buried in setup instructions — clear UX improvement.
- **`doctor.ts` module docblock** is exemplary: covers what's checked, exit-code contract, network-degradation behavior, and setup integration.

---

## 📋 Suggested Follow-up Issues

| Issue Title | Priority |
|-------------|----------|
| Add `archon doctor` to docs-web CLI reference | P2 |
| Improve `checkSlack`/`checkTelegram` test coverage (pass/fail paths) | P3 |

---

*Reviewed by Archon comprehensive-pr-review workflow*
*Full artifacts: `/Users/rasmus/.archon/workspaces/coleam00/Archon/artifacts/runs/5cb3c4395ee03a7bc2f90fcff546f3e4/review/`*

- Fix checkWorkspaceWritable false negative: separate try/catch for rmSync
  so a cleanup failure never reports the directory as unwritable
- Fix collectGitHubConfig empty catch: distinguish gh-not-installed (ENOENT)
  from gh-not-authenticated with actionable error messages
- Check spawnSync('gh', ['auth', 'login']) return value and warn if spawn fails
- Fix three stacked JSDoc blocks in setup.ts: restore orphaned JSDoc to
  collectClaudeBinaryPath and collectClaudeAuth, keep only correct block
  above probeClaudeBinarySpawns
- Fix doctorCommand allSettled rejection handler: use String(s.reason) for
  non-Error rejections instead of producing "undefined" output; add log.error
- Fix checkSlack/checkTelegram catch label: drop "network error: " prefix
  so JSON parse failures aren't mischaracterized
- Fix "Skip silently" wording in doctor.ts: visible output contradicted "silently"
- Fix doctor.test.ts module comment: BUNDLED_IS_BINARY is not spied, clarify
- Add checkBundledDefaults test: passes in dev mode without mocks
- Update CLAUDE.md: fix validate description "five" → "six" checks, add
  check:bundled-skill; add archon doctor to CLI section
- Update docs-web cli.md: add doctor command section after setup
- Add comment to check-bundled-skill.ts documenting substring check limitation
@Wirasm
Copy link
Copy Markdown
Collaborator Author

Wirasm commented May 4, 2026

⚡ Self-Fix Report (Aggressive)

Status: COMPLETE
Pushed: ✅ Changes pushed to archon/task-fix-issue-1494
Commit: 63b798d
Philosophy: Fix everything unless clearly a new concern


Fixes Applied (13 total)

Severity Count
🟠 HIGH 3
🟡 MEDIUM 4
🟢 LOW 6
View all fixes

HIGH

  • Orphaned JSDoc blocks (setup.ts:398-421) — Removed 3 stacked blocks; restored correct JSDoc above collectClaudeBinaryPath and collectClaudeAuth
  • CLAUDE.md validate "five" → "six" (CLAUDE.md:155) — Updated count and added check:bundled-skill to description
  • CLAUDE.md missing archon doctor (CLI section) — Added bun run cli doctor example

MEDIUM

  • spawnSync return not checked (setup.ts:933) — Check ghLoginResult.error, warn with install instructions on failure
  • checkWorkspaceWritable false negative (doctor.ts:100-108) — Separated rmSync into its own try/catch; cleanup failure no longer flips status to fail
  • Empty catch swallows gh error reason (setup.ts:912-918) — Check ENOENT to distinguish "gh not installed" from "gh not authenticated"; distinct messages
  • docs-web CLI reference missing doctor (reference/cli.md) — Added ### doctor section after ### setup

LOW

  • doctorCommand rejection handler undefined output (doctor.ts:207-211) — Use String(s.reason) fallback + log.error for debug trail
  • "network error" label for JSON parse failures (doctor.ts:144-178) — Dropped "network error: " prefix from Slack/Telegram catch message
  • "Skip silently" wording (doctor.ts:66-67) — Changed to "Skip"
  • Test module comment BUNDLED_IS_BINARY inaccuracy (doctor.test.ts:1-8) — Clarified that it's not spied; false in dev builds by default
  • check-bundled-skill.ts substring check undocumented (scripts/check-bundled-skill.ts:36) — Added NOTE comment documenting the limitation
  • checkBundledDefaults has no test (doctor.ts:111-125) — Added test; passes in dev mode without mocks

Tests Added

  • checkBundledDefaults: returns pass with workflow and command counts in dev mode in doctor.test.ts

Skipped (0)

(none — all findings addressed)


Validation

✅ Type check | ✅ Lint | ✅ Tests (9 passed in doctor.test.ts, 0 failures across full suite)


Self-fix · aggressive mode · fixes pushed to archon/task-fix-issue-1494

- Remove 12-line file JSDoc that restated the obvious; keep only the
  non-obvious setup-invocation note
- Drop existsSync() guard before execFileAsync() — TOCTOU-prone and
  redundant since the catch block already produces a clear error
- Collapse verbose allSettled comment to one line
…stall

`archon setup` previously copied the Claude skill into the user's project
but did not create a `.archon/` directory or a project config there. A
fresh user who wanted any project-scoped override (per-project assistant
defaults, custom docs path, etc.) had to `mkdir -p .archon` and create
config.yaml by hand — exactly the friction the v0.3.11 setup overhaul
was meant to remove.

Add `bootstrapProjectConfig(projectPath)` and call it after the skill
install. Writes a commented-out starter `.archon/config.yaml` with
example keys and a link to the configuration reference. Idempotent on
re-run (skips if the file already exists). Workflows/commands/scripts
subdirectories are intentionally not created — empty dirs would clutter
users' trees and the loaders handle their absence cleanly.

The wizard's final summary now shows the created config path so the
user knows where to put per-project overrides.

Tests:
- creates `.archon/config.yaml` when missing
- creates the `.archon` directory if absent
- is idempotent — leaves an existing user config untouched
- returns failed state without throwing on unwritable target
@Wirasm
Copy link
Copy Markdown
Collaborator Author

Wirasm commented May 4, 2026

PR Review Summary — multi-agent (#1566)

Six specialized agents reviewed the diff (code, docs, tests, errors, types, comments). Findings below.

Critical Issues

None — no data-loss or unsafe-fallback issues.

Important Issues (10 found)

Agent Issue Location
silent-failure-hunter gh auth login spawnSync checks .error (spawn failure) but not .status (process exit code). User cancels OAuth → setup prints "Configuration Complete" with GitHub listed as configured even though auth failed. packages/cli/src/commands/setup.ts:1071-1079
silent-failure-hunter probeClaudeBinarySpawns bare catch {} discards error reason. Caller warns "Could not spawn — saving anyway" with no hint whether it was ENOENT, exec format, perms, or timeout — and saves the path regardless. packages/cli/src/commands/setup.ts:965-972
silent-failure-hunter checkDatabase single broad catch wraps both import('@archon/core') and pool.query. Module-load failure surfaces as "Database not reachable: Cannot find module …" — misleads user into "Run archon setup" when fix is a binary rebuild. packages/cli/src/commands/doctor.ts:357-367
silent-failure-hunter checkWorkspaceWritable empty catch {} on rmSync. Repeated doctor runs can leave .doctor-probe-* files with no diagnostic trace; needs a warn log. packages/cli/src/commands/doctor.ts:380-384
code-reviewer bootstrapProjectConfig does existsSyncwriteFileSync without { flag: 'wx' }. TOCTOU race can silently overwrite user config (concurrent setup runs, manual edit during prompt). Use exclusive-create + EEXIST handling. packages/cli/src/commands/setup.ts (bootstrapProjectConfig)
code-reviewer case 'doctor': uses await import('./commands/doctor') while every other peer command (setup, serve, skill install, validate) is statically imported. No documented justification. Convert to static import for consistency. packages/cli/src/cli.ts:614
pr-test-analyzer checkClaudeBinary binary-mode path entirely untested — only the dev-mode skip is exercised. Spawn timeout/args/error formatting can ship broken. Inject BUNDLED_IS_BINARY or extract a probe helper. packages/cli/src/commands/doctor.ts:310-334
pr-test-analyzer checkDatabase has zero tests — no SQLite/Postgres path, no failure path. Inject pool/getDatabaseType. packages/cli/src/commands/doctor.ts:355-367
pr-test-analyzer checkSlack / checkTelegram only skip-path tested. Pass / fail / network-error→skip branches all uncovered. Use spyOn(globalThis, 'fetch'). packages/cli/src/commands/doctor.ts:403-456
docs-impact cli.md quick-start note "The version, help, chat, setup, serve commands work anywhere." After PR doctor is in the no-git list and missing here. packages/docs-web/src/content/docs/reference/cli.md:53

Suggestions (6 found)

Agent Suggestion Location
code-reviewer Add explicit test for the GH_TOKEN-only path so the `
docs-impact CLAUDE.md bundled-defaults note still says only check:bundled runs in validate; should mention check:bundled-skill too. CLAUDE.md:726
docs-impact overview.md CLI command table omits archon doctor row. packages/docs-web/src/content/docs/getting-started/overview.md:310
docs-impact troubleshooting.md "Claude Code not found" — add note pointing to archon doctor for verification. packages/docs-web/src/content/docs/reference/troubleshooting.md:314
pr-test-analyzer Add assertion that doctorCommand exits non-zero on failures and that Promise.allSettled rejected branch is counted as failure. packages/cli/src/commands/doctor.ts:463-499
type-design-analyzer Collapse platforms.X: boolean + X?: XConfig into platforms.X: XConfig | null. Removes redundant double-guards in generateEnvContent. Pre-existing pattern, not introduced by this PR. packages/cli/src/commands/setup.ts (SetupConfig)

Comment cleanup (4 noise comments)

  • packages/cli/src/commands/setup.ts ~1428 — "Offer to verify with archon doctor…" restates visible code + duplicates file-top docstring contract. Delete.
  • packages/cli/src/commands/setup.ts ~1419 — "Update instructions — always shown…" restates note() directly below. Delete.
  • packages/cli/src/commands/setup.ts ~1338 — "Fresh or update mode - collect everything…" restates branch label. Trim to the parenthetical or delete.
  • packages/cli/src/commands/doctor.test.ts:177-180 — Inline BUNDLED_IS_BINARY comment duplicates the file-top docstring. Delete.

Strengths

  • BootstrapProjectConfigResult discriminated union ('created' | 'existed' | 'failed' with error only on failed) is a clean type and the best new shape in the PR.
  • spyOn-only test strategy (no mock.module()) is correct per project rules; placement in batch 1 alongside setup.test.ts is appropriate.
  • writeScopedEnv has thorough behavioural coverage — merge semantics, force+backup, whitespace preservation, cross-scope isolation.
  • Promise.allSettled in doctorCommand properly logs rejected reasons via structured logger (getLog().error(...)) and prints to console — error reason is preserved.
  • clack required: true workaround (setup.ts:1118-1120) documents a real third-party quirk — exactly the WHY-non-obvious comment the project guidelines call for.
  • database removal is clean — no orphan database?: field, no dead branches in generateEnvContent.
  • gh auth login is correctly TTY-gated against CI hangs.
  • check-bundled-skill.ts mirrors the existing check:bundled pattern cleanly.

Verdict

NEEDS FIXES — no critical blockers, but 10 important issues. The gh auth login exit-code gap and the bootstrapProjectConfig TOCTOU are the highest-leverage fixes (correctness contract violations, not just diagnostics). The doctor test gaps undermine the PR's safety-net story — archon doctor is the user-facing "is my setup OK?" answer; its core branches need coverage before this stops being a draft.

Recommended Actions (in order)

  1. Check ghLoginResult.status !== 0 after spawnSync('gh', ['auth', 'login']) — surface non-zero as a warning instead of declaring success.
  2. Make probeClaudeBinarySpawns return { ok, reason } so the warning includes the spawn-failure reason.
  3. Use writeFileSync(..., { flag: 'wx' }) in bootstrapProjectConfig and catch EEXISTstate: 'existed'.
  4. Split checkDatabase into separate try blocks for the import vs the query so the messages distinguish module-load failure from connectivity failure.
  5. Add a warn log in the checkWorkspaceWritable rmSync catch.
  6. Convert cli.ts doctor case to a static import.
  7. Add tests for checkClaudeBinary binary mode, checkDatabase, and the checkSlack/checkTelegram pass + fail + network-error branches. Add doctorCommand exit-code assertion and Promise.allSettled rejection-path assertion.
  8. Update cli.md:53, CLAUDE.md:726, overview.md:310, troubleshooting.md:314.
  9. Remove the four noise comments.
  10. Optional: collapse platforms boolean/optional pairs into nullable config fields.

Production correctness:
- setup.ts: gh auth login now checks .status !== 0 so a non-zero exit
  (cancelled OAuth, failed callback) is surfaced instead of silently
  succeeding.
- setup.ts: probeClaudeBinarySpawns returns {ok, reason}; the warning
  now includes the actual spawn error (ENOENT/timeout/permissions).
- setup.ts: bootstrapProjectConfig uses writeFileSync flag 'wx' and
  catches EEXIST, eliminating the TOCTOU window between existsSync
  and the write.
- doctor.ts: split checkDatabase into module-load vs query try-catches
  so a missing @archon/core stops masquerading as "Database not
  reachable". Both branches now log structured errors.
- doctor.ts: checkWorkspaceWritable rmSync catch logs warn so repeated
  delete failures leave a diagnostic trace.
- cli.ts: doctor case uses a static import to match every other peer
  command.

Tests:
- checkClaudeBinary covers all four branches via an injected isBinary
  parameter (skip / no-path / spawn-pass / spawn-fail).
- checkDatabase covers sqlite + postgres pass, query failure, and
  module-load failure via injectable loadDeps.
- checkSlack / checkTelegram cover pass / fail / network-error->skip
  via spyOn(globalThis, 'fetch').
- checkGhAuth gains an explicit GH_TOKEN-only branch.
- doctorCommand asserts the exit-code contract (0 on all-pass, 1 on
  any fail, thrown checks counted) and Promise.allSettled non-short-
  circuit, via injectable check thunks.

Docs:
- cli.md: doctor added to the no-git command list.
- CLAUDE.md: validate now mentions check:bundled-skill alongside
  check:bundled.
- overview.md: archon doctor row added to the CLI command table.
- troubleshooting.md: points users at archon doctor after setup.
@Wirasm Wirasm marked this pull request as ready for review May 4, 2026 17:40
@Wirasm Wirasm merged commit 5e61faf into dev May 4, 2026
4 checks passed
prospapledge88 added a commit to prospapledge88/Archon that referenced this pull request May 5, 2026
… 6 (#11)

* docs/skill: general hardening — fix inaccuracies, fill workflow/CLI/env gaps, add good-practices + troubleshooting (coleam00#1363)

* fix(skill/when): document the full `when:` operator set and compound expressions

The skill reference previously stated "operators: ==, != only" which is
materially wrong — the condition evaluator supports ==, !=, <, >, <=, >=
plus && / || compound expressions with && binding tighter than ||, plus
dot-notation JSON field access. An agent authoring a workflow from the
skill would think half the operators don't exist.

Replaces the single-sentence section with a structured reference covering:
- All six comparison operators (string and numeric modes)
- Compound expressions with precedence rules and short-circuit eval
- JSON dot notation semantics and failure modes
- The fail-closed rules in full (invalid expression, non-numeric side,
  missing field, skipped upstream)

Grounded in packages/workflows/src/condition-evaluator.ts.

* feat(skill): document Approval and Cancel node types

Approval and cancel nodes are first-class DAG node types (approval since the
workflow lifecycle work in coleam00#871, cancel as a guarded-exit primitive) but the
skill never described either one. An agent reading the skill and asked to
"add a review gate before implementation" or "stop the workflow if the input
is unsafe" would fall back to bash + exit 1, losing the proper semantics
(cancelled vs. failed, on_reject AI rework, web UI auto-resume).

Approval node coverage (references/workflow-dag.md, SKILL.md):
- Full configuration block with message, capture_response, on_reject
- The interactive: true workflow-level requirement for web UI delivery
- Approve/reject commands across all platforms (CLI, slash, natural
  language) and the capture_response → $node-id.output flow
- Ignored-fields list + the on_reject.prompt AI sub-node exception

Cancel node coverage (references/workflow-dag.md, SKILL.md):
- Single-field schema (cancel: "<reason>")
- Lifecycle: cancelled (not failed); in-flight parallel nodes stopped;
  no DAG auto-resume path
- The "cancel: vs bash-exit-1" decision rule (expected precondition miss
  vs. check itself failing)
- Two canonical patterns — upstream-classification gate, pre-expensive-step
  gate

Validation-rules list updated to enumerate approval/cancel constraints
(message non-empty, on_reject.max_attempts range 1-10, cancel reason
non-empty), plus a forward note that script: joins the mutually-exclusive
set once PR coleam00#1362 lands.

Placement in both files is after the Loop section and before the validation
section, so this commit stays additive with respect to PR coleam00#1362's Script
node insertion between Bash and Loop — rebase is clean.

* feat(skill): document workflow-level fields beyond name/provider/model

The skill's Schema section previously showed only name, description, provider,
and model at the workflow level — which is most of a stub. Agents asked to
"use the 1M-context Claude beta" or "run this under a network sandbox" or
"add a fallback model in case Opus rate-limits" had no way to discover
that any of these fields existed at the workflow level.

Adds a comprehensive Workflow-Level Fields section covering:
- Core: name, description, provider, model, interactive (with explicit
  callout that interactive: true is REQUIRED for approval/loop gates on
  web UI — a common footgun)
- Isolation: worktree.enabled for pin-on/pin-off (the only worktree field
  at workflow level; baseBranch/copyFiles/path/initSubmodules are
  config.yaml only, so a cross-reference points there)
- Claude SDK advanced: effort, thinking, fallbackModel, betas, sandbox,
  with explicit per-node-only exceptions (maxBudgetUsd, systemPrompt)
- Codex-specific: modelReasoningEffort (with note that it's NOT the same
  as Claude's effort — this has confused users), webSearchMode,
  additionalDirectories
- A complete worked example combining sandbox + approval + interactive

All fields cross-referenced against packages/workflows/src/schemas/workflow.ts
and packages/workflows/src/schemas/dag-node.ts.

* feat(skill/loop): document interactive loops and gate_message

Interactive loop nodes pause between iterations for human feedback via
/workflow approve — used by archon-piv-loop and archon-interactive-prd.
The skill's Loop Nodes section previously omitted both interactive: true
and gate_message entirely, so an agent writing a guided-refinement
workflow wouldn't know the feature exists or that gate_message is
required at parse time.

Adds:
- interactive and gate_message rows to the config table (marking
  gate_message as required when interactive: true — enforced by the
  loader's superRefine)
- A dedicated "Interactive Loops" subsection explaining the 6-step
  iterate-pause-approve-resume flow
- Explicit call-out that $LOOP_USER_INPUT populates ONLY on the first
  iteration of a resumed session — easy to miss and a common surprise
- Workflow-level interactive: true requirement for web UI delivery
  (loader warning otherwise) so the full-flow example is complete
- Note that until_bash substitution DOES shell-quote $nodeId.output
  (unlike script bodies) — called out since the audit surfaced this
  inconsistency

* fix(skill/cli): complete the CLI command reference with missing lifecycle commands

The CLI reference previously documented only list, run, cleanup, validate,
complete, version, setup, and chat — missing nearly every workflow
lifecycle command an agent needs to operate a paused, failed, or stuck
run. The interactive-workflows reference assumed these commands existed
without actually documenting them.

Adds full documentation for:
- archon workflow status — show running workflow(s)
- archon workflow approve <run-id> [comment] — resume approval gate
  (also populates $LOOP_USER_INPUT on interactive loops and the gate
  node's output when capture_response: true)
- archon workflow reject <run-id> [reason] — reject gate; cancels or
  triggers on_reject rework depending on node config
- archon workflow cancel <run-id> — terminate running/paused with
  in-flight subprocess kill
- archon workflow abandon <run-id> — mark stuck row cancelled without
  subprocess kill (for orphan-cleanup after server crashes — matches
  the coleam00#1216 precedent)
- archon workflow resume <run-id> [message] — force-resume specific
  run (auto-resume is default; this is for explicit override)
- archon workflow cleanup [days] — disk hygiene for old terminal runs
  (with explicit callout that it does NOT transition 'running' rows,
  a common confusion)
- archon workflow event emit — used inside loop prompts for state
  signalling; documented so agents don't invent their own mechanism
- archon continue <branch> [flags] [msg] — iterative-session entry
  point with --workflow and --no-context flags

Also:
- Adds --allow-env-keys flag to the `workflow run` flag table with
  audit-log context and the env-leak-gate remediation use case
- Adds an "Auto-resume without --resume" note disambiguating when
  --resume is needed vs. when auto-resume handles it
- Adds --include-closed flag to `isolation cleanup`, which was
  previously missing; converts the flag list to a structured table
- Explains the cancel/abandon distinction (live subprocess vs. orphan)

All grounded in packages/cli/src/commands/workflow.ts, continue.ts,
and isolation.ts.

* feat(skill/repo-init): add scripts/ and state/, three-path env model, per-project env injection

The repo-init reference was missing two first-class .archon/ directories
(scripts/ since v0.3.3, state/ since the workflow-state feature) and had
nothing to say about env — the #1 thing a user hits on first-run when
their repo has a .env file with API keys.

Directory tree updates:
- Adds .archon/scripts/ with the extension->runtime rule (.ts/.js -> bun,
  .py -> uv) so agents know where to put named scripts referenced by
  script: nodes.
- Adds .archon/state/ with explicit "always gitignore" callout — these
  are runtime artifacts, not source. Previously undocumented in the skill.
- Adds .archon/.env (repo-scoped Archon env) and distinguishes it from
  the target repo's top-level .env.
- Adds a "What each directory is for" list so the structure isn't just
  a tree with no narrative.

.gitignore guidance:
- state/ and .env added as must-gitignore (state/ matches CLAUDE.md and
  reference/archon-directories.md — skill was lagging).
- mcp/ demoted to conditional — gitignore only if you hardcode secrets.

New "Three-Path Env Model" section:
- ~/.archon/.env (trusted, user), <cwd>/.archon/.env (trusted, repo),
  <cwd>/.env (UNTRUSTED, target project — stripped from subprocess env).
- Precedence (override: true across archon-owned paths) and the
  observable [archon] loaded N keys / stripped K keys log lines so
  operators can verify what actually happened.
- Decision tree for where to put API keys vs. target-project env vs.
  things Archon shouldn't touch.
- Links to archon setup --scope home|project with --force for writing
  to the right file with timestamped backups.

New "Per-Project Env Injection" section:
- Documents both managed surfaces: .archon/config.yaml env: block
  (git-committed, $REF expansion) and Web UI Settings → Projects →
  Env Vars (DB-stored, never returned over API).
- Names every execution surface that receives the injected vars:
  Claude/Codex/Pi subprocess, bash: nodes, script: nodes, and direct
  codebase-scoped chat.
- Documents the env-leak gate with all 5 remediation paths so an agent
  hitting "Cannot register: env has sensitive keys" knows the options.

Grounded in CHANGELOG v0.3.7 (three-path env + setup flags), v0.3.0
(env-leak gate), and reference/security.md on the docs site.

* fix(skill/authoring-commands): correct override paths and add home-scoped commands

The file-location and discovery sections described an override layout that
does not match the actual resolver. It showed:

  .archon/commands/defaults/archon-assist.md  # Overrides the bundled

and claimed `.archon/commands/defaults/` was where repo-level overrides
lived. In fact the resolver (executor-shared.ts:152-200 + command-
validation.ts) walks `.archon/commands/` 1 level deep and uses basename
matching — putting `archon-assist.md` at the top of `.archon/commands/`
is the canonical way to override the bundled version. The `defaults/`
subfolder is a Archon-internal convention for shipping bundled defaults,
not a user-facing override pattern.

Also, home-scoped commands (`~/.archon/commands/`, shipped in v0.3.7)
were completely absent — agents authoring personal helpers wouldn't
know they could live at the user level and be shared across every repo.

Changes:
- File Location section now shows all three discovery scopes (repo,
  home, bundled) with precedence ordering and 1-level subfolder rules
- Duplicate-basename rule documented as a user error surface
- Discovery and Priority section rewritten with accurate 3-step lookup
  order — no more references to the nonexistent defaults/ override path
- Adds the Web UI "Global (~/.archon/commands/)" palette label note so
  users authoring helpers for the builder know what to expect

No code changes — this is a pure fix of stale/incorrect skill reference
material.

* feat(skill): add workflow good-practices and troubleshooting reference pages

Closes two gaps from the audit. The skill previously had zero guidance on
designing multi-node workflows (what to avoid, what to reach for first,
how to structure artifact chains) and zero guidance on where to look
when things go wrong (log paths, env-leak gate remediations, orphan-row
cleanup, resume semantics).

New references/good-practices.md (9 Good Practices + 7 Anti-Patterns):

- Use deterministic nodes (bash:/script:) for deterministic work, AI for
  reasoning — the single biggest quality lever
- output_format required whenever downstream when: reads a field — the
  most common source of "workflow silently routes wrong"
- trigger_rule: none_failed_min_one_success after conditional branches —
  the classic bug where all_success fails because a skipped when:-gated
  branch doesn't count as a success
- context: fresh requires artifacts for state passing — commands must
  explicitly "read $ARTIFACTS_DIR/..." when downstream of fresh
- Cheap models (haiku) for glue, strong for substance
- Workflow descriptions as routing affordances
- Validate (archon validate workflows) + smoke-run before shipping
- Artifact-chain-first design
- worktree.enabled: true for code-changing workflows (reversibility)
- Anti-patterns with before/after YAML examples for each (AI-for-tests,
  free-form when: matching, context: fresh without artifacts, long flat
  AI-node layers, secrets in YAML, retry on loop nodes, tiny
  max_iterations, missing workflow-level interactive:, tool-restricted
  MCP nodes)

New references/troubleshooting.md:

- Log location (~/.archon/workspaces/<owner>/<repo>/logs/<run-id>.jsonl)
  with jq recipes for common queries (last assistant message, failed
  events, full stream)
- Artifact location for cross-node handoff debugging
- 9 Common Failure Modes, each with root cause + concrete fix:
  - $BASE_BRANCH unresolvable
  - Env-leak gate (5 remediations)
  - Claude/Codex binary not found (compiled-binary-only)
  - "running" forever (AI working / orphan / idle_timeout)
  - Mid-workflow failure and auto-resume semantics
  - Approval gate missing on web UI (workflow-level interactive:)
  - MCP plugin connection noise (filtered by design)
  - Empty $nodeId.output / field access (4 causes)
- Diagnostic command cheat sheet (list, status, isolation list, validate,
  tail-log, --verbose, LOG_LEVEL=debug)
- Escalation protocol (version + validate + log tail + CHANGELOG + issue)

SKILL.md routing table now dispatches "Workflow good practices /
anti-patterns" and "Troubleshoot a failing / stuck workflow" to the new
references so an agent can find them without having to know they exist.

* docs(book): update node-types coverage from four to all seven

The book is the curated first-contact reading path (landing page → "Get
Started" → /book/). Both dag-workflows.md and quick-reference.md were
stuck on "four node types" — missing script, approval, and cancel. A user
reading the book as their first introduction would form an incomplete
mental model, then find three more node types in the reference section
later with no explanation of when they arrived.

book/dag-workflows.md:
- "four node types" → "seven node types. Exactly one mode field is
  required per node"
- Table now lists Command, Prompt, Bash, Script, Loop, Approval, Cancel
  with one-line "when to use" for each, and cross-links to the dedicated
  guide pages for Script / Loop / Approval
- New sections below the table for Script (inline + named examples with
  runtime and deps), Approval (with the interactive: true workflow-level
  note that's easy to miss), and Cancel (guarded-exit pattern) — keeping
  the existing narrative shape for Bash and Loop

book/quick-reference.md:
- Node Options table now includes script, approval, cancel rows
- agents row added (inline sub-agents, Claude-only)
- New "Script-specific fields" and "Approval-specific fields" subsections
  so the cheat-sheet is actually complete rather than pointing users
  elsewhere for the required constraints
- Retry row callout that loop nodes hard-error on retry — previously
  omitted
- bash timeout note widened to cover script timeout (same semantics)

Both files are docs-web content; the CI build on the docs-script-nodes
PR (coleam00#1362) previously validated the Starlight build path with a similar
table addition, so this should render clean.

* fix(skill/cli): remove nonexistent \`archon workflow cancel\`, fix workflow status jq recipe

Two accuracy issues from the PR code-reviewer (comment 4311243858).

C1: \`archon workflow cancel <run-id>\` does NOT exist as a CLI subcommand.
The switch at packages/cli/src/cli.ts:318-485 dispatches on list / run /
status / resume / abandon / approve / reject / cleanup / event — running
\`archon workflow cancel\` hits the default case and exits with "Unknown
workflow subcommand: cancel" (cli.ts:478-484). Active cancellation is
only available via:
  - /workflow cancel <run-id> chat slash command (all platforms)
  - Cancel button on the Web UI dashboard
  - POST /api/workflows/runs/{runId}/cancel REST endpoint

cli-commands.md: removed the \`### archon workflow cancel <run-id>\`
subsection; kept the \`abandon\` subsection but made it explicit that
abandon does NOT kill a subprocess. Added a call-out box at the bottom
of the abandon section explaining where to go for actual cancellation.

troubleshooting.md "running forever" section: split the original
cancel-vs-abandon advice into three bullets — Web UI / CLI abandon (for
orphans, no subprocess kill) / chat \`/workflow cancel\` (for live runs
that need interruption). Added an explicit "there is no archon workflow
cancel CLI subcommand" parenthetical since the wrong command was being
suggested in flow.

I1: the \`archon workflow list --json\` diagnostic used an incorrect jq
filter. workflow list's --json output (workflow.ts:185-219) has shape
{ workflows: [{ name, description, provider?, model?, ... }], errors: [...] }
with no \`runs\` field — \`jq '.workflows[] | select(.runs)'\` returns empty
unconditionally. Replaced with \`archon workflow status --json | jq '.runs[]'\`,
which matches the actual shape of workflowStatusCommand at
workflow.ts:852+ ({ runs: WorkflowRun[] }). Also tightened the narration
to distinguish JSON from human-readable status output.

No change to the commit history in this PR — these are follow-up fixes
to claims I introduced in earlier commits of this branch (f10b989 for
C1, 66d2b86 for I1).

* fix(skill): remove env-leak gate references (feature was removed in provider extraction)

C2 from the PR code-reviewer (comment 4311243858). The pre-spawn env-leak
gate was removed from the codebase during the provider-extraction refactor
— see TODO(coleam00#1135) at packages/providers/src/claude/provider.ts:908. Zero
hits for --allow-env-keys / allowEnvKeys / allow_env_keys / allow_target_repo_keys
across packages/. The CLI's parseArgs (cli.ts:182-208) has no
--allow-env-keys option, and because parseArgs uses strict: false, an
unknown --allow-env-keys would be silently ignored rather than error.

What remains accurate and is NOT touched:
- Three-Path Env Model section (user/repo archon-owned envs are loaded;
  target repo <cwd>/.env keys are stripped from process.env at boot)
  still correctly describes current behavior, grounded in
  packages/paths/src/strip-cwd-env.ts + env-integration.test.ts
- Per-Project Env Injection section (Option 1: .archon/config.yaml env:
  block; Option 2: Web UI Settings → Projects → Env Vars) is unchanged —
  both remain the sanctioned way to get env vars into subprocesses

Removed claims (all three files):
- cli-commands.md: --allow-env-keys flag row in the workflow run flags
  table
- repo-init.md: the "Env-leak gate" subsection at the end of Per-Project
  Env Injection listing 5 remediations (all of which reference UI/CLI/
  config surfaces that don't exist). Replaced with a succinct callout
  that explains the actual current behavior — target repo .env keys are
  stripped, workflows that need those values should use managed
  injection — so the reader still gets the "where to put my env vars"
  answer
- troubleshooting.md: the "Cannot register: codebase has sensitive env
  keys" section (error message that can no longer be emitted)

If the env-leak gate is ever resurrected per TODO(coleam00#1135), the docs can be
re-added then. The CHANGELOG v0.3.0 entry describing the gate is a
historical record of past behavior and does not need to be rewritten.

* fix(skill/troubleshooting): correct JSONL event type names and field name

C3 from the PR code-reviewer (comment 4311243858). The troubleshooting
reference's event-types table used _started / _completed / _failed
suffixes, but packages/workflows/src/logger.ts:19-30 shows the actual
WorkflowEvent.type enum is:

  workflow_start | workflow_complete | workflow_error |
  assistant | tool | validation |
  node_start | node_complete | node_skipped | node_error

The second jq recipe also queried `.event` but the discriminator is `.type`.

Fixes:
- Event table: renamed columns (_started → _start, _completed → _complete,
  _failed → _error). Explicitly called out the field name as `type` so the
  reader knows what jq selector to use
- Replaced the "tool_use / tool_result" row with a single `tool` row and
  listed its actual payload fields (tool_name, tool_input, duration_ms,
  tokens) — tool_use/tool_result are SDK message kinds that appear within
  the AI stream, not top-level log event types
- Added a `validation` row (was missing; it's emitted by workflow-level
  validation calls with `check` and `result` fields)
- Removed `retry_attempt` row — this event type is not emitted to the
  JSONL file. Retry bookkeeping goes through pino logs, not the workflow
  log file
- Added an explicit callout that loop_iteration_started /
  loop_iteration_completed (and other emitter-only events) go through
  the workflow event emitter + DB workflow_events table, NOT the JSONL
  file. Pointed readers to the DB or Web UI for loop-level detail. This
  distinguishes the two parallel event systems — easy to conflate
  (store.ts:11-17 uses _started/_completed/_failed for the DB side,
  logger.ts uses _start/_complete/_error for JSONL)
- Fixed the "all failed events" jq recipe: .event → .type and _failed → _error
- Minor cleanup: the inline "tool_use events" mention in the "running
  forever" section said the wrong event name — updated to "tool or
  assistant events in the tail"

Grounded in packages/workflows/src/logger.ts (canonical JSONL event
shape) and packages/workflows/src/store.ts (the parallel DB event
naming, which the reviewer correctly flagged as different and worth
keeping distinct).

* fix(skill): two stragglers from the code-reviewer audit

Cleanup of two references that slipped through the earlier C1 and C3 fixes:

- references/troubleshooting.md:126: \`node_failed\` → \`node_error\`
  (the "Node output is empty" diagnostics section references the JSONL
  log, which uses the logger.ts enum — not the DB workflow_events table
  which does use \`node_failed\`). The C3 fix corrected the event table
  and one jq recipe but missed this inline mention.

- references/interactive-workflows.md:106: removed \`archon workflow
  cancel <run-id>\` (nonexistent CLI subcommand) from the
  troubleshooting bullet. This was pre-existing before the hardening
  PR but fell within the C1 remediation scope. Replaced with the
  correct triage: reject (approval gate only) vs abandon (orphan
  cleanup, no subprocess kill) vs chat /workflow cancel (actual
  subprocess termination).

Grounded in the same sources as the earlier C1/C3 commits:
packages/cli/src/cli.ts:318-485 (no cancel case) and
packages/workflows/src/logger.ts:19-30 (JSONL type enum).

* feat(skill): point to archon.diy as the canonical docs source

The skill had no reference to archon.diy (the live docs site built from
packages/docs-web/). Several reference files said "see the docs site"
without naming the URL, leaving the agent to guess or grep the repo for
the hostname. An agent with the skill loaded should know that when the
distilled reference pages don't cover a case, the full canonical docs
are one WebFetch away.

SKILL.md: new "Richer Context: archon.diy" section between Routing and
Running Workflows. Covers:
- When to reach for the live docs (longer examples, tutorial framing,
  features the skill only mentions in passing, "where's that
  documented?" user questions)
- URL map — 13 starting points covering getting-started, book (tutorial
  series), guides/ (authoring + per-node-type + per-node-feature),
  reference/ (variables, CLI, security, architecture, configuration,
  troubleshooting), adapters/, deployment/
- Precedence: skill refs first (context-cheap, tuned for agents), docs
  site as escalation. Prevents agents defaulting to WebFetch when a
  local skill ref already covers the answer

Also upgrades the 5 existing generic "docs site" mentions across
reference files to concrete archon.diy URLs with anchor fragments where
helpful:
- good-practices.md: Inline sub-agents pattern → archon.diy/guides/
  authoring-workflows/#inline-sub-agents
- troubleshooting.md: "Install page on the docs site" → archon.diy/
  getting-started/installation/
- workflow-dag.md: "Workflow Description Best Practices" → anchor link;
  sandbox schema reference → archon.diy/guides/authoring-workflows/
  #claude-sdk-advanced-options
- repo-init.md: Security Model reference → archon.diy/reference/
  security/#target-repo-env-isolation (deep-link into the section that
  covers the <cwd>/.env strip behavior)

URL source of truth: astro.config.mjs:5 (site: 'https://archon.diy').
URL structure mirrors packages/docs-web/src/content/docs/<section>/
<page>.md — verified by the 62 pages the docs build produces.

* docs/skill: add parameter-matrix.md quick-lookup reference

New reference for the archon skill: a single-glance lookup of which
parameter works on which node type, an intent-based "how do I..." table,
a consolidated silent-failure catalog, and an inline agents: section
(previously only referenced via archon.diy).

Purpose is complementary, not duplicative:
- workflow-dag.md remains the authoring guide
- dag-advanced.md remains the hooks/MCP/skills/retry deep-dive
- good-practices.md remains the patterns and anti-patterns
- parameter-matrix.md is the grep-this-first lookup when you know the
  outcome you want but not which field gets you there

Also registers the new reference in SKILL.md routing table.

* feat(cli): setup overhaul + archon doctor + complete bundled skill (coleam00#1566)

* feat(cli): setup overhaul + archon doctor + complete bundled skill (coleam00#1494)

Make binary installs the official primary path. Setup wizard becomes
a dead-simple "AI + skippable adapters" flow, the embedded skill ships
all 21 files, and `archon doctor` provides a canonical "is my setup OK?"
checklist.

Changes:
- bundled-skill.ts: embed all 21 .claude/skills/archon/ files (was 18,
  missing good-practices.md, parameter-matrix.md, troubleshooting.md)
- scripts/check-bundled-skill.ts: CI guard that fails when bundled-skill.ts
  drifts from .claude/skills/archon/; wired into `bun run validate`
- setup.ts: remove database prompt (SQLite implicit), remove Discord,
  validate Claude binary via spawn test, probe `gh auth status` and
  optionally run `gh auth login`, add Telegram security note + empty-
  allowlist warning, append update instructions and offer to run
  `archon doctor` at the end
- doctor.ts: new `archon doctor` command — green/red checklist for
  Claude binary, gh auth, database, workspace writability, bundled
  defaults, and adapter token pings (best-effort)
- cli.ts: register `doctor` and add to noGitCommands
- doctor.test.ts: spyOn-based tests for individual check functions
- setup.test.ts: drop SetupConfig.database and platforms.discord
  references, anchor DATABASE_URL assertion to line start

Closes coleam00#1494

* fix: address review findings from PR coleam00#1566

- Fix checkWorkspaceWritable false negative: separate try/catch for rmSync
  so a cleanup failure never reports the directory as unwritable
- Fix collectGitHubConfig empty catch: distinguish gh-not-installed (ENOENT)
  from gh-not-authenticated with actionable error messages
- Check spawnSync('gh', ['auth', 'login']) return value and warn if spawn fails
- Fix three stacked JSDoc blocks in setup.ts: restore orphaned JSDoc to
  collectClaudeBinaryPath and collectClaudeAuth, keep only correct block
  above probeClaudeBinarySpawns
- Fix doctorCommand allSettled rejection handler: use String(s.reason) for
  non-Error rejections instead of producing "undefined" output; add log.error
- Fix checkSlack/checkTelegram catch label: drop "network error: " prefix
  so JSON parse failures aren't mischaracterized
- Fix "Skip silently" wording in doctor.ts: visible output contradicted "silently"
- Fix doctor.test.ts module comment: BUNDLED_IS_BINARY is not spied, clarify
- Add checkBundledDefaults test: passes in dev mode without mocks
- Update CLAUDE.md: fix validate description "five" → "six" checks, add
  check:bundled-skill; add archon doctor to CLI section
- Update docs-web cli.md: add doctor command section after setup
- Add comment to check-bundled-skill.ts documenting substring check limitation

* simplify: trim doctor.ts JSDoc and remove redundant existsSync check

- Remove 12-line file JSDoc that restated the obvious; keep only the
  non-obvious setup-invocation note
- Drop existsSync() guard before execFileAsync() — TOCTOU-prone and
  redundant since the catch block already produces a clear error
- Collapse verbose allSettled comment to one line

* feat(setup): bootstrap project-scoped .archon/config.yaml on skill install

`archon setup` previously copied the Claude skill into the user's project
but did not create a `.archon/` directory or a project config there. A
fresh user who wanted any project-scoped override (per-project assistant
defaults, custom docs path, etc.) had to `mkdir -p .archon` and create
config.yaml by hand — exactly the friction the v0.3.11 setup overhaul
was meant to remove.

Add `bootstrapProjectConfig(projectPath)` and call it after the skill
install. Writes a commented-out starter `.archon/config.yaml` with
example keys and a link to the configuration reference. Idempotent on
re-run (skips if the file already exists). Workflows/commands/scripts
subdirectories are intentionally not created — empty dirs would clutter
users' trees and the loaders handle their absence cleanly.

The wizard's final summary now shows the created config path so the
user knows where to put per-project overrides.

Tests:
- creates `.archon/config.yaml` when missing
- creates the `.archon` directory if absent
- is idempotent — leaves an existing user config untouched
- returns failed state without throwing on unwritable target

* fix: address multi-agent review findings from PR coleam00#1566

Production correctness:
- setup.ts: gh auth login now checks .status !== 0 so a non-zero exit
  (cancelled OAuth, failed callback) is surfaced instead of silently
  succeeding.
- setup.ts: probeClaudeBinarySpawns returns {ok, reason}; the warning
  now includes the actual spawn error (ENOENT/timeout/permissions).
- setup.ts: bootstrapProjectConfig uses writeFileSync flag 'wx' and
  catches EEXIST, eliminating the TOCTOU window between existsSync
  and the write.
- doctor.ts: split checkDatabase into module-load vs query try-catches
  so a missing @archon/core stops masquerading as "Database not
  reachable". Both branches now log structured errors.
- doctor.ts: checkWorkspaceWritable rmSync catch logs warn so repeated
  delete failures leave a diagnostic trace.
- cli.ts: doctor case uses a static import to match every other peer
  command.

Tests:
- checkClaudeBinary covers all four branches via an injected isBinary
  parameter (skip / no-path / spawn-pass / spawn-fail).
- checkDatabase covers sqlite + postgres pass, query failure, and
  module-load failure via injectable loadDeps.
- checkSlack / checkTelegram cover pass / fail / network-error->skip
  via spyOn(globalThis, 'fetch').
- checkGhAuth gains an explicit GH_TOKEN-only branch.
- doctorCommand asserts the exit-code contract (0 on all-pass, 1 on
  any fail, thrown checks counted) and Promise.allSettled non-short-
  circuit, via injectable check thunks.

Docs:
- cli.md: doctor added to the no-git command list.
- CLAUDE.md: validate now mentions check:bundled-skill alongside
  check:bundled.
- overview.md: archon doctor row added to the CLI command table.
- troubleshooting.md: points users at archon doctor after setup.

* chore(changelog): document Tier 5 setup overhaul cherry-pick batch (3 commits)

Adds CHANGELOG entries under [Unreleased]:

- Changed: setup wizard simplified to AI + skippable adapters (notes
  Discord/Postgres still runtime-supported via .env, just dropped from
  the interactive wizard).

- Fixed: 3 commits picked
  - 2c15439 — skill docs hardening (good-practices.md, troubleshooting.md, expanded workflow-dag.md)
  - 9122673 — parameter-matrix.md reference
  - 5e61faf — archon doctor + setup overhaul + complete bundled skill (coleam00#1566, deferred from PR #4)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Rasmus Widing <152263317+Wirasm@users.noreply.github.com>
Co-authored-by: Cole Medin <cole@dynamous.ai>
Co-authored-by: cjnprospa <sirhcle.j23@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

Setup overhaul: binary install primary, dead-simple wizard, archon doctor, embedded skill (v0.3.11)

1 participant