Conversation
…ar metadata (#1851) * feat(workflows): typed artifacts — output_type + engine-written sidecar metadata Foundation slice of the context-passing epic (#1847). Lets a node declare a semantic `output_type`; when set, the executor persists the node's output as a typed sidecar artifact so other nodes and later runs can locate it by type instead of guessing filenames. Foundation for cold-resume pointer recovery (#1846) and selective/by-reference context passing (#1848). - schema: optional `output_type` on dagNodeBaseSchema (general field — valid on every node type incl. bash/script; NOT added to *_AI_FIELDS), threaded through the transform's `base` group so it reaches all variants. - artifacts-index: new helper writes `nodes/<id>.md` + per-node `<id>.meta.json` (no shared index file → no parallel-layer write contention; index derived on read by globbing). `readNodeArtifacts`/`latestNodeArtifactOfType` for lookup. Corrupt/missing entries are non-fatal (skip + warn). Node id sanitized to a safe path segment. - executor: best-effort sidecar write at the single layer-result funnel (covers all node types); a metadata write never fails an otherwise-successful node. - nodeArtifactSchema in schemas/ (named to avoid clashing with the unrelated workflow-event artifactTypeSchema). Opt-in and additive: default behavior is unchanged when no node sets output_type. Tests: artifacts-index unit suite (write/read/latest/corrupt-skip/path-escape); dag-executor integration (typed node writes sidecar; untyped node writes none). Validation: bundled gates, all-package type-check, lint --max-warnings 0, format:check, full @archon/workflows test suite — all pass. * fix(workflows): harden typed-artifacts per PR review Address review findings on the typed-artifacts feature. All changes are additive/local; behavior is unchanged for nodes that don't set output_type. Correctness / invariants - node-artifact schema: producedAt -> z.string().datetime() so the lexicographic "latest" sort in latestNodeArtifactOfType can't silently return the wrong artifact for a corrupt/non-ISO value (rejected on read); size -> .int().nonnegative(); outputType -> .min(1) (matches output_type on the node config and the sibling artifactFileSchema). - writeNodeArtifact: collision guard. Two distinct node ids that sanitize to the same filename segment (e.g. `a.b` / `a_b`) now throw instead of silently overwriting — first writer wins, the loser is logged via the best-effort caller. Re-writing the same id (resume) still overwrites without error. Error handling (fail-fast) - readNodeArtifacts: only swallow readdir ENOENT ("no artifacts yet"); a real EACCES/ENOTDIR/EIO now warns + rethrows instead of masquerading as empty, matching script-discovery / workflow-discovery precedent. - index_entry_invalid warn now includes the Zod issues so schema drift is debuggable. Tests - best-effort write failure is non-fatal (node still completes when the sidecar write throws). - bash node with output_type writes a sidecar with no sessionId. - assert sessionId propagation in the AI-node integration test. - artifacts-index: collision throws, same-id overwrite, schema-invalid skip. Simplification - writeNodeArtifact params -> Omit<NodeArtifact, 'path' | 'size'> (DRY). - drop the redundant call-site conditional-spread for sessionId (the guard lives inside writeNodeArtifact). - latestNodeArtifactOfType: reduce -> explicit loop. - correct the misleading "concurrent contention" rationale in the JSDoc (writes are sequential post-allSettled; per-node files isolate nodes/runs). Docs - CLAUDE.md: output_type in the node-field list, nodes/ in the artifacts layout, node-artifact in the engine-schema list. - authoring-workflows guide: output_type row in Common fields, a Typed Artifacts subsection, and a quick-reference entry. Deferred (with rationale) - No user-facing chat warning on best-effort write failure: there are no consumers of these artifacts yet (cold-resume/context-passing land in #1846/#1848); a chat message per miss would be noise. Revisit with the first latestNodeArtifactOfType caller. - Did NOT constrain the global node `id` charset to fix the collision: that touches every workflow and could reject existing definitions — out of scope for this PR. The local write-time guard covers it without a breaking change. - Kept the lazy logger (review suggested removing it): dag-executor.test.ts mocks @archon/paths createLogger and imports this module transitively, so the deferral is load-bearing for test isolation. - AGENTS.md mirror skipped: it doesn't exist on this branch (added to dev after the branch forked); the 1:1 copy syncs when the branch catches up.
…lizer (#1878) * fix(web/console): gate /console behind auth + correct run-event normalizer Two correctness/security fixes that block the console from replacing the old chat + dashboard UI. Security: /console/* mounted OUTSIDE SessionGate, so with web auth enabled the console was reachable without login while every other route is gated. Wrap it in SessionGate — still outside Layout so it keeps omitting TopNav. SessionGate is a passthrough when web auth is disabled (the solo default), so this is a no-op there. Correctness: the run-event normalizer read keys the server never writes — - node duration read `duration`, but the row carries `duration_ms`, so every rendered node duration was null; - approvals checked `approval_pending`/`approval_resolved` with a `resolution` key, but the server writes `approval_requested`/`approval_received` with `decision` + `comment`/`reason`, so approvals fell through to the raw-JSON text fallback; - `node_skipped_prior_success` (emitted on resume for already-done nodes) was dropped entirely. Also carry the node_completed enrichment already in the payload (output preview, cost, stop reason, turns) for the upcoming per-node detail view. Adds the first test under experiments/console and wires src/experiments/console/ into the web test script so CI starts covering the console as it is promoted. * fix(web/console): address PR #1878 review (I1/I2 + S1/S2/S4/S5/S6 + docs) I1 approval_received now matches `decision` explicitly — an unknown/missing decision stays unresolved (null) instead of silently rendering as approved (the exact silent-mismatch class this normalizer set out to kill). I2 reworded the approval comment to future tense and noted that nothing renders approval events in the run stream today (paused gates use run.approval metadata). S1 transition mapping → NODE_TRANSITION_BY_EVENT record lookup (flatter, shows the whole map; node_failed kept explicit so its test guards the default). S2 enrichment JSDoc: dropped the "per-node accordion (follow-up)" forward reference; stated the completed-only null contract instead. S6 App.tsx comment: SessionGate is a passthrough after a brief (cached) auth-status check, not a literal no-op. S4 added a node_failed test (guards the map vs the `?? 'skipped'` default). S5 added error (error>message priority), workflow_*→system, and nodeName fallback tests; plus an I1 guard test (unknown decision → unresolved). docs narrowed the stale "CI does not guarantee these routes work" line in experiments/README.md now that console primitives run in CI. Skipped with rationale: S3 discriminated-union-on-transition refactor (defer to the PR that renders the fields); argsSummary dead-read cleanup (pre-existing, no consumer yet); a SessionGate automated test (needs jsdom/testing-library infra not present in @archon/web — a future SessionGate unit test is the right target).
…ng (#1877) (#1879) * Fix: reuse Pi extension loader per process so 2nd+ session doesn't hang (#1877) With Pi extensions enabled (the default), the 2nd and every subsequent sendQuery() per process hung in session setup and idle-timed-out after 30 min, breaking every Pi workflow with 2+ AI nodes. Pi's reload() re-invokes every installed extension factory from scratch; that process-scoped state is never torn down between calls (dispose() never emits session_shutdown), so the 2nd reload() deadlocks on the first call's still-live state. Changes: - Add getOrCreateReloadedExtensionLoader(): a process-level cache that builds and reload()s the extension-bearing DefaultResourceLoader once per (cwd, systemPrompt, skillPaths) and reuses it across sendQuery calls. createAgentSession with a supplied loader skips its own reload(), so reuse is safe; each call still gets its own session + bindExtensions(session_start). - provider.ts: use the cache on the extensions-on path; keep a fresh per-call loader on the extensions-off path (no reload(), no re-entrancy hazard). - Add regression tests: reload/construct run once across identical sequential calls; distinct cwd or systemPrompt each reload once; extensions-off never caches and never reloads. Fixes #1877 * docs(pi): document loader-cache growth bound and why eviction is omitted (#1877) Address self-review feedback on the unbounded-growth concern: clarify the cache is bounded by distinct (cwd, systemPrompt, skillPaths) tuples and explain that naive size-cap/LRU eviction is unsafe — dropping an in-use loader would make a running workflow's next node reload a second copy of the same extensions while the first is live, re-introducing the deadlock the cache prevents. * Address ultrareview findings on the Pi loader-cache fix (#1877) - I1: test the primary documented invariant — concurrent same-layer callers share a single in-flight reload() (gated mock + Promise.all; asserts one construct/one reload/same loader). Caching the resolved value instead of the Promise would fail this. - I2/S1: trim the docblock's unverifiable agent-rooms specifics (keep the general "factories build process-scoped singletons" mechanism) and drop the Pi SDK `sdk.js` build-artifact filename that will rot on restructure. - I3: correct the inline comment — setFlagValue itself unconditionally sets; the no-op comes from the provider's `if (runner)` guard, skipped when the runner is undefined. - S2: test failure-eviction — a rejected reload evicts its cache entry so the next call retries cleanly instead of returning a poisoned promise. - S3: rethrow reload() failures with an actionable pointer (extensions dir / enableExtensions: false), preserving the original error as `cause`. - S4: build the shared loader options once, then dispatch — removes the duplicated spread across the if/else branches. - S6: test additionalSkillPaths keying (distinct skill sets → distinct loaders). Deferred S5 (positional key fn → Pick<>): the public helper already uses Pick<> to document the keyed fields; the 3-line internal key fn stays explicit.
…ns (#1881) * feat(web/console): show run provenance — input message + platform icons Console run cards/rows/detail showed status, workflow, cost, and elapsed but never what input started a run — the prompt was parsed into the Run primitive (toRun) yet rendered nowhere, and the origin badge was text-only. This surfaces it: - OriginBadge: a Lucide icon per platform (web/cli/slack/telegram/discord/github; none for unknown), mirroring the old dashboard's PLATFORM_ICONS. - ActiveRunCard: an `input` row folded into the activity grid (truncated, full text on hover) for running + paused cards. - RecentRunRow: the message truncated inline beside the workflow name; the row stays h-9 (title tooltip for the full text). - RunStartedLine: a blockquote input line under "Workflow X started". - RunDetailHeader: a full-width `input` sub-row. - run.test.ts: first test for the primitive — normalizeOrigin (all platforms + case-insensitive + unknown fallback), userMessage default, pending→running, conversationPlatformId passthrough. (normalizeOrigin is now exported.) Deviation from plan (documented): the "from chat →" link is DEFERRED, not built. The console has no conversation-deep-link route — ChatPage is mounted at p/:projectId/chat and manages its active conversation internally, so a chat link has no valid destination. Per the plan's "do not invent a route" constraint, the link AND the workerPlatformId/parentPlatformId fields that only existed to feed it were dropped (they'd be unread data). This ships the unambiguous wins that fully address the stated need ("what did I enter… cli/chat"); the link moves to a follow-up once console chat supports conversation deep-linking. * fix(web/console): address PR #1881 review (I1-I3 + S1-S5) I1 RecentRunRow layout regression: when userMessage is empty the workflow name now fills the full width (flex-1) again instead of being capped at 55% with blank space + needless truncation; capped at 55% only when a message shares the line. I2 ActiveRunCard comment: "always" → "when present" (the input row is gated on a non-empty message). I3 OriginBadge comment: "mirrors" → "extends" — the icon map is a 7-origin superset of the old 5-entry map, adds discord, and renders no icon for unknown (vs the old Globe fallback). S1 Extracted a local `hasValue` type-guard to replace the repeated null/undefined/'' chains on currentNode/lastTool (also narrows to string). S2 Named the grid guard `showDetailGrid` (matches the file's elapsed/canOpen idiom). S3 RecentRunRow: rerun-param guard uses `!== ''` to match the render guard. S4 Backfilled run.ts coverage: unknown-status→running, readCost (positive, the $0.00 >0-guard, and non-numeric→null), and approval-metadata parsing (well-formed, message default, malformed→null). 29 → 36 console tests. S5 RunDetailHeader: the deferred "from chat" link now has a tracked anchor — TODO(#1882) (filed the follow-up issue).
…vider tiers (#1873) Wires the model-alias resolver (PR #1867) into execution: cross-provider `tiers:` config key, aiProfile threaded through executeWorkflow → executeDagWorkflow → resolveNodeProviderAndModel (loop nodes included), warn-on-conflict, per-provider effort routing, internal call-sites (chat=large, title=small), and bundled defaults retrofitted to tiers-only with a validate gate. Closes #1872.
… + repair + fail-fast (PR 1/2) (#1883) * feat(workflows): reliable cross-provider structured output — validate + repair + fail-fast (PR 1/2) Keystone of the cross-provider structured-output plan (#1849). Makes output_format handoffs trustworthy across every provider and removes two silent failures. - Schema validation for ALL providers: parsed structured output is validated against the node's declared JSON Schema (ajv) — even SDK-enforced providers (Claude/Codex/OpenCode) need this net for refusal / max_tokens-truncation edges. - JSON repair tier: jsonrepair recovers trailing commas, single quotes, unquoted keys, and truncated tails (gated to object-shaped input + object-only results so it can't coerce prose into bogus arrays). - Honest capability tier: ProviderCapabilities.structuredOutput is now 'enforced' | 'best-effort' | false (was a meaningless boolean true for all five). Claude/Codex/OpenCode = enforced; Pi/Copilot = best-effort. Threaded through the OpenAPI schema and regenerated web type. - Fail-fast (no silent degrade): a node that declares output_format but returns no schema-valid structured output now FAILS instead of completing with poisoned prose. Routed through the existing per-node catch. - No-silent-drop field access: $node.output.field resolution is strict — a field not in the producer's declared schema, or a schemaless node whose output isn't JSON / lacks the key, throws (fails the consuming node). The only value that resolves to '' is an author-declared-optional field. Captured via NodeOutput.declaredFields; shared by prompt/script substitution and when:. Behavior changes (intended, fail-fast): schemaless bash/script nodes missing a referenced key now throw; when: field-ref errors throw (reverses #1673 fail-closed for field refs); explicit null on a declared field resolves to '' (was "null"). Bundled-default workflows audited safe (all field refs target AI nodes whose schemas declare the fields as required). Reask loop (Tasks 10-13) is PR 2. * test(workflows): e2e smokes for cross-provider structured output (PR 1) Two CLI-runnable smokes exercising the new behavior against Claude (enforced): - e2e-structured-output (positive): output_format validation (Task 7), declared field access, declared-OPTIONAL absent → '' (no throw), schemaless bash JSON field access. Completes successfully. - e2e-structured-output-failfast (negative): a $node.output ref to a field NOT in the producer's schema fails the node with an OutputRefError (Task 9 no-silent-drop) instead of silently resolving to ''. Both verified end-to-end via `bun run cli workflow run ... --no-worktree`. * Address ultrareview findings on structured-output PR 1 (#1883) Comment/doc accuracy (the contract-inverting set — highest value): - I1: structured-output.ts module + tryParseStructuredOutput docstrings no longer say "degrades via warning" (now fail-fast); "three failure modes" → four (jsonrepair). - I2: types.ts + pi/copilot capability comments stop describing the reask loop as present (it's PR 2) — now "validate; reask = PR 2". - I3: condition-evaluator module docstring distinguishes fail-closed-skip (bad syntax) from throw-and-fail (.field ref that can't resolve). - I4: docs updated where now-wrong — dag-workflows.md ("fails open" → the two real modes), ai-assistants.md (Pi/Copilot "degrades to warning" → validates + fails), authoring-workflows.md (output_format validation + strict field access), CLAUDE.md/AGENTS.md (output_format parenthetical + structuredOutput tiered union). - S7: condition-evaluator return-'' comment corrected (lenient null → "null"). Code: - I5: tryJsonParseObject is object-only across all tiers (tier-1 no longer returns bare arrays); removes the tier-1-accepts / tier-3-rejects inconsistency. Tests aligned. - I7: uncompilable output_format schema now surfaces a user-facing warning (not just a log) — the silent validation-bypass is no longer silent. - S2: dropped "// Task 7/8 —" planning labels (kept the prose). - S3: StructuredValidationResult is a discriminated union ({valid:true} | {valid:false; errors}). - S4: $node.output.field on a skipped/pending producer throws a clear 'producer-not-run' OutputRefError instead of a misleading "not a JSON object". Tests: - I6: executor integration test — a when: ref to a field not in the producer's schema FAILS the node (regression guard against a silent skip). - S8: declaredFieldsFromSchema(properties:null) + producer-not-run cases. Deferred (with rationale): S1 (keep `false` tier — plan-specced, honest "none" state), S6 (unknown-node throw — direction/scope expansion beyond field resolution), S8 best-effort integration mirror (PR1 fail-fast is tier-agnostic; meaningful only once PR2 adds reask), S5 (tier-3 swallow already observable at executor level; would add noise + a logger dep to a pure module), S9 (no real CLAUDE.md violation).
…-bottom (#1885) * feat(web/console): chat workflow-completion card + SSE-lock + jump-to-bottom Three changes that make the console's project-scoped chat a credible /chat replacement — all web-side; the data already flowed and was being dropped. Completion card: a chat-launched workflow's `workflow_result` message (summary + {workflowName, runId}) was swallowed because ChatStream filters the whole `workflow_` category prefix and toMessage never parsed `workflowResult`. Now message.ts parses `workflowResult` (mirroring the `dispatch` parse, both fields guarded), and ChatStream lets `workflow_result` through the filter — without un-suppressing the other workflow_* narration — and renders a new ConsoleWorkflowResultCard: status icon + node counts + duration + cost + "Open run →" + the summary body. The card fetches authoritative state via skill.getRun (same call RunDetailPage uses) and degrades to the summary alone if the run can't be loaded. SSE-lock: the composer stayed disabled ~6s (SETTLE_MS) after each reply because the SSE lock event wasn't wired. ChatPage now passes a useCallback-stable onLockChange to useConversationSSE that clears the settle timer + setBusy(false) on conversation_lock:false. The settle timer remains the correctness fallback when SSE is absent. (Stable callback is required — the hook's effect depends on it, so an inline lambda would reconnect the EventSource every render.) Jump-to-bottom: an `atBottom` state (from an onScroll handler) shows a floating button when scrolled up; kept in sync with the existing auto-scroll lastBottomRef. Tests: message.test.ts covers the workflowResult parse (well-formed, malformed→null, dispatch regression, isSystemCategory). 36 → 42 console tests. * fix(web/console): address PR #1885 review — render the card (C1) + hardening C1 (critical): the completion card never rendered its rich content. useEntity returns `error: Error | undefined` (cache.ts), so `error !== null` was always true and `run` was always null → the summary-only fallback every time. Compare against `undefined`. Verified end-to-end against a real completed run: GET /api/workflows/runs/:id returns {run, events}, so the rich path now executes (16/22 nodes computed on that run). I1: guard meta.workflowResult with `!= null` (not `!== undefined`) so an explicit JSON null can't reach `typeof wr.workflowName` and throw. +regression test. I2: countTerminalNodes deduped by nodeId — a resumed run reuses one run id and emits both the original `completed` and a later node_skipped_prior_success, which double-counted nodes. Relocated to primitives/event.ts (pure RunEvent reducer, its natural home) and unit-tested (6 cases incl. the dedup guard). I3: console.warn on the two silent swallows (getRun error path, parseMetadata catch) — the PR's whole point is to stop silently dropping completions. S1: dropped the dead `| null` from the useEntity generic (getRun never resolves to null; undefined is the cache-miss signal). S3: extracted NEAR_BOTTOM_PX for the duplicated 120px scroll threshold. S5/S6: corrected the card JSDoc (loading also degrades to summary) and documented why ParsedMetadata.workflowResult stays inline/untrusted. Deferred — S2 (overeng): kept RESULT_GLYPH/RESULT_LABEL as Partial<Record> plus a clarifying comment rather than padding them to a total record with running/paused entries a completion card never displays. Validation: web type-check / lint / format:check clean; 51 console tests pass.
…1890) * feat(web/console): settings core — assistant config + system panel Adds the console's first settings surface (/console/settings, global) — the parity floor before cutover. PR #4 of the console sequence (after #1878/#1881/#1885). - skills/settings.ts: getConfig/updateAssistantConfig/getHealth/getUpdateCheck plus the pure buildAssistantUpdate(form) transform (8 unit tests). skills/providers.ts: listProviders. Types from @/lib/api.generated (console isolation boundary). - store/keys.ts: config/health/providers/updateCheck keys (health reuses the literal 'health' so it shares lib/health's cache entry). - lib/health.ts: full HealthResponse + useHealth(); useIsDocker derives from it. - AssistantConfigPanel: default-assistant picker (registered providers) + free-text model per provider + codex reasoning/web-search; dirty-gated Save → PATCH /api/config/assistants → ~/.archon/config.yaml → invalidate(K.config) re-seeds. Model is free-text for every provider (Archon does not validate model strings). - SystemPanel: status/adapter/db/version, concurrency (active/maxConcurrent, coerced defensively — concurrency is an open record), running workflows, platform badges, update-check. - ConsoleApp: /console/settings route, gear header link, ',' global keybinding; shortcuts.ts catalogue entry. Honors the error-is-undefined cache contract throughout (the #1885 gotcha). Excludes the GitHub device-flow panel (PR #5) and env-var editing (project-scoped). Validation: web type-check / lint / format:check clean; 59 console tests pass (8 new). Verified end-to-end on an isolated server: read APIs return the expected shapes, save round-trips to config.yaml (binary paths preserved via the server deep-merge), and a browser smoke of /console/settings renders both panels (5 providers, codex effort/web-search, system grid, Save dirty-gated). * fix(web/console): address PR #1890 review — comments + update-check silent failure I1: correct the buildAssistantUpdate JSDoc. Verified the PATCH route does NOT safe-filter per field on the write path — it validates only provider ids and merges the body into config.yaml unfiltered (safe-filtering is read-path only). The real invariant is that this function only ever attaches codex-only fields to the codex entry; the comment now says that instead of the false "server safe-filters" claim. I2: rewrite the K.health note. This PR routes lib/health through K.health, so the old "lib/health already caches under this literal" premise is stale; the invariant is that both consumers read via useHealth() to share one cache entry. I3 (silent failure): SystemPanel showed "checking…" forever on a failed update-check (the error was destructured away). Surface updateError via an UpdateStatus helper → "update check unavailable". S1: SettingsSection children typed ReactNode (ReactElement|ReactElement[] fought the `cond && <el/>` pattern). S5: replaced the nested update-status ternary with the UpdateStatus helper + early returns for the health loading/error states. S6: extracted the shared SettingsSection card shell (PR #5 is the 3rd consumer), a SELECT_CLASS const for the two codex selects, and bound activePlatforms once. Docs: added /console/settings to the console README routes. Deferred (with rationale): - S2 (re-seed on providers identity): latent only — nothing invalidates K.providers, and a ref-snapshot "fix" introduces a config-vs-providers load-order race. Keep the simple [config, providers] effect. - S3 (literal-union effort/webSearch types): kept bare string so seedForm tolerates an out-of-enum value in config.yaml; the <select> is the write-side enforcement. - S4 (show both load errors): an error panel showing the first error is acceptable. Validation: web type-check / lint / format:check clean; 59 console tests pass.
…utput (PR 2/2) (#1889) * feat(workflows): validate-and-reask loop for best-effort structured output (PR 2/2) Completes the cross-provider structured-output plan (#1849). Best-effort providers (Pi/Copilot) now get a bounded reask loop instead of failing on the first miss. - Task 10 — reask loop. When a node declares output_format AND the provider is 'best-effort' AND the parsed output is missing/schema-invalid, the executor re-runs sendQuery with the schema errors appended, up to STRUCTURED_OUTPUT_MAX_REASKS (3). Enforced providers and non-output_format nodes get 0 reasks (unchanged fail-fast). Fresh session per attempt; cost accumulates across attempts; no reask on idle-timeout/abort/SDK-error. Exhaustion → the existing fail-fast throw. Implemented by extracting the stream into a runStreamPass() closure (no body re-indent) wrapped in the reask while-loop. - Task 11 — observability. Logs dag.structured_output_reask per attempt and sends one chat notice on the first reask so auto-correction isn't invisible. - Task 13 — docs. "Structured output guarantees" matrix in ai-assistants.md. - Tests: best-effort malformed→fixed recovers within reasks (2 sendQuery calls → completed); exhaustion fails loudly (1 + 3 calls → node_failed). Registered Pi in the dag-executor test registry so capabilities resolve to 'best-effort'. Task 12 (capability tests) landed in PR 1. Depends on #1879 (Pi extension-loader reuse) — already merged — since reask fires multiple Pi sendQuery calls per process. * Address ultrareview findings on the reask loop (#1889) Functional bugs (the 3 real-but-narrow ones): - I1: reset batchMessages per attempt in runStreamPass — else a failed reask attempt's prose could flush to the user via an intermediate `msg.flush` chunk. - I2: an idle-timeout on a best-effort output_format node now throws a timeout-specific error instead of the misleading "the model replied with prose" message; corrected the inaccurate "post-loop guards handle them" comment. - I3: carry the accumulated reask cost onto nodeCostUsd every pass so the exhaustion throw path reports cost across ALL attempts, not just the last. Docs (self-contradiction + stale "PR 2" references now that reask ships): - I4: ai-assistants.md Pi/Copilot rows said reask was "planned" — now "re-asks 3×". - S4: dropped "reask = PR 2"/"lands in PR 2" from pi/copilot capabilities, the shared structured-output docstring, and the types.ts capability doc. - S5: added a reask mention to CLAUDE.md/AGENTS.md output_format + authoring docs. - S6: comment fixes (runStreamPass now states what it resets; buildReaskPrompt no longer overclaims clean prompt/schema separation). Tests: - I5: enforced provider (claude) makes exactly ONE sendQuery on a validation miss (regression guard against the best-effort gate silently allowing 4× calls). - I6: best-effort MISSING structured output (not just invalid) triggers reask and recovers. - S1: recover test asserts accumulated cost (0.01 + 0.02 = 0.03); idle-timeout on an output_format node does NOT reask (1 call, "timed out" error). - S2: extracted scheduleReask() so the two reask blocks can't drift. - S3: dropped the (Task 10) labels from the reask test names. Deferred S7 (for-loop vs while(true)): while(true) with inline throws is clearer for the two distinct exhaustion messages (invalid vs missing); a for-loop would push the throws after the loop and need failure-reason tracking.
…1884) (#1894) * Fix: warn on the bash node $node.output double-quote footgun + docs (#1884) In bash: nodes, $nodeId.output[.field] is injected pre-quoted by Archon (single-quoted inline for small values, $(cat ...) for >32 KB). Wrapping it in double quotes corrupts both forms — `var="$n.output.field"` becomes the literal `'value'`, so downstream comparisons silently fail. Numeric/boolean fields are injected raw, so it "works" for them, which makes the bug intermittent. - validator: emit a `warning`-level issue when a bash node body contains a double-quoted `$nodeId.output` substitution, with the unquoted-idiom hint. - docs: a caution callout in authoring-workflows.md, the shell-quoting footgun section in variables.md, and a cross-reference from loop-nodes.md. Extracted from the autonomous #1888 run, dropping its off-scope changes (model tier→concrete reverts across 14 bundled workflows, which would undo #1873's model-alias adoption, plus unrelated validateModelRef/title-generator refactors). Closes #1884 * Address code-review + silent-failure-hunter findings on #1894 - Regex false positive (code-reviewer): the old `/"[^"\n]*\$…\.output/` matched the CLOSING quote of an unrelated prior string and slid across `;` to a correctly-unquoted ref — `echo "hi"; x=$a.output` warned spuriously. Require the opening `"` to be an operand: `/(?:^|[=\s])"…/m`. + a regression test. - Wrong shell claim (silent-failure-hunter): the comment + both docs said double-quoting `$(cat ...)` "enables word-splitting" — backwards. `var="$(cat ...)"` is correct bash; the footgun is only the SMALL/inline case (`var="'value'"`). Rewrote the rationale: the rule is unconditional because you can't predict output size at author time. - Under-detection (silent-failure-hunter): loop `until_bash` substitutes via the same escapedForBash=true path, so the footgun applies there too — but it wasn't checked. Extended the lint to `loop.until_bash` (+ test). - Hint accuracy: dropped the "the injected single-quotes are the quoting" overclaim (implied $var never needs quoting) for "injected already quoted", and noted the numeric/boolean false-positive so the warning isn't dismissed wholesale.
…ror re-render (#1896) * feat(web/console): GitHub identity panel (device flow) + fix cache error re-render Adds GithubIdentityPanel to /console/settings: the per-user GitHub device flow (connect → poll → disconnect), ported from the old UI into the console's react-query-free skill/cache layer. Renders NOTHING on solo-PAT installs (GET /api/auth/github 401 = no web identity) so they never see an irrelevant panel. PR #5 of the console sequence (the cutover line). - skills/github.ts: inline device-flow types (not yet in api.generated) + the four verbs + the pure interpretPollStatus (7 unit tests). K.githubConnection + barrel. - GithubIdentityPanel: useEntity(K.githubConnection); HttpError 401 → render null (hide); ported start/poll loop with a cancelledRef unmount guard; user_code + verification link while pending; invalidate on connect/disconnect. - Mounted as the third settings panel. DEVIATION (root-cause fix, beyond the plan's scope): the 401-hide did not work as planned. The console cache's useEntity never re-rendered on the ERROR transition — getSnapshot returned cache.get(key), which stays undefined on error, so useSyncExternalStore bailed out and `error` (read outside the store) was never surfaced; the panel hung on "Loading…" instead of hiding. This silently broke EVERY error branch in the console, including #1890's Assistant/System panels. Fixed at the root: a per-key version counter bumped on every notify() is now the snapshot, so error transitions re-render. The extra re-renders happen only on errors (success already changes the cached object's identity), so there is no hot-path cost. Verified end-to-end in a browser against an isolated server: solo (401) → panel hidden (headings: Settings | Assistant | System); multi-user (200 via X-Archon-User) → panel shows "Connect GitHub". Real ~/.archon untouched (isolated ARCHON_HOME). Validation: web type-check / lint / format:check clean; 66 console tests pass (7 new). * fix(web/console): address PR #1896 review — disconnect-error swallow + polish I1 (silent failure): a failed disconnect set `message` but not `phase`, and the JSX gated on `phase === 'error'`, so disconnect errors rendered nothing (the button just re-enabled). Render on `message !== null` — both connect/disconnect clear it on start. I2: scoped down the cache "no tearing" comment. data/error/loading can briefly co-exist (loading is still true when an error first lands — inflight clears in a later .finally), so consumers check error before loading, as the panels do. S1: reworded the 401-hide comment + github.ts docstring — 401 = "no web identity" (the solo-PAT state AND a logged-out user on a web-auth install), not "universal solo-PAT". S2: avoid the one-render "Connect GitHub" flash after a successful connect — keep phase 'pending' (button "Connecting…") until the status refetch flips connected; reset phase at the start of disconnect. S3: defensive `default` in interpretPollStatus — an unrecognized server status (inline-type drift) now fails terminally instead of crashing the poll loop on `undefined.kind`. +test. Docs: console README route now lists GitHub identity. Deferred (with rationale): - S4 (consolidate userCode/verificationUri into one object): optional churn; they are set/cleared together in one place, so the "both or neither" footgun is contained. - S5 (cache regression test): locking "error transition re-renders" needs either React/jsdom (the console deliberately has none) or a production test-seam export into the core primitive; the fix is browser-verified end-to-end. Acknowledged gap. Validation: web type-check / lint / format:check clean; 67 console tests pass.
Phase 2 PR 1/4 — the inert foundation: encrypted remote_agent_user_provider_keys store, opt-in gate (TOKEN_ENCRYPTION_KEY) + boot assert, Archon-owned credential delivery map, resolve+inject seams in executeWorkflow and the chat orchestrator (no-op until a credential exists), and CLI run attribution. Part of #1891.
…e) (#1903) * feat(web/console): chat surface restyle to Direction B (Modern Console) Pure visual restyle of the experimental console's project-scoped chat (`/console/p/:projectId/chat`) faithfully recreating the Direction B "Modern Console" design from the Claude design handoff. All existing SSE streaming, tool calls, approvals, pending input, run cards, node dividers, and workflow_result navigation behavior is preserved. - AgentAvatar: new 30px gradient-ring SVG (useId-scoped gradient ids) mirroring chat-icons.jsx - MessageItem: role-branched render — user → outlined-magenta bubble + meta row; assistant → soft surface-elevated card + 30px AgentAvatar + meta row. MD_COMPONENTS preserved verbatim - ChatComposer: rebuilt cbox shell with magenta focus-within ring, decorative lead buttons, auto-grow textarea, gradient .brand-bar Send button + kbd-hint row. Keymap (Enter/Shift+Enter/Escape) byte-identical - ChatStream: row gap 1.5 → 14px (compact density). Filter logic untouched - ConsoleWorkflowResultCard: 3px status-colored left accent bar, 34px status-tinted icon badge, color-keyed border + bg-tint per status (failed=rose + 'exit 1' chip, completed=teal, cancelled=neutral). useEntity, error→summary fallback, navigation, metadata preserved Scope is locked to `packages/web/src/experiments/console/components/`. No production-web modules touched. No theme.css / index.css token additions. StreamCard / ToolCallItem / WorkingIndicator / NodeDivider / PendingInputBanner / WorkflowDock / DraftRunCard / ChatPage / primitives / skills unchanged. * fix(web/console): replace remaining hex literals with brand tokens Address PR review findings (HIGH + MEDIUM auto-fix candidates). Fixes applied: - MessageItem.tsx:115 user-bubble color: #F6E9F6 -> color-mix(in oklch, white, var(--brand-magenta) 12%). Harmonises with bg/border/glow that already use --brand-magenta via color-mix; satisfies scope's "no hex literals introduced" guardrail. - AgentAvatar.tsx:28 inner-disc fill: #15171d -> var(--surface-elevated). SVG element fill on inline JSX honours CSS variables; comment updated to drop the now-stale "hard-coded" claim. Validation: type-check, lint, format:check all clean. Review artifacts: artifacts/runs/5906bfd23bc1f42c85feb2a6649fba98/review/ * fix(web/console): center chat stream at 940px and fix timestamp letter-spacing The design's .stream-inner caps the message column at 940px centered, matching the composer — the stream previously spanned the full width, pinning user bubbles to the viewport edge and leaving dead space. Also: timestamps used tracking-[0.3em] where the design specifies letter-spacing .3px, and stream padding now matches the design's 26px 30px 18px. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web/console): use real Archon shield logo in agent avatar The handoff's AgentAvatar drew a placeholder triangle mark; swap it for the actual brand asset (/favicon.png, same as the console topbar) inside the gradient ring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web/console): sidebar v2 + runs view restyle from design handoff Sidebar (design v2): Projects header with count pill, filter input, owner-grouped list with hairline section labels, 28px monogram tiles (gradient on the active project), LIVE pulse on the selected row, and a drag-to-resize right edge (232-440px, persisted to localStorage). Runs view: status filters restyled as mono uppercase sub-tabs with gradient underline and count pills in their own strip; section headers with count pills; 300px rounded search field; recent rows on the design grid (status | body | id | duration | cost | CLI) with a copy-CLI-command button, origin + quick actions on hover; draft card with magenta glow, kbd hint chips, gradient Start-run button; active running cards tinted amber per design. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web/console): run-detail graph + log restyle from design handoff v3 Graph: dotted canvas, 168x46 rounded nodes with status tints (completed green, failed rose with soft ring, pending/skipped dimmed), status-colored kind glyphs, and orthogonal elbow connectors computed from node rects. Log: node-transition rows with offset gutter, dashed leader line and muted-entered / green-completed states; tool calls as design rows (violet TOOL tag, bold name, args preview, duration, expandable detail); agent messages in a log variant (violet left accent, mono body, no avatar); toolbar tabs in mono with magenta-accent filter checkboxes. Footer: gradient Resume + ghost Abandon on failed runs; completed and cancelled runs get a Re-run button that pre-fills the draft composer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web/console): add-project modal restyle from design handoff v4 Blurred scrim, centered 520px card with brand-gradient top accent, segmented GitHub/Local control with sliding indicator, 46px icon input with magenta focus ring, live clone-path hint derived from the typed GitHub URL, and Esc/backdrop/x/Cancel all close. Submit flow (skill calls, error surface, submitting state) unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web/console): settings + env-vars modal + project actions restyle (design v5) Settings: 22px page title over a centered 680px column; cards at 15px radius with bold titles; provider rows as elevated cards where the default assistant gets a magenta tint + DEFAULT pill; mono inputs and selects with magenta focus rings; gradient Save button. System card rows on hairline separators with a green ok status and platform pills. Sidebar project actions: env-vars button now a recognizable key icon and More actions a true 3-dot icon (29px hit areas); the menu is a styled popover with a trash-icon red Remove project item. Env-vars modal: blurred scrim, 560px card with gradient top accent, project name beside the title, panel key list with 34px trash buttons, dashed Add variable row (NAME/value inputs, magenta focus), ghost Close, and Esc-to-close. Keys-only API model unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web/console): custom select chevrons in settings Native select arrows render pinned to the control's right edge; the design (.set-select) hides them with appearance:none and paints its own chevron 11px from the edge. Adds a SelectShell wrapper that overlays the design chevron on all three settings selects. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… + per-node cost/turns (#1907) * feat(web/console): fold node lifecycle into one divider + node filter + per-node cost/turns Run-stream primitive fixes for the console run-detail view: - foldNodeRuns() in primitives/event.ts folds a node's 2-3 transitions (started + terminal, plus resume-time skipped_prior_success) into one NodeRun keyed by nodeId. RunStream now renders ONE divider per node instead of one per transition — fixes the duplicate node markers. countTerminalNodes() is re-expressed via foldNodeRuns so the dedup is single-sourced. - Thread nodeId through pairToolEvents -> the tool timeline entry (it was dropped), so tools carry node identity. - Node filter dropdown in StreamToolbar: pick a node to isolate its slice of the stream. Hybrid predicate — exact nodeId match for node-attributed entries, time-window fallback for node-blind ones (Claude message-inline tools / prose) so the filter works across providers. Pure client-side .filter(); persisted to localStorage; auto-resets when the node is absent. - NodeDivider goes transition->status (adds 'running' for in-flight nodes) and surfaces per-node cost + turns inline ($x · Nt) with stop_reason under the System detail toggle. Anchored on nodeId (fixes the latent name!=id graph-scroll case). Tests: foldNodeRuns fold/precedence/ordering/null-id + cost/turns carry; pairToolEvents nodeId threading. * review: address PR #1907 suggestions (S1-S5, S7) - S1: add 2 foldNodeRuns precedence tests (failed→completed, completed→failed) pinning the central 'ever-completed wins' claim — guards a future precedence reorder. - S2: reset-guard uses useLayoutEffect (not useEffect) so a stale node filter resets before paint — no one-frame 'Waiting for first event…' flash when navigating to a cached run whose node set lacks the selection. - S3: broaden the nodeWindow / visible-filter comment — artifact + system rows (not just message-inline tools) also use the time-window fallback. - S4: source cost/turns/stop from the completed transition, not whatever terminal transition exists (a failed/skipped terminal carries none); JSDoc tightened to match. - S5: status fold is if/else-if in documented precedence order; foldNodeRuns hoisted to one useMemo in RunStream (timeline + nodeWindow share the snapshot). - S7: document the console localStorage keys (README table); PR body Linked Issue fixed (dropped dangling Closes, linked follow-ups #1908/#1909). Deferred (conscious): S6 — NodeRun stays flat-nullable; a discriminated-union refactor waits for a second consumer (the deferred node-detail panel), per the reviewer's own recommendation.
…gacy (#1915) * feat(web): make /console the default UI, re-root classic UI under /legacy Console cutover, structural half (PR-A of 2): - Routing: "/" now redirects to /console (was /chat). The classic UI's shared Layout group is re-rooted under /legacy (/legacy, /legacy/chat, /legacy/dashboard, /legacy/workflows[/builder][/runs], /legacy/settings) for a deprecation window; SessionGate still wraps both. The legacy workflow builder stays reachable — the console links out to it rather than rebuilding it. - Nav off the top, into the sidebar: deleted the console's top banner; the brand (logo + Archon + console pill) now heads the ProjectRail, and a menu section under "Add project" (separated by a border-t divider) holds Settings (/console/settings), Workflows (/legacy/workflows), and the "Old UI" escape hatch (/legacy). - Cross-UI links: console "Old UI" -> /legacy; the classic TopNav already links to /console (unchanged) as the "new UI" return path. - Re-rooting fallout: prefixed every absolute internal link in the old UI with /legacy (~27 sites across 19 files: TopNav tabs + brand + the dashboard-badge comparison, Sidebar, chat/dashboard/workflows/sidebar components). react-router v7 does not rewrite absolute links under a path prefix, so each had to be prefixed explicitly. API (/api/*) and /console links untouched. Login still lands on "/" -> /console. Old top-level bookmarks (/chat etc.) now 404; compat redirects deferred (issue #1912). Verified: / -> /console (no top banner, sidebar brand + menu); /legacy loads classic chat with all nav prefixed; "Old UI" <-> "new UI" round-trip. * review: address PR #1915 findings (I1, I2, S3, S5) - I1: /legacy index now Navigate to "chat" (URL /legacy/chat) so the TopNav Chat tab highlights on landing via "Old UI" — restores the pre-cutover redirect behavior (was rendering ChatPage at bare /legacy, no tab match). - I2: prefixed the 5 dead route references in docs-web adapters/web.md (/dashboard, /settings, /workflows/runs/:runId, /workflows/builder, /workflows) with /legacy so readers don't hit 404s. - S3: extracted the repeated 130-char nav-link className into a file-local RailNavLink (rule-of-three — Settings/Workflows/Old UI). - S5: CLAUDE.md console framing updated (default at /, classic UI under /legacy). Skipped (YAGNI for a surface slated for deletion in #1912): S1 (LEGACY constant across 19 files — the legacy files get deleted wholesale, not re-prefixed, so the constant's rename-safety payoff doesn't apply) and S2 (badge-flag refactor on the deprecated TopNav). S4 was a verify-only note — the rail layout was confirmed in the smoke test.
#1916) * feat(web/console): wire chat file uploads + cap tool-call input height Console cutover, polish half (PR-B of 2) — decoupled from the structural cutover (PR-A): - Chat uploads: the composer's 📎 was inert. Now it opens a file picker, shows attached files as removable chips (name + size + ✕), validates size (10 MB) / count (5) / type client-side, and threads the files through ChatPage -> skill.sendMessage, which already builds the multipart upload (zero backend/skill change). Accepted-type set is copied (not imported) from the old MessageInput per the console isolation rule; the server remains the authoritative validator. First-message uploads aren't supported (createConversation is JSON-only) and are surfaced as a notice rather than dropped silently. - Tool-call input cap: the expanded tool input <pre> dumped unbounded JSON (a wall of text on busy nodes, e.g. 130-tool TodoWrite). Capped to max-h-[320px] overflow-auto so each expanded call stays scannable. The Result block was already char-capped. Verified: 📎 opens the picker, a chip renders with name + size + remove; console test suite green (80). * review: address PR #1916 findings (I1-I3, S1-S3) - I1: first-message file-drop now uses a distinct non-error "notice" channel (amber, separate from the red error stripe) and is reworded to be actionable once the agent replies ("re-attach and send once the chat has started") rather than an unactionable "now" while the composer is locked. - I2: corrected the inaccurate "mirrors the old MessageInput set" comment — it's an approximate subset (documented as such, can drift; server is authoritative). - I3: addFiles accumulates ALL rejection reasons ("Skipped N file(s) — …") instead of overwriting to show only the last. - S1: extracted isAcceptedFileType + formatBytes + the limits into a tested primitives/file.ts (file.test.ts covers the empty-MIME extension fallback, no-extension/dotfile rejection, case-insensitivity, and formatBytes). Also hardened isAcceptedFileType to strip a `;charset=…` MIME parameter (real-world + Bun's File append one) so the exact pdf/json checks still match. - S2: added MAX_FILE_MB so the size error reads "larger than 10 MB" (kept the explicit String() coercions — the repo's restrict-template-expressions needs them). - S3: marked file-attachments shipped in console-open-questions.md (with the first-message caveat) and documented uploads + the limitation in the README. S4 was a no-op (type design rated adequate, no change requested).
API-key connect surface on top of the #1899 per-user credentials foundation: core connect-service, REST routes (GET/PUT/DELETE /api/auth/providers), the `archon ai` CLI command, and a console Settings panel. API-key kind only; OAuth subscription login + Codex auth.json delivery are a later slice. Gated on TOKEN_ENCRYPTION_KEY; solo installs unchanged. Part of #1891 (PR 2/4).
* Fix: make workspace sync non-destructive (#1516) Chat sync for managed source clones could hard-reset to origin on ordinary messages, discarding tracked local edits or branch tips. Make sync non-destructive by default and keep destructive reset explicit for worktree creation. Changes: - Added mode-based workspace sync with local/remote state classification - Updated chat sync to use safe fast-forward behavior and clearer sync events - Kept managed worktree creation on explicit reset mode - Persisted detected codebase default branches - Added regression coverage for dirty, ahead, diverged, and fast-forward cases Fixes #1516 * Fix register project default branch test * fix: address review findings Fixed: - Add SQLite default_branch migration for existing databases - Keep codebase branch metadata in sync when re-registering paths - Surface unexpected merge-base failures and log short SHA telemetry failures - Update stale sync/schema docs and comments Tests added: - Branch persistence and detached HEAD coverage for clone/register flows - Orchestrator register-project branch detection coverage - Git sync error handling coverage - SQLite/codebase/schema coverage for default_branch Skipped: - none * simplify: reduce complexity in changed files
…3a) (#1921) * feat(credentials): subscription (OAuth) auth + Codex/Pi delivery (PR 3a) Completes the per-user credential story (PR-2 shipped API keys) with subscription login — Claude Pro/Max, ChatGPT/Codex, GitHub Copilot — delegated to Pi's @earendil-works/pi-ai/oauth, plus delivery to all three native runtimes AND Pi. Backend only; the AI-settings UI + config tier editor are PR-3b. - @archon/providers/oauth: SDK-boundary wrapper (the Pi SDK dep stays in @archon/providers; core consumes the wrapper, not the SDK directly). - core/credentials/oauth-bridge.ts: held-session state machine over Pi's login() — manual-code (Anthropic/Codex) + device-code (Copilot); start -> poll(code?), sessions bound to userId, short-TTL, abortable. - persistProviderOAuth + the Archon->Pi OAuth provider map (oauth-providers.ts). - store oauth read path: decrypt -> getOAuthApiKey (auto-refresh) -> re-save on rotation -> usable bearer (in-flight Map; was a null stub). - delivery: real Codex auth.json (setup-auth.ts shape) + buildPiAuthJson aggregate (keys + subscriptions, Pi AuthStorage format) + a Copilot branch. - providers/pi: honor ARCHON_PI_AUTH_PATH -> AuthStorage.create(authPath) so Pi reads the per-run auth.json WITHOUT moving its home (models.json/settings.json preserved). - workflow inject (store-adapter): write the aggregate Pi auth.json + set ARCHON_PI_AUTH_PATH. - server: POST /api/auth/providers/:provider/oauth/{start,poll} (gated, requireWebUser, no secret echoed; already exempt from the Better Auth catch-all). - cli: `archon ai login <provider>` drives the bridge in-process. Unit-tested: bridge state machine (stubbed provider), store oauth read (mocked getOAuthApiKey), delivery (Codex/Pi/Copilot), connect-service oauth, routes, CLI. NEEDS LIVE SMOKE (real OAuth can't be unit-tested): the bridge test confirmed Pi's Anthropic/Codex login() starts a localhost callback server (EADDRINUSE) — on the headless VPS the manual-code path must be the one taken; verify before relying on the Claude/Codex subscription path (Copilot = device-code, no callback server). Also verify the Codex auth.json field names and the Pi oauth backend-id map against one real login. Part of #1891 (PR 3a). * fix(credentials): address PR #1921 review (C1 + I1–I5 + S1–S7 + docs) C1: buildCodexAuthJson mapped wrong field names → empty account_id/id_token. Verified vs pi-ai@0.76.0 (openai-codex.js:100-104,332): the blob is { access, refresh, expires, accountId } (camelCase) with NO id_token. Fix: account_id ← accountId; id_token stays best-effort (Pi doesn't surface one) with a live-verify note. Delivery test now asserts the full tokens object. I1: startOAuth no longer returns a bogus url-less result when login() rejects before the first callback — the .catch resolves firstSignal and start throws (route → 500, CLI → message). I2: oauth resave-on-rotation retries once and logs at ERROR (Anthropic/Codex invalidate the old refresh token, so a failed resave = lockout, not "next read re-refreshes"). Comment corrected. I3: pollOAuth also sweeps expired sessions; a new login per user aborts the prior session (releases its callback server — fixes the fixed-port EADDRINUSE). I4: Pi OAuth error text is sanitized (sanitizeCredentials) + truncated before it reaches a client, and logged via sanitizeError. I5: tests added — inflight coalescing, oauth decrypt-fail + missing-ciphertext, rotation/resave, bridge cancel/early-throw/per-user-abort, route start-500 + poll gate-off-404. S1: rotation check compares access/refresh/expires (not key-order-sensitive JSON). S2: externalMode(session) extracts the pending→manual fallback. S3/S4: comments on the unused poll :provider param and onPrompt/onManualCodeInput. S5: 6 stale PR-marker comments cleaned + the factually-wrong delivery.ts blob comment corrected (the blob is passed through; getOAuthApiKey is keyed by Pi's id, not Archon's). S6: pollLoginLoop uses a pendingCode var. Docs: CLAUDE.md — `ai login`, the /oauth/{start,poll} endpoints + subscriptionAvailable, the catch-all exemption now lists providers*, and @archon/providers/oauth. Deferred: CLI manual-code branch test (needs @Clack mocking; device flow covered) and docs-web ai-assistants.md (nice-to-have). * fix(pi): read ARCHON_PI_AUTH_PATH from per-call env, not process.env Per-user credential delivery injects the per-run auth.json path on the per-call requestOptions.env channel, which the executor deliberately keeps out of process.env (subprocess isolation). The Pi provider read the path from process.env only, so the written auth.json was never loaded and subscription-backed Pi runs failed with "no credentials for provider '<backend>'". Read requestOptions.env first, fall back to process.env for a shell-level override. Found via VPS smoke: a claude->anthropic subscription delivered the auth.json (the claude->anthropic backend map and Pi's getApiKey both verified working in isolation), but Pi never saw the per-run path. API-key Pi backends were unaffected — those already read requestOptions.env. * fix(credentials): gate codex subscription login off (Pi drops id_token) The Codex CLI requires a valid id_token JWT in CODEX_HOME/auth.json, but Pi's openaiCodexOAuthProvider drops the OpenAI id_token from the token exchange, so the delivered auth.json carries an empty id_token and Codex crashes with "invalid ID token format" (verified on the VPS live smoke). Exclude codex from SUBSCRIPTION_PROVIDERS so `ai login codex` and the /oauth/start route refuse it with a clear message; the delivery/refresh code (ARCHON_TO_PI_OAUTH, buildCodexAuthJson) stays intact for re-enable. Gate startOAuth on SUBSCRIPTION_PROVIDERS too (defense in depth, single source of truth). API-key codex (OPENAI_API_KEY / `ai key set codex`) is unaffected. See #1924.
…rride (#1919) * Fix: treat DEFAULT_AI_ASSISTANT as a fallback default, not a hard override (#1171) In Docker, saving "Default Assistant = Claude" in the Web UI Settings persisted the YAML change, but every subsequent loadConfig() call re-applied the DEFAULT_AI_ASSISTANT env var as a hard override and discarded the saved preference. The Settings save appeared to work but was silently masked on every page reload. Changes: - applyEnvOverrides now accepts the raw globalConfig and repoConfig and only applies DEFAULT_AI_ASSISTANT when neither file explicitly set the assistant - loadConfig passes the raw config sources into applyEnvOverrides - Updated the existing test to assert the corrected precedence (config-file wins over env var) and added regression tests for the env-var-as-fallback path and for repo-config precedence over the env var Fixes #1171 * fix: address review findings for DEFAULT_AI_ASSISTANT fallback - Add WARN log when DEFAULT_AI_ASSISTANT has an invalid provider name but a config file is present (typo would otherwise go undetected) - Add test asserting invalid env var is silently ignored when config IS set - Update docs to describe DEFAULT_AI_ASSISTANT as a fallback, not override * simplify: use concise arrow form for pathMatches in new test
…teration (#1923) * Fix: interactive loop resume crashes with error_during_execution (#1208) When resuming a paused interactive loop gate (e.g. archon-piv-loop), iteration 2+ always failed with error_during_execution because needsFreshSession was (loop.fresh_context || i === 1) — but on resume the loop begins at startIteration >= 2, so the condition was never true and the executor blindly passed the stored gate sessionId to the SDK. After hours of human review wait that session is typically expired, which is the failure the issue reporter observed. Changes: - packages/workflows/src/dag-executor.ts: force a fresh session on the first iteration of every interactive loop resume by extending needsFreshSession with (isLoopResume && i === startIteration). Subsequent iterations in the same resume continue to thread currentSessionId from the fresh first iteration, preserving multi-turn continuity. User feedback is already carried via \$LOOP_USER_INPUT so session continuity is not required. - packages/workflows/src/dag-executor.test.ts: update the existing "interactive loop resumes from stored iteration" test to expect an undefined sessionArg (the test was enforcing the buggy behavior). - packages/workflows/src/dag-executor.test.ts: add a regression test that simulates a resumed interactive loop with a stale stored sessionId and asserts the SDK is invoked with undefined and no failure events are emitted. Coordinates with #1291 (fail loudly on SDK isError) and #1294 (orchestrator-side stale-session recovery) without reintroducing any retry loop — we simply avoid passing the stale id. Fixes #1208 * fix: address review findings for #1923 - Broaden comment in dag-executor.ts: remove false implication that the stale-session guard is Claude-only; fix applies to any provider with sessionResume support - Update loop-nodes.md: document that the first iteration after resuming from an interactive loop approval gate is also always fresh, so users relying on fresh_context:false across a gate boundary are not surprised * simplify: remove unused callCount tracking in regression test
…ch detection (#1925) * Fix: cleanup service now reads worktree.baseBranch from .archon/config.yaml (#1419) The cleanup service called getDefaultBranch() three times without first consulting the repo's .archon/config.yaml. On repos that use 'master' (or any non-main default) without origin/HEAD set, this threw env_cleanup_error once per tracked environment on every startup — and the error's own remediation ("Set worktree.baseBranch in .archon/config.yaml") was ineffective because that setting was never read by this service. Changes: - Import loadRepoConfig in cleanup-service.ts - Add private resolveBaseBranch(repoPath, cwd) helper that prefers config.worktree.baseBranch (trimmed) and falls back to getDefaultBranch - Replace the three getDefaultBranch call sites in runScheduledCleanup, getWorktreeStatusBreakdown, and cleanupMergedWorktrees - Mock loadRepoConfig in cleanup-service.test.ts and add four regression tests covering: master via config, whitespace-trimmed config value, fallback when no config, and whitespace-only config value Fixes #1419 * test: harden mock resets in getWorktreeStatusBreakdown and cleanupMergedWorktrees Both describe blocks now call resolveBaseBranch (which calls loadRepoConfig) after the PR fix, but their beforeEach blocks weren't updated to clear and reset mockLoadRepoConfig. Any future test using the sticky mockResolvedValue form in a prior suite would silently bleed in. Added mockLoadRepoConfig.mockClear() and mockLoadRepoConfig.mockResolvedValue({}) to both, matching the pattern already applied to the removeEnvironment and runScheduledCleanup suites. Also updated stale inline comments from "getDefaultBranch returns 'main'" to "resolveBaseBranch returns 'main' (no config → getDefaultBranch fallback)" to reflect the new indirection. * simplify: remove issue reference and historical clause from resolveBaseBranch comment
…base (#1917) * Fix: add /setproject deterministic chat command (#1044) Chat conversations had no explicit, deterministic way to bind to a registered codebase — the orchestrator had to infer the project from natural language. This adds `/setproject <name>` as a deterministic slash command that writes `codebase_id` + `cwd` to the conversation row, completing the chat half of the project-binding primitive (#1886). Changes: - Add resolveCodebaseName helper with 4-tier matching (exact → case-insensitive → prefix → substring, with ambiguity detection) in orchestrator-agent.ts - Add handleSetProject function that resolves a project name and calls updateConversation with codebase_id + cwd - Register 'setproject' in the deterministic commands list and dispatch block in handleMessage - Surface /setproject under the Projects section of /help - Add 8 dispatch tests covering all 4 resolution tiers, not-found (with and without registered projects), ambiguity, and no-args Closes #1044 * fix: address review findings for /setproject command - Rename log event project.setproject_completed → project.set_completed to match the {domain}.{action}_{state} convention used by peer handlers (project.register_completed, project.update_completed, project.remove_completed) - Add per-tier debug logging to resolveCodebaseName checkTier, mirroring the observability pattern in resolveWorkflowName (router.ts) - Add /setproject row to commands reference doc in packages/docs-web * simplify: inline sendMessage cast in /setproject test cases
…1920) * Fix: scoped direct chat must run in repo cwd, not workspaces root (#1179) Direct chat sessions attached to a codebase were spawning the AI provider with ~/.archon/workspaces as the working directory regardless of codebase_id, so file reads, ls, and relative git commands resolved against the wrong directory. The workflow path at orchestrator-agent.ts:721 already used conversation.cwd ?? codebase.default_cwd correctly; the direct-chat path at :1076 ignored codebase_id entirely. Changes: - orchestrator-agent.ts: resolve cwd as conversation.cwd ?? codebase.default_cwd when codebase_id is set (mirroring the workflow path); fall back to ensureArchonWorkspacesPath() for unscoped chats or when the scoped codebase row is missing (with a warn for the deleted-codebase case). - orchestrator-agent.test.ts: update the existing sync test to assert the scoped provider cwd is the repo path, and add a four-case regression suite covering scoped (default_cwd), scoped-with-conversation.cwd, unscoped, and deleted-codebase fallback. Supersedes closed PR #1233. Fixes #1179 * test(core): seed mockListCodebases in discoverAllWorkflows beforeEach Seven tests in the `discoverAllWorkflows — remote sync` suite were silently hitting the deleted-codebase fallback path after the cwd fix in #1179, because `mockListCodebases` was not reset or seeded in their `beforeEach`. Add `mockListCodebases.mockReset()` + a safe `[]` default to the `beforeEach`, then add `mockReturnValueOnce([codebase])` to each of the seven affected tests so they correctly exercise the scoped happy path. Also removes a redundant `mockGetCodebase.mockReturnValueOnce(null)` in the deleted-codebase test — `beforeEach` already provides that default. * simplify: flatten cwd resolution nesting in orchestrator-agent
… 3b) (#1926) * feat(settings): AI Settings UI + config tiers editor + CLI parity (PR 3b) Sectioned console "AI Settings" (Model Tiers / Provider Auth / Defaults), a config tiers read/write API, and full CLI parity (ai tier, ai default). - config: PATCH /api/config/tiers (ungated — works on solo installs) + tiers and tierDefaults on GET /api/config; updateGlobalConfig tiers branch (per-key merge, null unsets); tierEntry/tiersConfig Zod schemas. - cli: ai tier set/list/unset + ai default — write ~/.archon/config.yaml, no credential gate (these are config, not credentials). - console: ModelTiersPanel (tier editor), Provider Auth now offers an API key for every provider plus subscription login for claude/copilot via a manual (paste-code) + device SubscriptionLoginFlow; sectioned SettingsPage. - codex subscription stays API-key-only (gated, #1924); the UI reads subscriptionAvailable so re-enabling needs no UI change. Part of #1891. * fix(settings): address PR #1926 review (I1-I3 + S1-S7) - I1: SubscriptionLoginFlow stashes onDone in a ref and the start/poll effect depends only on [provider] — a parent re-render no longer restarts the OAuth session (dropping the in-flight login / pasted code). - I2: validate tier `effort` against the provider's vocabulary up front (validEffortsForProvider/isEffortValidForProvider) — the route 400s and the CLI exits 1 on an invalid value instead of routePresetEffort silently dropping it at run time. (+ route + CLI tests) - I3: tests for buildTiersUpdate (the form→PATCH OR-blank transform) and the all-tiers-unset → tiers:undefined collapse in updateGlobalConfig. - S1/S2: corrected the `thinking` (accepted-on-read/dropped-on-write + UI-save data-loss) and `tierDefaultsFor` (returns undefined) doc comments. - S3: refreshed the cli/ai.ts file docstring (ai login ships; tier/default are ungated config) and dropped the stale "PR-3b" label in settings.ts. - S4: aiTierListCommand degrades a buildAiProfile throw to {} (aligns with the route's tierDefaultsFor) instead of failing the whole listing. - S5: extracted normalizeOAuthCode to lib/oauth-code.ts (console.warn on parse failure, not silent) with unit tests for all four paste forms. - S6: providerCatalog → registeredProvidersList (parity with siblings). - S7: ModelTiersPanel.onSave guards async setState with cancelledRef. docs-web reference/{api,cli}.md updates deferred to PR-4 (the docs PR).
* fix(web/console): replace 📎 emoji with lucide Paperclip icon (#1928) The 📎 attach button in the console chat composer rendered compressed/ distorted because emoji glyphs are sized by font-size/line-height (not width/height), and the button had no fixed dimensions or leading-none. Swap the raw emoji for the `Paperclip` lucide-react SVG icon, which has intrinsic h-4 w-4 dimensions and renders at a predictable size inside the existing button shell. Changes: - Import Paperclip from lucide-react - Replace 📎 text glyph with <Paperclip className="h-4 w-4" /> in ChatComposer.tsx attach button Fixes #1928 * fix(web/console): enlarge attach icon, align composer controls, add pointer cursor Bump the Paperclip icon from h-4 to h-5 and box both the attach and `/` controls to a fixed 22px height so the icon, slash, and placeholder sit on one line. Add cursor-pointer to the attach button so it shows the hand cursor on hover. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(web/console): update ChatComposer docblock after emoji-to-icon swap Review finding (LOW): docblock still referenced the removed 📎 emoji twice; now describes the lucide Paperclip icon. Review artifacts: ~/.archon/workspaces/coleam00/Archon/artifacts/runs/04cf6dbbb845b1132e1fd6e2c8b9effd/review/ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs: add recommended-workflows design spec Approved design for repo-curated recommended workflows surfaced in the Workflows page grid and sidebar dropdown, declared via recommendedWorkflows in the cloned project's .archon/config.yaml. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(workflows): recommended workflows pinned per project - Add RepoConfig.recommendedWorkflows?: string[] (per-project only; GlobalConfig/MergedConfig untouched) - Parse and sanitize key in loadRepoConfig (defensive: non-array/non-string ignored, debug log only) - GET /api/workflows now returns recommended: string[] (declared order, filtered to discovered names) - WorkflowList pins recommended cards under "Recommended for this project" header + divider; filters still apply - WorkflowInvoker sidebar dropdown renders <optgroup> Recommended / Other workflows when list non-empty - Extract shared partitionWorkflows helper; new tests cover config parse, API ordering/intersection, partition - Docs: configuration.md gains a "Recommended workflows" section - Zero-config safe: absent/empty key → flat list, no behavioral change Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(web/console): pin recommended workflows in the run picker Surface the repo-curated recommendedWorkflows list (PR #1929) in the console DraftRunCard workflow picker: pinned on top in declared order under a 'Recommended for this project' header, divided from the rest. - skills/workflows.ts: listWorkflows now returns { workflows, recommended } - DraftRunCard: order recommended-first, pass names to the picker - WorkflowPicker: render Recommended / Other workflows groups with a divider Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(web/console): extract + cover recommended-workflow ordering; drop design spec - Extract orderWithRecommended() pure helper from DraftRunCard with 5 unit tests (declared order, unknown-name filtering, source sort, empty, no-dupe). - Remove the design spec doc from the PR (not a committed artifact). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): dedupe recommended workflow names in partition helpers Repeated names in a repo's recommendedWorkflows config no longer pin the same workflow twice (duplicate cards / React key collision). Collapse to first occurrence in both partitionWorkflows and orderWithRecommended so all three UI consumers (WorkflowList, WorkflowInvoker, console DraftRunCard) stay safe regardless of input source. Adds dedup test coverage to both. Addresses CodeRabbit review on PR #1929. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…xonomy, deployment shape, activation funnel (#1944) * feat(telemetry): daily archon_active server heartbeat for honest active-install metrics Long-running servers emit archon_started once per boot and then go silent, so a server-only install drops out of DAU/WAU after day one. Add an anonymous archon_active heartbeat fired every 24h from the server entrypoint (unref'd interval, never holds the process open). Same categorical properties and privacy invariants as archon_started — no new data categories, so no first-run notice re-show or schema bump. CLI needs nothing: each invocation already emits archon_started. Docs updated (README + docs-site security/configuration references). * feat(telemetry): schema v3 — chat turns, failure taxonomy, feature adoption, deployment shape, registration counts Closes the four biggest analytics blind spots while keeping the anonymous, categorical-only contract: - chat_turn_handled: one event per direct-chat AI turn (platform + provider + completed/failed; never message content or conversation ids). Workflow and registration routing paths are excluded by construction. - Failure taxonomy on workflow_failed: fixed-enum error_class (fatal/transient/unknown via the existing classifyError) + failed_node_type. Raw error text still never leaves the machine. - Feature-adoption flags on workflow_invoked: uses_output_format, uses_output_type, uses_persist_session, uses_mcp, uses_skills, uses_fresh_context. - Deployment shape on archon_started/archon_active (server): adapter booleans, db kind, web-auth/multi-user flags, GitHub auth mode — gates only, never config values. Distinguishes solo vs team installs. - codebase_registered: pure count at the createCodebase choke point (all three registration surfaces funnel through it) for the activation funnel. TELEMETRY_SCHEMA_VERSION bumped to 3; first-run notice re-shows once (stamp v3) so existing installs re-consent to the broader capture. README + docs-site telemetry sections updated; no-throw tests added. * fix(telemetry): address multi-agent review findings on PR #1944 Important findings — all addressed: - T1: add toTelemetryErrorClass unit tests (all three mappings + round-trip through classifyError) in executor-shared.test.ts - T2: add chat-turn telemetry guard tests — plain conversation captures exactly one completed turn; /invoke-workflow routing turn captures none (with updateConversation auto-attach as positive control); AI error result captures a failed turn - T3: add enabled-path wire-serialization test for deployment shape — gunzips the posthog-node batch body, asserts snake_case keys present, omitted fields absent, and privacy invariants riding on the event - C1: reword captureArchonActive JSDoc — no longer claims 'no schema bump' in a file whose version is 3; clarifies the v3 bump covers the revision - C2: reword firstFailedNodeTaxonomy call-site comment — stored (insertion) order is layer-array order for parallel layers, not completion order - D1: .env.example telemetry block updated to the v3 event/category set - D2: security.md categorical-data paragraph updated (deployment shape, failure class, message-content/conversation-id exclusions) Suggestions adopted: - S1: assert errorClass + failedNodeType in dag-executor failure-telemetry tests - S2: assert the six adoption booleans on workflow_invoked (positive + negative) - S3: default-never exhaustiveness guard in toTelemetryErrorClass - S4: heartbeat comment documenting the sync fire-and-forget contract - S5: ChatTurnProperties doc — why open strings, callers must pass registry ids - S6: dagNodeTelemetryType fallthrough comment (future node types) - S7: no_ai_response early returns documented as intentionally uncounted - S8: module header now lists chat activity + registration count - S9: explicit if-assignment in firstFailedNodeTaxonomy; batch-mode capture comment now cross-references stream mode instead of duplicating it - D3: README adoption list includes fresh-context loops Validation: bun run validate exit 0; all package tests green (145 orchestrator-agent, 226 paths, 44 executor). --------- Co-authored-by: Archon Maintainer Bot <maintainer-implementer@archon.local>
* Fix: snapshot resolved workflow run configuration (#2397) Persisted workflow_started events only recorded the workflow name, forcing observability consumers to reconstruct effective run settings from other records. Changes: - Snapshot resolved assistant, provider, model, isolation, base, user, input, and origin fields - Preserve workflowName and asynchronous persistence behavior - Add focused coverage for precedence, modes, nulls, platform origin, and child runs Fixes #2397 * simplify: reduce complexity in changed files
* Fix: retry Codex availability failures (#2386, #2425) Codex 503 circuit-breaker responses and model-capacity errors were classified as fatal or unknown, bypassing the default transient retry loop. Changes: - Treat generic auth error text as fatal only after concrete transient signals - Classify model-capacity errors as transient - Add regression coverage and update retry precedence documentation Fixes #2386 Fixes #2425 * simplify: reduce complexity in changed files
… form, and installer env-var docs (#2437) * fix(release,install): correct the commit boundary, git pull form, and installer env-var docs Five defects found by running /release and /test-release end to end for v0.7.1. release skill: - Step 4 derived the changelog from `git log main..dev`. main squash-merges, so that range never loses previously-released commits and grows every release: it returned 61 commits for 0.7.1 when 25 were new, and the other 36 were already written into the 0.7.0 changelog. Use dev's last `Release x.y.z` commit as the boundary, exclude the two release-plumbing commit kinds, and cross-check the result against the previous version's recorded PR numbers. - `git pull origin main` aborts with "Need to specify how to reconcile divergent branches" wherever pull.rebase is unset, and would rewrite dev's history where it is set to true. Pass --no-rebase explicitly at all four call sites. - Step 1 had no guard for commits stranded on main. ae704a7 had sat there since 0.7.0; the content was hand-forward-ported via #2403 but the commit never was, so main was not an ancestor of dev. Add an ancestry check that fails closed. - Document that CI's update-homebrew job races the Step 9/10 pushes, and that a clean homebrew/archon.rb merge is not evidence the SHAs are right — both branches edit the same four lines. Verify against the published checksums. - Require flagging a bump that contradicts the range (feat: commits under patch). test-release skill: - The curl-mac snippet set INSTALL_DIR on curl rather than bash, so the installer never saw it, targeted /usr/local/bin, and failed on sudo after the checksum had already verified. - Test 4 asserted a refusal from the env-leak gate. The shipped guard strips the keys and proceeds, so the old criteria reported FAIL against a binary that was working correctly. Assert on the strip line and its key count instead. installer: - scripts/install.sh documented both VERSION= and INSTALL_DIR= on the curl side, so users copying either example silently get the defaults. Mirror in packages/docs-web/public/install updated in lockstep (test-install.sh enforces byte-equality). install.ps1 is unaffected. Refs #2436. * fix(release,test-release): address CodeRabbit review on #2437 Applied 7 of 8 findings. Testing the fixes surfaced two further bugs in the first pass that the review had not flagged. Found while testing this commit (not in the review): - The boundary grep `^Release [0-9]` matched main's SQUASH commit (`Release 0.7.1 (#2435)`) in preference to dev's own version-bump commit (`Release 0.7.1`). The squash commit's ancestry excludes dev's individual history, so the range expanded to 383 commits — the same class of failure this whole change exists to prevent. Anchored with `$` so only the bare subject matches. - `^chore(homebrew):` under --extended-regexp parses the parens as a group and matches the literal `chorehomebrew:`, silently filtering nothing. Escaped. From the review: - Implement the first-release fallback that the prose already promised. Verified that an empty LAST_RELEASE yields `..origin/dev`, which resolves against HEAD and returns zero commits — a false "nothing to release". - Move the release/homebrew plumbing exclusions into the git log invocation instead of leaving them as prose the reader must apply by hand. Anchored the Release pattern to a full x.y.z so a feature commit starting with that word is not swallowed. - Derive the previous changelog heading from CURRENT_VERSION rather than hard-coding 0.7.0/0.6, which would have rotted at 0.7.2. - Make the Step 9 and Step 10 convergence checks exit non-zero. Previously they printed divergence and continued into formula and tap publication. - Replace the formula SHA check. `grep -q "$digest" formula` only asked whether a value appeared anywhere, so swapped platform digests both passed, and an empty extraction degenerated to `grep -q ""` which matches any file. Now parses the sha256 following each platform's own URL, requires a 64-char hex digest, and compares exact values. - Assert the formula version equals the release instead of printing it. - Pin the env-leak assertion to the test repo's resolved path and an exact key count of 1. The prior regex accepted any positive count from any path. Declined: - MD028 blank-lines-in-blockquote. Nothing lints markdown in CI or package.json, and the blank lines separate distinct warnings that read worse merged. Verification: - Corrected Step 4 reproduces the real 0.7.1 range: boundary resolves to 6c6945c, and 0.7.0..0.7.1 yields 26 commits against the 25 hand-derived for the shipped changelog. The extra is ae704a7, which genuinely entered dev via the mid-release resync and is then excluded editorially as internal docs. - New formula check passes on the real v0.7.1 formula and, in a negative test with two digests swapped, fails on both — the case the old check missed. - Exact-match env-leak assertion verified against a live binary run; the `pwd -P` resolution is required because the binary logs /private/tmp where the test path is /tmp.
…ently dropped at parse (#2459) * test(workflows): guard against workflow-level schema fields being silently dropped at parse parseWorkflow hand-assembles its result field by field, so a field added to workflowDefinitionSchema but not to that object literal is silently discarded: the YAML parses, the workflow loads, and the feature is inert. That already happened. `requires:` landed in workflowBaseSchema in ab81248 (2026-06-01) without touching the loader, and the assembly block only arrived in 2d7bf58 (2026-07-16) — six weeks in which the GitHub capability gate could never fire for a discovered workflow, fixed incidentally inside an unrelated PR. This is the third instance of one pattern: parallel enumerations that must agree with nothing enforcing agreement. The ref-surface enumerations carry a KEEP IN SYNC comment and were found broken anyway (#2450); the nested key sets are derived from each schema's .shape and cannot drift (#2455). This applies the derived form to the second case. The field list comes from workflowDefinitionSchema.shape, so a new schema field fails the test until it is given a fixture. Deliberately not solved by deriving the assembly itself — the hand assembly exists because of warn-and-drop, and schema.parse() would reject a bad field instead of logging and dropping it. The per-field assertion clears the mock logger first so it can tell the two failure causes apart: a warning means the fixture value is invalid (warn-and-drop working as designed), silence means a valid field was dropped (the actual bug). Verified by breaking it both ways: removing `requires` from the object literal reproduces the historical bug and fails with the right diagnosis, and adding a new schema key fails the ratchet until a fixture exists. * test(workflows): tighten the parity guard after review Addresses I1, I2, I3 and S1, S3, S4 from the review on #2459. No change to what the guard catches; all six make a precision tool more precise. I1 — the docblock claimed warn-and-drop universally. Re-verified the field audit against loader.ts rather than taking it on faith: 4 of the 20 hard-reject (name, description, nodes, evidence_policy at :619-629), 13 warn-and-drop, and 3 coerce silently with no log at all (provider :423, model :425, persist_sessions :473 — there is no invalid_provider/invalid_model/invalid_persist_sessions warn event anywhere in the file). Rewritten to say most rather than all, and to point at loader.ts as the authority instead of restating a per-field table that would rot the moment a field changes category. I3 — the two-branch failure message was backwards for exactly those 3 silent fields: a bad `provider: 123` fixture is discarded with no warning, so the message confidently blamed the loader and sent the reader into parseWorkflow when the fixture was at fault. That is the same failure the message exists to prevent, and the one I hit during development with a bad `thinking: true` fixture. Fixed by ranking rather than verdict: a warning is still strong evidence the fixture is wrong, but silence now names both causes and points at the fixture first. Chosen over listing the three exceptions in a comment, which would duplicate loader.ts and rot. This subsumes S2's unstated-invariant concern. I2 — effort, thinking and sandbox used presence checks where the other 17 fixtures check values, and their schemas transform deterministically, so exact checks are available. Verified by mutation: returning effort:'low' and thinking:{type:'disabled'} from the loader now fails both round-trips, where before it left them green. S1 — the hand-assembly literal predates 2d7bf58; only the requires entry landed there. Reworded so it cannot be skimmed as "the mechanism didn't exist until then". S3 — the two diagnostic strings moved out of the assertion into a named message. S4 — nodes?.length, so a dropped nodes yields a clean false instead of a TypeError. Verified: full validate green (132 batches, 0 fail); the I3 message re-checked by running a deliberately invalid provider fixture; I2 re-checked by mutation.
…mocks can intercept it (#2461) * fix(workflows): resolve the include-expander logger per call so test mocks can intercept it The lazy logger in include-expander.ts carries a comment saying the deferral exists "so test mocks can intercept createLogger". The module-level cache defeats that: it only delivers for whichever mock is installed at the FIRST call. Bun's mock.module is process-wide and irreversible, so once another test file in the same process warms the cache, a later mock.module('@archon/paths') cannot intercept these warns. The cost was three red tests in loader.test.ts whenever it shared a bun test process with include-expander.test.ts — in either order, and specific to that pair. CI never saw it because package.json happened to run the two files in different batches, so the protection was an accident of arrangement rather than a guard. What it cost was developer time: the two files are a natural pair to run together while working on include expansion, and the failure gives no hint about its cause. Resolving per call costs one rootLogger.child(). Both call sites are warn-only paths reached once per include node at discovery, never in a hot loop. The two files now share a batch, so CI enforces the fix instead of avoiding the collision. Verified by reverting the fix on the new batch: it goes red with exactly the three original failures, and green again with it. Scope: 108 files carry the same cachedLog pattern. Only this one has a demonstrated failure, and a blanket change would be a 108-file patch with no test proving the others were ever broken. Fixed where it bites; the comment records why. * docs(workflows): correct the call-site frequency note on the include-expander logger The second getLog() site sits inside the per-child-node loop and fires once per unresolved command node, not once per include node. Warn-only discovery path either way, so the conclusion is unchanged.
…seable output (#2460) * fix(workflows): stop a declared output_format from silencing an unparseable output resolveNodeOutputField treated "no parseable object at all" as a declared-optional field and returned empty, but only on the declared-schema path — the schemaless path threw. So declaring output_format made a broken producer QUIETER than declaring nothing, which is backwards. It bites hardest on workflow: sub-run nodes. Their output_format is never validated against the child; executeWorkflowNode uses it for one thing, deriving declaredFields. A child that returns prose instead of JSON therefore turned every declared field into '' with no error, no warning and no failing test, while the same child under a node with no output_format failed loudly. Now both paths throw 'unparseable'. The leniency that was actually intended survives untouched: a declared field missing from a payload that genuinely parsed still resolves to '' — this only changes the case where there is no object to read. AI nodes are unaffected: when structured output validates, nodeOutputText is overwritten with the serialized JSON, so a completed AI node persists JSON and the resume path (which rehydrates text only) still parses it. This fixes the inversion, not the absence of validation — a child emitting JSON that does not match the declared schema still passes. That half stays open. * fix: apply CodeRabbit auto-fixes Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai> * docs(workflows): correct when an undeclared output field surfaces The variables reference said an undeclared $node.output.field was a 'load-visible mistake'. The loader never validates field names against output_format — loader.ts has no reference to output_format, declaredFields, or not-in-schema. The 'not-in-schema' error is raised by resolveNodeOutputField at execution and fails the consuming node, like every other OutputRefError reason. The mechanism described was right; only the timing was wrong. Authors reading this page need to know they find out mid-run, not at load. * fix(workflows): tell a clipped output apart from a producer that emitted no JSON The unparseable error told the author to 'Emit JSON containing <field>'. For output clipped at the persisted-event cap that advice is wrong — the node already did, and only a resumed run sees the clipped copy. output_format sits on dagNodeBaseSchema, so a bash node can declare one, and bash stdout is exactly what formatPersistedBashOutput clips at 32 KiB. In-run the full stdout is returned and parses; getDagResumeSnapshot rehydrates the clipped copy, which does not. Before #2456 that resolved to '' on the declared path, so nobody saw it. Now it fails, and it should fail with advice that points somewhere real. Adds an OutputRefError reason 'truncated', chosen at both throw sites so the declared and schemaless paths stay symmetric — the property this PR exists to establish. The marker moves to utils/output-truncation.ts so the writer (dag-executor) and the reader (output-ref) cannot drift; the string is byte-identical and nothing about what gets persisted changes. The detector is anchored to end-of-string, so output that merely quotes the phrase is still reported as a plain producer error. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
CLAUDE.md is the highest-trust file in the repo — every agent reads it and none
re-derives it — so a stale claim fails in the "looks fine" direction.
Four claims verified false on dev:
validate steps nine -> ten (gained test:install)
@archon/core 20 -> 33 test batches
@archon/workflows 5 -> 18
@archon/isolation 3 -> 5
ADD COLUMN count 36 -> 39
isolation docker/ packages/isolation/src/docker/
-> packages/isolation/docker/, a sibling of src/
The counts are replaced with pointers to where the truth lives rather than with
fresh numbers. Every one of them was correct when written; restating today's
figure only resets the clock, and nothing tells the next person who splits a test
batch to come back here. The load-bearing rules are untouched: split conflicting
mock.module() calls into separate invocations, and every ADD COLUMN ... NOT NULL
carries a DEFAULT.
The path is a straight correction — the runner image sits beside src/, not in it.
Net 153 characters smaller. No code changes; docs only.
…report better (#2464) * docs: cut CLAUDE.md blocks that transcribe what the tools already report Second pass on the accuracy audit. The first fixed false claims; this removes the blocks that generate them — 1008 lines to 690, and 456 fenced lines to 128. The test applied to each block: does a reader learn something here they could not get faster by running the thing? Five failed it, and each was checked against the docs site and the code before being cut. CLI command dump (136 lines) -> the three rules --help omits `archon --help` and reference/cli.md (640 lines) are both more current than this block already was: it never mentioned chat, setup, workflow search/install, continue, or doctor --full. What survives is what neither tells you — the git-repo requirement, isolation-by-default, and the --json approve/reject/resume semantics. packages/ tree (99 lines) -> package roots in dependency order Every dependency constraint it encoded is already stated, more precisely, in Package Split a hundred lines below. Order verified against each package.json: every package depends only on those above it. Inside a package, ls and the file docblocks outlive any tree drawn here. assistant defaults yaml (38) -> a paragraph reference/configuration.md carries the full key set. Kept the two keys worth knowing before you look: the binary paths, and settingSources. ~/.archon tree (20) -> a paragraph reference/archon-directories.md carries the layout. Kept the rule that matters: artifacts and logs live outside the repo and are never committed. The repo-level .archon/ tree stays — eight lines, referenced constantly. SDK type patterns, import patterns (40) -> rule lines Paired correct/wrong examples where the rule is one sentence. Also corrects a claim the #2459 review surfaced: the sqlite.test.ts parity test no longer compares table names only. It checks columns in both directions against tracked allowlists, so a column added to one dialect and forgotten in the other now fails CI. Kept deliberately: the logging idiom and the error-handling patterns, which encode exact shapes meant to be copied, and every behavioural section — Core Principles, Engineering Principles and Product Direction are byte-identical. Verified no section was lost: the heading list is unchanged. * docs: cut the remaining CLAUDE.md blocks that restate their own prose Second half of the shrink pass, under the inverted default: cut when in doubt, and justify what stays rather than what goes. logging (24 lines) -> two sentences The "Event naming rules" bullets immediately below already state the convention the block illustrated. What survives is the call shape — structured object first, event name second — and the three fields an error log must carry. error handling (29) -> two sentences Both blocks were generic try/catch. The git one's actual content was classifyIsolationError, and the sentence under it already said so. Kept the rule that matters: log the raw error AND send the classified message, because doing one of the two is the bug the pattern exists to prevent. running in worktrees (22) -> one sentence A transcript of `bun dev` plus three curl calls. The port rule is the content and it is stated in prose right below; the API calls are in the API Endpoints section. dev / testing / type-check / validate command blocks (35) -> four lines Script-name lists that package.json reports. Kept the ports (3090, 5173) and the generate:types ordering constraint, neither of which a script name tells you. Retained blocks and why each survives: packages/ root list (12) — dependency ORDER is the rule, and `ls` cannot tell you which package may depend on which. Verified against every package.json. repo-level .archon/ tree (6) — the canonical statement of what Archon reads from a repository, with no docs-site equivalent that is more current. CLAUDE.md is now 574 lines from 1008, and 22 fenced lines from 456. Section headings unchanged; Core Principles, Engineering Principles and Product Direction remain byte-identical.
…odes (slice-2 PR-A) (#2223) * feat(workflows): per-child worktree isolation for workflow: sub-run nodes Slice-2 PR-A. A `workflow:` sub-run node can now run its child in its own git worktree via `isolation: worktree` instead of sharing the parent's checkout — the foundation for parallel fan-out (PR-C), where N children writing into one checkout would hit the worktree-in-use collision. - Add the ChildIsolationResolver structural port in @archon/workflows (child-isolation.ts), mirroring the container write-back port: it imports only local types, so @archon/workflows keeps zero @archon/isolation deps. - Inject it via ExecuteWorkflowOptions.resolveChildIsolation. runChildWorkflow calls the port per child at spawn time when node.isolation === 'worktree', uses the returned cwd for the child's working_path + execution, and fails the node fast when no resolver is wired (never a silent shared-checkout fallback). Resume reuses the child's own recorded worktree. - Lift the load-time superRefine rejection of isolation: 'worktree' on workflow nodes; widen WorkflowNode.isolation to enum(inherit|worktree). - Add createChildWorktreeResolver in @archon/core (which already depends on both @archon/workflows and @archon/isolation) and wire it into the CLI and orchestrator dispatch/resume/background call sites. Git-repo codebases only — folder projects have no resolver, so the node fails fast. Refs #2121 (Phase 2, slice-2 plan PR-A), #1764. PR-C/D stack on this branch. * fix(workflows): address PR review on per-child worktree isolation Review round on #2223 (5 Important + 3 Suggestions). Each item: - I1: thread resolveChildIsolation into all three childOpts constructions in runChildWorkflow so a nested grandchild `workflow:` node also isolates (nesting is first-class up to the depth cap); previously it always fail-fasted "requires an injected resolver". One-line note that the sibling `container:` context has the same non-propagation gap (out of scope here). New 2-level nested isolation test. - I2: existsSync guard on resumeFailedChild.working_path before reuse — a child worktree pruned by `isolation cleanup` between failure and resume now fails with the CLI-precedent "worktree may have been cleaned up" message instead of a deep ENOENT. New pruned-worktree resume test. - I3: route resolver failures through classifyIsolationError() (in the resolver, the layer that may import @archon/isolation); the executor keeps the sub-run context prefix. - I4: paired workflow.child_worktree_create_failed Pino error line on the resolver throw path (was only the _created info line). - I5: resolver-throws test — node fails cleanly, error carries the message, no orphan child row. - S1: superRefine rejects `isolation:` on non-workflow node types (mirrors the `with:` guard) + loader test. - S2: assert req.codebaseId === config.codebaseId in the resolver, making the "resolver guard" doc true rather than fixing the comment. - S3: stamp isolation_env_id + branch_name into the child run metadata at spawn (mirrors the container path) for PR-E console grouping. Also completes the four core @archon/isolation test mocks with classifyIsolationError (now imported transitively via the resolver). Refs #2121 (slice-2 PR-A), #1764. * docs(workflows): document sub-run isolation and the shared-checkout rules `isolation:` on a `workflow:` node is new YAML surface and had no page, and `mutates_checkout:` — the field the shared-checkout story depends on — had never been documented at all. authoring-workflows.md, in the existing sub-run section: - "Choosing the child's checkout with `isolation:`" — the three values, the rule that the engine never infers isolation, and what a child worktree actually costs: it branches from the repo base branch (so it does NOT see the parent's uncommitted work), nothing merges it back, it registers a tracked environment that `isolation list`/`cleanup`/`complete` then govern and that outlives the run, and resume reuses it rather than re-creating it. - "When a worktree can't be created" — the exact fail-fast message, the two causes (folder project, no resolved codebase), and the fact that `archon validate workflows` cannot catch either, since resolver availability is a property of the run and not of the file. - "Running sub-runs side by side" — the path lock, why siblings collide on a shared checkout, the warning that resume does not recover a lock-cancelled child, and `mutates_checkout: false` as the supported way for read-only children to coexist. Adds `mutates_checkout` to the workflow-level schema block and `isolation` to the node-fields table. - Drops the stale "isolation: worktree is reserved and rejected at load time" bullet and the matching non-goal. book/isolation.md gets a short note so an `archon/task-…-child-0` branch in `isolation list` is recognisable, with the resume caveat. reference/cli.md notes child worktrees under `isolation list`. The constitution's case-law table records the sub-run row as shipped and adds a rejected row for isolation inferred from an unrelated field — the decision that governs this surface and the fan-out one behind it. * fix(workflows): make the child worktree identifier unique per sub-run node The identifier was `<parentRunId8>-child-<childIndex>`. It omitted the node id, and the executor never passes childIndex, so every isolated child of a given parent computed the same string. Two `workflow:` nodes with `isolation: worktree` in one parent run resolved to the identical branch and worktree path. Nothing failed. WorktreeProvider.create() finds the existing worktree, verifyWorktreeOwnership passes (same canonical repo), and it ADOPTS it — an INFO worktree_adopted line, no error. So: - two children shared a checkout the author explicitly isolated - a second isolation_environments row pointed at the same path, so `complete <branch>` on either removed the worktree out from under the other - same-layer nodes landed on the same working_path, where the sibling path-lock cancels one — #2180 Defect A, in the case where the author got it right, and not recoverable by resume Extract buildChildIdentifier(parentRunId, nodeId, childIndex) producing `<parentRunId8>-<nodeSlug<=16>-<sha256(nodeId)[0..8]>-child-<n>`. Interpolating the node id verbatim is not enough: WorktreeProvider.slugify() truncates at 50 chars, so two long node ids sharing a prefix collide exactly as before, and a long enough node id truncates `-child-<n>` away, which would collide fan-out siblings as soon as #2224 passes a real childIndex. Bounding the readable part and carrying the full node id in a hash keeps the branch legible and the key as fine-grained as the thing it names. Worst case (69-char node id, 4-digit index) is 45 chars. The slug is also trimmed of a trailing separator, since truncating at 16 can land mid-separator and leave a `--` that the provider's slugify collapses — which would make the stored workflow_id differ from the branch derived from it. Tests: seven over buildChildIdentifier (two nodes in one parent, long node ids sharing a 30-char prefix, fan-out siblings, distinct parents, worst-case slug cap, determinism for resume, slug-shape invariant) plus one resolver test asserting two resolve() calls with different node ids reach provider.create with different identifiers. Reverting the identifier fails three of them; dropping only the separator trim fails the slug-shape one. Docs updated to the new branch shape in the three places that named it, plus the two stale docstrings. Refs #2121 (slice-2 PR-A), #2180. * fix(workflows): stop silent worktree adoption on the child-isolation path WorktreeProvider.create() returns an existing worktree at the computed path instead of failing. That is what made the identifier collision invisible, so it should not stay quiet now that the identifier is fixed. Adoption is kept rather than rejected. Resolution only runs on a fresh child spawn — resume reuses the child's recorded working_path and never re-resolves — so the only worktree that can be sitting at a per-(parent, node, index) path is this same child slot's own from an earlier attempt. The reachable case is a spawn that created the worktree and then died before its run row was written: the parent's resume finds no child row, spawns fresh, recomputes the same identifier, and re-uses it. Rejecting would wedge that resume permanently. A sibling's checkout is no longer reachable at all. So adoption is now recorded instead of hidden: - WARN workflow.child_worktree_adopted with the parent run, node, child index, branch and path. The provider's own INFO line doesn't know it is looking at a sub-run child and carries no parent/node context. - `adopted` on the isolation_environments row and on the existing child_worktree_created line. The row is the durable half — a log line is gone by the time anyone asks why two runs touched one checkout. If this ever fires for a sibling, the identifier has regressed and these are the evidence. No change to IsolationRequest or WorktreeProvider — the resolver reads the metadata.adopted discriminant the provider already returns, so the other callers of the adoption path are untouched. Tests: a re-used worktree records adopted: true, a fresh one records false. Both confirmed failing without the change. Refs #2121 (slice-2 PR-A). * fix(workflows): thread the child-isolation resolver through the parent auto-resume maybeResumeParentRun re-enters executeWorkflow when a sub-run child reaches a terminal state, but it built the options bag from the hydrated resume state alone. resolveChildIsolation was dropped there, so a parent that paused at a child's approval gate and then auto-resumed could not spawn ANY subsequent isolated child: the node failed with "requires an injected child-isolation resolver" on a git repo, via the CLI, with the resolver correctly wired at the top. That breaks the shape the docs recommend — isolated implement, gate, isolated review — which is the regression test added here. It drives the real recursion: the first node's child pauses at its gate, the child is approved and resumed in its own worktree, its completion fires the parent auto-resume hook in-process, and the parent's second isolated node must get its own worktree. The assertion is the resolver call sequence plus the parent completing with two distinct child checkouts, not "nothing threw" — the failure mode is success-shaped otherwise. Confirmed failing before the fix (resolver calls: ['implement'], expected ['implement', 'review']). The resolver is a plain parameter rather than part of ResumePayload: that union carries what was RECORDED about the prior run, and a resolver is a live capability of the surface driving the process — it cannot be rehydrated from a run row. Forwarding the child's own resolver is correct because a child inherits the parent's codebase_id and the resolver is codebase-bound, rejecting a mismatch loudly. Also corrects two comments in the same path that claimed resume never re-resolves. It can: the re-entry looks the child up by (parent_run_id, parent_node_id), and a resume whose child row is gone takes the fresh-spawn path. * fix(core): keep the child worktree identifier slug-shaped when the node slug is empty buildChildIdentifier trimmed a trailing separator off the truncated node slug but nothing handled the slug being empty outright. `id: z.string()` carries no pattern, so `###`, `___`, `日本語` and `🚀` are all node ids that load today, and each slugifies to nothing — producing `3f9a1c2b--56dc6d47-child-0`. The provider's own slugify collapses that `--`, so the stored isolation_environments.workflow_id stops being byte-identical to the branch derived from it, and an env row is no longer findable from its own branch name. Cosmetic in impact, but it is a stated invariant of this function and the slug-shape test claimed to cover it. Segments are now joined with empties dropped instead of interpolated. The hash of the full node id is unconditional, so the four inputs above stay distinct from each other — dropping the readable part costs legibility, not uniqueness. The four inputs are added to the existing slug-shape test, plus a test pinning uniqueness and the absence of a doubled separator. Both fail before the change. * docs(core): give the real reason child worktree adoption is safe The adoption-safety comment argued that a sibling's checkout is unreachable here because resolution only happens on a fresh spawn and resume never re-resolves. The conclusion is right; the reason is not. resolve() IS reachable on resume: the parent's re-entry finds its child by (parent_run_id, parent_node_id), and when that row was never written — or was deleted — the node takes the fresh-spawn path and resolves again. An adversarial probe hit exactly that. The two properties that actually make adoption safe are named instead, because both are load-bearing and one of them is invisible: 1. buildChildIdentifier is deterministic in (parentRunId, nodeId, childIndex), so a re-spawn of the same slot recomputes the same path — whatever is there is that slot's own from an earlier attempt, never a sibling's. 2. isolationDb.create() is an UPSERT (ON CONFLICT ... DO UPDATE), so the re-spawn refreshes the existing env row rather than failing on the unique index. Nothing said so, and "simplifying" it to a plain INSERT would break precisely the recovery the comment claims to protect. Comment-only; no behaviour change. * docs(workflows): record that --base does not reach sub-run children createChildWorktreeResolver passes only baseBranch to WorktreeProvider.create — never baseOverride or fromBranch, unlike the top-level CLI path. So a child worktree is cut from origin/<base> of the CANONICAL repo using levels 2-4 of the base-branch precedence table only, and `archon workflow run parent --base release/2.0` still branches every isolated child off the repo's configured base. Nothing said so anywhere. Three corrections while verifying the surrounding text against the code: - the fallback chain runs worktree.baseBranch (repo config) > the codebase's stored default branch > git auto-detection. The guide named only the first two; worktree.baseBranch outranking the codebase default holds for children too, since the provider reads it from the canonical repo's own config. - "does not see the parent's uncommitted edits" understated it. Cut from origin/<base>, a child sees neither uncommitted NOR committed parent work. - "Resuming the parent never re-creates a child's worktree" is only true while the child's run row survives; if that row is gone the node spawns fresh. It lands on the same branch name either way, which is why this is safe. The precedence table in the CLI reference now says what its scope is, since a reader arriving there would otherwise assume level 1 applies everywhere. * fix(core,workflows): configure isolation from the child resolver, and close the round-3 findings M1 — a child worktree could be created by a process that never called configureIsolation(), silently dropping the repo's entire `worktree:` block. The CLI configures isolation inside `else if (wantsIsolation && codebase)` and the orchestrator only via getResolver(); both are top-level-worktree paths. But `--no-worktree` skips them, and that is the exact shape the authoring guide endorses — "a parent started with --no-worktree can still hand an isolated child its own worktree". On that path the provider falls back to the factory's no-op loader, so `baseBranch`, `path`, `remote` and `copyFiles` are all ignored. `copyFiles` is the one that bites: files the repo seeds into worktrees (.env and friends) never arrive and the child's build fails for no visible reason. Verified by running rather than reading. Against a real temp repo with `worktree.copyFiles: ['.env']`: unconfigured process → .env absent from the child worktree; configured → present. No error either way. This is the PR's to fix rather than a pre-existing gap: on dev the CLI's only getIsolationProvider() call sits inside the configured branch, so `--no-worktree` created no worktree anywhere in the process and the no-op loader was unreachable. This PR is what makes worktree creation reachable from a process that skipped it. Fixed at the resolver factory rather than at the two call sites the review proposed. The resolver is the thing that needs a configured provider, so binding it there covers all three current construction sites and any added later, and is a smaller diff than patching CLI and orchestrator separately. configureIsolation is idempotent — it swaps the loader and drops the lazily-rebuilt singleton — and all three loaders are the same function. M2 — two docs stated the opposite of what ships. CLAUDE.md said `worktree` was "reserved/rejected"; it is accepted (dag-node.ts:485). book/quick-reference.md said the same. CLAUDE.md is loaded into context by every agent working in this repo, so it was actively steering the next implementer — and #2224 builds on this exact field. S1 — the resolver's catch block and codebase-mismatch guard had zero coverage. Both are this PR's own new code, and every existing "resolver throws" test uses a hand-rolled fake, so a refactor dropping the classifyIsolationError call or renaming the paired _failed event would have passed CI. The isolation mock now returns a distinctive prefix so the test proves the call rather than the shape. S3 — `childCwd = priorPath ?? cwd` was the last silent shared-checkout fallback. `working_path` is nullable; for a child the author isolated on purpose, falling back to the parent's checkout is precisely the collision the isolation was asked to prevent. Unreachable today, so this is defense-in-depth — but this PR is what gave the fallback a dangerous meaning, since before it every child shared the parent's checkout anyway. S4 — RunChildWorkflowArgs.isolation hand-duplicated the Zod enum, against the repo's derive-never-hand-craft rule. Now WorkflowNode['isolation']. S5 — reference/database.md claimed a sub-run always shares its parent's checkout, and that the path-lock excludes only the ancestor chain. It excludes descendants too (executor.ts:1296). Tests: M1 and S3 confirmed failing before their fixes. S1's two are coverage pins on existing behaviour, so they pass either way by design. * fix(core): expose loadRepoConfig from every config-loader test mock CI (ubuntu) failed on the previous commit with: SyntaxError: Export named 'loadRepoConfig' not found in module packages/core/src/config/config-loader.ts M1 added `import { loadRepoConfig }` to child-isolation-resolver.ts. Three test files mock '../config/config-loader' with factories that export only loadConfig, and mock.module replaces the module process-wide — so when the resolver is evaluated AFTER one of those factories registers, its named import has nothing to bind to and fails at module-eval, not at call time. This is the documented hazard in CLAUDE.md ("when you add an export to a module, grep for mock.module('<that module>' and update every factory"); it applies just as much to adding a new *importer* of an existing export. Three other factories already stub loadRepoConfig for exactly this reason — orchestrator-agent, orchestrator-isolation, cleanup-service — so the remedy has precedent in-tree. Audited every config-loader factory rather than only the one CI named: store-adapter (the failure), clone, and orchestrator all lacked it. orchestrator would have failed the same way once its batch ran, since orchestrator.ts imports createChildWorktreeResolver. Not reproducible locally: on macOS the resolver module is evaluated before the mock registers and the import binds against the real module, so `bun test src/workflows/` passes either way. Linux orders it the other way. Verification is CI, not the local run — flagging that rather than implying local green proves it. --------- Co-authored-by: Archon Maintainer Bot <maintainer-implementer@archon.local>
… PR-C) (#2224) * feat(workflows): add mapWithLimit bounded-concurrency helper A pure sliding-window pool (start cputime unlimited filesize unlimited datasize unlimited stacksize 7MB coredumpsize 0kB addressspace unlimited memorylocked unlimited maxproc 2666 descriptors 1048576, refill on settle) returning results in input order as PromiseSettledResult, so one rejection never aborts the rest. Backs the fan-out concurrency cap — the top-level DAG layer loop is an unbounded Promise.allSettled, which a runtime-length fan-out must not inherit (runaway N-wide layer, #1961). * feat(workflows): add fan_out schema + load-time validation Adds fan_out: { items, as?, max_parallel, join } to workflow (sub-run) nodes. items is a $node.output ref to a JSON array (data — the child target stays a static name per the constitution); max_parallel defaults to 5; join is all_success (default) | all_done, with first_success staged in the enum but rejected fail-fast as PR-D. A fan_out node defaults isolation to worktree — its N children would otherwise collide on the shared parent checkout and the run-in-progress path lock. Loader scans fan_out.items in the 3-surface ref check and requires its producer to be an upstream dependency (else it races). The include expander namespaces fan_out.items refs inside inlined blocks. fan_out on a non-workflow node, and inside a loop_group body, are rejected. * feat(workflows): execute dynamic fan-out with joins + 1:N resume executeWorkflowNode branches to a fan-out path when node.fan_out is set: resolve items to a JSON array (fail closed on non-array/malformed; empty array is a valid zero-width expansion), spawn one governed child per item through mapWithLimit(max_parallel), and reduce the N outcomes via join. all_success fails the node on any child failure (later spawns skipped fail-fast); all_done aggregates all terminal children, representing failed entries as { error, status }. The aggregate is a JSON array in item order threaded as $node.output; cost/tokens sum onto the node's single result. Generalizes the slice-1 1:1 sub-run re-entry table to 1:N keyed by metadata.child_index (stamped in runChildWorkflow), so parent resume skips completed instances and re-drives only the failed ones. Per #2180, a fan-out child that pauses at a gate fails the node with an autonomous-fan-out pointer and is cancelled — the single parent gate slot can't hold N children; a non-fan-out workflow node keeps full slice-1 gate support. * test(workflows): cover dynamic fan-out end-to-end subrun.test.ts (real executor recursion): N-item all_success spawns N children in their own worktrees with an ordered aggregate and item to $ARGUMENTS; empty items completes with []; malformed items fails closed; all_success serial fail-fast leaves later items unspawned; all_done keeps a partial failure; max_parallel bounds concurrency; child cost rolls up; parent resume re-drives only the failed instance; a paused fan-out child fails the node (#2180) and is cancelled. Plus loader cases (fan_out accepted/defaults, dangling + non-dependency items refs, non-workflow + loop_group-body + first_success + max_parallel:0 rejections) and an include-expander namespacing case. * fix(workflows): reconcile fan-out #2180 gate path (review round) Addresses the review of the fan-out reconciliation path: - C1: split the blocked pre-check by status. A genuinely gate-'paused' child keeps the autonomous-fan-out failure (cancel + gate message); an ambiguous 'running'/'pending' child (crash-orphan vs a live run in another process) is NO LONGER auto-cancelled — the node fails WITHOUT mutating it (CLAUDE.md lifecycle rule), surfacing a last_activity_at staleness-keyed wait/abandon action. The gate message is never shown for a non-gate cause. - C2: fan-out auto-cancellations stamp metadata.cancelled_reason (fan_out_gate/fan_out_sibling/fan_out_orphan). On resume a fan-out-tagged cancel is recovered (flipped to failed → re-driven) so 'remove the gate, then resume' actually works; a user's out-of-band cancel (untagged) stays terminal and is never resurrected. - I1: when the node's fate is sealed (a pause, or an all_success failure), in-flight siblings are cooperatively cancelled (tagged fan_out_sibling, re-queried live since runChild is synchronous) so they stop at their cancel poll instead of burning a full turn. - I2: an out-of-range child_index (items shrank) is warned + a live orphan cancelled (tagged) instead of silently dropped. - I3: restore the subrun_completed_without_output warn in both join reducers. - I4: gate message names the offending child index + run id; the cancelled row records why. - S1/S2/S4: doc callout on items.length vs MAX_CASCADE_RUNS; an item-content hash in child metadata warns on a non-deterministic producer on resume; a duplicate child_index is debug-logged. S3: notify the platform immediately on the items-resolution and #2180 failure paths. * test(workflows): cover fan-out reconciliation fixes - C1: a running child found on resume fails the node WITHOUT cancelling it (surfaces a 'may still be running' wait/abandon message, not the gate one). - C2: a fan-out-cancelled gate child re-drives on resume once the gate is removed (same row, node completes); a user-cancelled (untagged) child stays cancelled and is never resurrected. - I1: an early failure cooperatively cancels the in-flight siblings (tagged fan_out_sibling) — a slow-sibling / instant-fail workflow makes the in-flight window real. Mirrors the real store's completeWorkflowRun CAS guard in the in-memory harness so a mid-flight cancel sticks. - I2: an out-of-range child_index (shrunk items) is cancelled + tagged. - Strengthen the paused-gate test to assert the fan_out_gate tag + the enriched (index + run id) message. The gate test uses max_parallel: 1 so the single gate observation is deterministic (concurrent gate children get sibling-cancelled by I1, which is correct but timing-dependent). * fix(workflows): make sub-run isolation explicit-only, and catch the fan-out collision The schema injected `isolation: 'worktree'` whenever a `workflow:` node carried `fan_out` and no explicit `isolation:`. That is the engine inferring isolation from an unrelated field. How many children a node spawns says nothing about whether they write — N review or research children over a shared checkout is the ordinary case, and handing each one a worktree costs a branch, a tracked environment and a cleanup for nothing. Removing the default alone makes things worse, so both halves land together. Siblings are deliberately not excluded from the working-path lock (executor.ts:1259), and N fan-out children of one node are siblings of each other. With the default gone they collide: one keeps the path, the rest cancel themselves, and a cancelled child is threaded as terminal on re-entry, so the parent fails identically on every resume. Verified in the harness — removing the default without the guard produces 2 child rows and a failed run for a 3-item, max_parallel-2 fan-out. So the collision is refused before a single child row exists, with a message naming all three ways out: `mutates_checkout: false` on the child if it only reads, `isolation: worktree` on the node if the children write, or `max_parallel: 1` for serial-in-place. The check lives at spawn time in executeFanOutWorkflowNode, not at load: the child target resolves when the node runs (#2200), so load time cannot see the child's `mutates_checkout`. It counts the indices this attempt will actually drive rather than items.length, so a resume with one instance left to re-drive is never blocked from recovering, and it fails open when the target name doesn't resolve — runChildWorkflow reports that with a better message. Tests: three e2e cases (the guard fires with zero children spawned and zero resolver calls; read-only children fan out in the parent checkout with no worktrees; max_parallel 1 is a valid serial-in-place fan-out) plus a loader case pinning that an omitted `isolation:` stays omitted. All four fail before the change. The existing fan-out fixtures now declare what they actually do — read-only children get `mutates_checkout: false`, the two that write a marker file get `isolation: worktree`. Refs #2121 (slice-2 PR-C), #2180. * docs(workflows): document fan_out — the fields, the checkout rules, gates, and resume `fan_out:` shipped as YAML surface with nothing under packages/docs-web. An author had no way to answer, without reading source, what the four fields do or what happens on a resume. authoring-workflows.md gains "Fanning out over a list with `fan_out:`" in the existing sub-run section: - the four fields and their defaults (`max_parallel: 5`, `join: all_success`), including that `as` is reserved and has no runtime effect today - `max_parallel` bounds concurrency, NOT total child count: `items` is unbounded, so cost scales with the list and a list wider than MAX_CASCADE_RUNS (500) can leave children uncancelled when the parent is abandoned - the join table, and that `first_success` is rejected at load rather than silently degraded - isolation: the same explicit-only rule, the sibling path-lock collision, the exact guard message, and a table mapping what the children do to which of the three escapes to use — plus why the check can only run at spawn time - gates: around a fan-out, never inside one. Written as the intended shape — gates bracket an autonomous middle, and both bracket positions were verified — with the real asymmetry (#2438) called out: the same child pauses correctly 1:1 and hard-fails fanned out - resume: `child_index` keying, what is re-driven vs threaded vs left alone, and what a changed item list does in both directions (shorter → orphan cancelled; same index, new content → warn) The node-fields table mentions `fan_out`, and the sub-run non-goals drop dynamic fan-out (shipped) while keeping racing and the loop_group guard. The constitution's case-law table records fan-out as shipped and why it is admissible: the expansion is data (a static target, a runtime count), each child is its own governance object, and the parent DAG stays flat and static. * chore(web): regenerate the OpenAPI client types `fan_out` is new YAML surface and never reached the generated types, so the web workflow builder's DagNode had no shape for it. Regenerated against a local server and re-formatted (the generator emits 4-space/double-quote output that prettier normalizes). The regeneration also picks up three fields that had drifted from dev before this branch: `evidence_policy` on the workflow schema, `settingSources` on AI nodes, and `schema` on the health response. Noted here so they are attributable to a catch-up regeneration rather than to fan-out. * test(workflows): make two fan-out resume tests deterministic Both failed intermittently under load — roughly one run in three of the package's test batches on a busy machine, and neither failure had anything to do with the behaviour under test. Cause is the same in both: with `max_parallel > 1` and `join: all_success`, the first child to fail seals the node's fate and cooperatively cancels whichever siblings are still in flight, tagged `fan_out_sibling` (I1). Which child wins that race is timing. So: - `1:N re-entry` asserted indexes 0 and 2 were `completed` after run 1; when a sibling lost the race it was `cancelled` instead. - `a user-cancelled fan-out child (untagged) is NOT resurrected` cancelled index 0 assuming it had failed on its own. When index 0 was the loser it already carried `cancelled_reason: fan_out_sibling`, which IS recoverable, so resume legitimately resurrected it and the test asserted the opposite of its subject. Reproduced deterministically by making the item at index 0 sleep. Both go serial (`max_parallel: 1`), which pins run 1 exactly and leaves each test's actual subject untouched. The user-cancel test now asserts its precondition (index 0 is `failed`, untagged) so the race can't silently come back as a different test. `1:N re-entry` also gains a real observable for its headline claim. Row count cannot show that a completed child was threaded rather than re-driven — a re-drive reuses the row — and neither can a side-effect file, since the child's own DAG resume skips its already-completed node. `completed_at` can: forcing index 0 back to `failed` before the resume changes it, and that was verified. Serial run 1 means index 2 is never spawned, so the test now covers three re-entry paths in one pass instead of two: a completed child threaded, a failed child re-driven in place, and a never-started index spawned fresh. * fix(workflows): reject fan_out.as at load instead of silently ignoring it `as` names the per-item value for the `$INPUTS.<as>` channel that PR-B (#2214) will add. `$INPUTS` exists nowhere in packages/workflows/src today, so an author who writes `as: task` and then `$INPUTS.task` in the child prompt gets the literal string delivered to the model — wrong output, no error, no warning. A field that quietly does nothing reads as a working feature. Staged the same way `join: first_success` already is: the key stays in the schema so PR-B lifts a guard rather than migrating anyone's YAML, and the superRefine rejects it now with a message naming the channel that does work today — the item arrives as the child's `$ARGUMENTS`. This is the isolation rule applied to a second field: the engine shouldn't accept a declaration it has no intention of honouring. Refs #2121 (slice-2 PR-C), #2214. * fix(workflows): name the causal child in the all_success failure, not a casualty The join reported the lowest-index non-completed outcome. Under `max_parallel > 1` that is routinely the wrong child: sealing the node's fate cancels every sibling still in flight and skips every index not yet spawned, and those casualties can sit BELOW the child whose failure caused them. With the failing item last, the node reported `child 0 (run run-2) cancelled` — no error text, and the real failure invisible. The operator has to go read child run rows to find out what actually happened. Casualties are now excluded by identity rather than by heuristic: a sibling this node cancelled is recorded in `failFastVictims` as it is cancelled, and an index never spawned carries the synthetic skip error, now a named constant instead of a prose literal. The lowest-index outcome that is neither wins. That selection does not depend on which child won the race. Whichever sibling loses is excluded either way, and a sibling that genuinely failed on its own stays eligible — so the message is stable across runs even though the set of cancelled siblings is not. It falls back to the old behaviour if every bad outcome is a casualty: the seal logic shouldn't produce that (something has to cause the seal), but a failed join must always name a child rather than report nothing. The test orders the children with a shared marker file rather than sleeps — the waiters block until the failing child creates it, so the failure always lands first and only the cancel window is timed. It asserts its construction actually materialised (index 2 failed, at least one lower index is a tagged casualty) separately from its subject, so a scenario that didn't build fails there instead of silently weakening into an assertion the old code would also pass. Reverting the selection reproduces the exact message quoted above. Refs #2121 (slice-2 PR-C), #2180. * docs(workflows): correct the fan_out.as and failure-message descriptions Two statements in the fan_out docs stopped being true with the fixes above. `as` was described as having "no effect today"; it is now rejected at load, so the row says that and says why — an accepted-and-ignored field would deliver a literal `$INPUTS.<as>` to the model. The join section explained fail-fast cancellation without saying what it does to the failure message. Adds the part an operator needs when reading one: the cancelled siblings and skipped items are casualties, they can sit at lower indices than the real failure, and the message names the causal child regardless. * feat(workflows): fan-out children run to terminal instead of racing each other Semantics decision: each child of a fan-out runs to completion or failure before anything below the node continues, and no child's outcome ends another's run. `all_success` used to seal the node on the first bad child — later indices were never spawned and in-flight siblings were cooperatively cancelled and tagged `fan_out_sibling`. That saved spend by deciding one child's fate from another's, which is not the engine's call, and it made an interrupted sibling's recorded outcome depend on which child happened to finish first. The verdict is unchanged (any failed child still fails the node); only the means are. `all_done` is unaffected in outcome. Three consequences, each worked out rather than assumed: **A paused child now has to be cancelled the moment it is seen, not at the join.** This is the one place a fan-out still ends a run it did not have to, and removing the fail-fast is what made the timing matter. A pause is not terminal, and a non-terminal run keeps holding its working path — `getActiveWorkflowRunByPath` counts `paused` as active. With the seal gone the next sibling actually spawns, loses the path lock to the paused child, and self-cancels with NO reason tag: it reads as a user cancel, is never re-driven, and the parent then fails identically on every resume. Found by running the gate-recovery test, which went red for that reason. Cancelling the paused child immediately frees the lock; it is decided by that child's own state, it is the same cancel the gate path was about to apply, and every sibling still runs to its own terminal state. **The causal-child selection is gone, and that is real simplification.** With no fail-fast there are no casualties: nothing is cancelled by a sibling and no index goes unspawned, so the lowest-index non-completed outcome IS the failure. `pickCausalOutcomeIndex`, `failFastVictims` and the synthetic skip constant all lose their reason to exist. **`fan_out_sibling` stays readable.** Nothing writes it any more, but a run that was in flight across the upgrade has rows carrying it. Dropping it from the recoverable set would make those children read as user-cancelled — terminal, never re-driven — so the parent would fail every resume with no way back. Kept in the type and the set, documented as read-only legacy. Tests: two contract tests (a failed child does not cancel its in-flight siblings; all_success runs every child to terminal and then fails). Both use sleeping survivors on purpose — with instant children they pass with or without the fail-fast, because the siblings finish before any cancel could reach them. Two tests that had been pinned to `max_parallel: 1` purely to dodge the fail-fast race are now concurrent and deterministic, and the causal-child test lost the marker-file choreography it needed to place casualties below the real failure. That choreography existed only to work around behaviour this removes. Docs: the join section describes the new contract and states the cost plainly — worst-case spend is items.length, not "until the first failure", which is what makes #1961's budget ceiling load-bearing rather than theoretical. Refs #2121 (slice-2 PR-C), #2180, #2438. * docs(workflows): add the independence rule to the constitution Parallel children are independent by default. Anything that couples their fates is opt-in, and must come from the author's declaration rather than be inferred. Same instinct as the isolation rule — the engine never guesses what the author must have meant — applied to lifecycle instead of storage. It is recorded because one wrong assumption generated a family of wrong features. Treating a fan-out as one job split N ways that jointly succeeds or fails makes four decisions look obviously correct: default the join to all-or-nothing, cancel siblings once the outcome is sealed, let a winner abort the losers, infer isolation from child count. Under the real model — N independent workers with different scopes whose outputs aggregate — all four destroy the thing the feature exists for. Case law gains three rejections and reverses one earlier admission: - first_success racing: was admitted as 'a join rule — coordination', which describes its shape and misses what it does. The winner aborts and cancels the losers. Cannot be reshaped; racing without terminating the losers is not racing. The want underneath it is served by N distinct nodes converging on a collector. - Fail-fast sibling cancellation: rejected. The siblings' output is exactly what a partial failure should preserve. - all_success as the DEFAULT: rejected as a default, retained as an option. - Threshold joins: rejected. How many results are enough is judgement, and belongs in a node reading the all_done aggregate. * feat(workflows): default fan_out join to all_done `join` defaulted to `all_success`, which assumes a fan-out is one job split N ways that jointly succeeds or fails. That is the uncommon case. Two research children with different scopes, or ten triage children over ten issues, are ten jobs that happen to run together — one failing says nothing about the other nine, and their output is still the point. Removing the fail-fast fixed half of this: a failed child no longer cancels its siblings. But under `all_success` it still failed the node, so the downstream collector received nothing even though every sibling had run to completion and produced good output. Same loss, later in the pipeline. `all_done` becomes the default: every terminal outcome aggregates, failures represented as `{error, status}` in their own slot, and the node succeeds. Failure becomes data the next node can read. `all_success` stays for the genuinely dependent case — where a gap makes the aggregate meaningless rather than smaller — it just stops being what an author gets by accident. No threshold join, and the docs say why rather than leaving it to be proposed again: how many results are enough depends on which children failed and why, it changes between runs, and that is judgement for a script or prompt node reading the aggregate with `when:` gating what follows. An enum cannot weigh it, and adding one starts a policy language inside a YAML field. Docs: the fields and join tables carry the new default, the worked example drops its explicit `join:` (the default is now the right answer for a per-file review fan-out), and a "Why all_done is the default" section states the independence reasoning with a worked all_done → script → `when:` shape. Two resume tests now declare `join: all_success` explicitly. Both are about re-driving a failed instance, which needs the node to fail — they are the dependent case, and saying so is more honest than relying on a default. Refs #2121 (slice-2 PR-C), #1961. * fix(workflows): say racing is rejected, not deferred, and disambiguate the two joins The `first_success` rejection told authors it was "not yet supported (PR-D)" and to wait. #2250 is closed and racing is rejected outright in the constitution — the earlier admission was reversed — so that was a promise nobody was going to keep. Third instance of the same defect this branch has now fixed three times: `fan_out.as` advertising a `$INPUTS` channel that did not exist, and the collision guard recommending `max_parallel: 1` to a racing author. The message now says rejected rather than deferred, gives the reason a reader can act on (a winner cancels the losers, which couples children meant to be independent), and points at the shape that actually serves the want: separate nodes with their own models feeding one collector, nothing cancelled. The docstring and the superRefine comment carried the same dead assumption and are rewritten with it. The enum value stays. Its original justification — "so the eventual PR only has to lift a guard" — is gone, and a better one replaces it: an author whose YAML already says `first_success` gets a message naming the rejection and the alternative rather than an opaque unrecognised-value error. The comment says so, so the next person does not remove it as dead weight. Docs get "Why there is no racing join", with the separate-nodes-plus-collector pattern written out. It is worth stating that this is strictly better than racing at the thing racing was wanted for: the attempts can differ by MODEL, which a fan-out cannot express; every output survives for the collector to weigh instead of being thrown away; and selection is a judgement made by a node that reads the work rather than by a stopwatch. Also disambiguates the two joins, which now diverge: `trigger_rule` (any node, default `all_success`, decides whether THIS node runs given its dependencies) versus `fan_out.join` (fan-out only, default `all_done`, reduces one node's N children). They share value names, so the node-fields table cross-references and the trigger_rule section carries a note explaining why the defaults differ — upstream dependencies are steps you chose to sequence, fan-out children are N instances of one step. Refs #2121 (slice-2 PR-C), #2250, #2447. * fix(workflows): address the #2224 review — two defects, two stale artifacts, and the rest **`fan_out_orphan` was missing from the recoverable-cancel set.** The type has three members and the docblock names all three as engine-cancelled and therefore recoverable; the set had two. An orphan the engine cancelled read as a user cancel, so items shrinking and then growing back left those slots dead under all_done and failed the node on every resume under all_success. **`tokens` were computed and then dropped from the fan-out `node_completed` event.** Worse than the cost gap it sits next to: `getDagResumeSnapshot` rebuilds cumulative usage by summing `data.tokens` and never reads `cost_usd`, so tokens are the axis that survives resume — and fan-out was the one node type that did not persist them. On Codex, which reports no cost either, the loss was total. Threaded through `writeCompleted` at both call sites. Also in the same area, and now documented rather than fixed: cost and tokens are Σ *completed* children, not Σ children, because usage is persisted only inside `completeWorkflowRun`. A child that burns tokens and then fails records nothing. Inherited from the 1:1 path, but `all_done` being the default makes a partly failed run ordinary rather than exceptional, so the docblock now says so and points at the upstream fix. **The collision preflight failed open.** `resolveFanOutChildDefinition` collapsed every failure to `undefined` at `debug`, and an unknown or ambiguous name reaches that branch without any exception. "We could not check" then read exactly like "we checked and it is fine", skipping the only guard against a path-lock cascade the engine cannot recover from. It now reports why it failed and the preflight fails closed — with a message about the unresolvable target, not a collision the author cannot yet act on. **A child of this node with no `child_index` was dropped with a bare `continue`.** That is a 1:1 child left behind when a node grows a `fan_out:`; `findChildRuns` filters on `parent_node_id` alone, so nothing else would ever find it and it stayed live, billing and untracked. Same warn + cancel-if-live as the out-of-range branch directly below it. Smaller: the child-lookup failure now notifies like every other early-failure branch; two comments that read as present tense but described a prior revision are corrected; `.field` on a fan-out aggregate gets its own error case pointing at the script-node pattern instead of telling the author to change a producer prompt that does not exist (the fail-loud behaviour is correct and unchanged). Type safety: the `parent_node_id` / `child_index` / `fan_out_item_hash` literals are now shared constants with a typed reader, so writer and reader agree by symbol rather than by luck on an untyped JSONB column where a typo silently no-ops. `loader.ts` also hand-rolled a second copy of the canonical output-ref regex inside the function that carries its own KEEP IN SYNC warning — hoisted to one source, with both call sites building their own RegExp so the g-flag's mutable lastIndex is not shared. Docs: the constitution's independence rule now carves out the paused-child exception explicitly — it is not a coupling, because nothing about a sibling decides it — and `Case law` is promoted to a `##` sibling of both rules rather than reading as a subsection of independence alone. `quick-reference.md` gains the `fan_out` field row it was missing and, with `api.md` and `cli.md`, the gate carve-out. `CLAUDE.md`'s `workflow:` node description gains a fan_out clause. Tests: token persistence asserted on the existing cost test; resume + preflight with partial completion (which would silently reinstate the items.length-vs-pendingCount bug this branch already fixed once); orphan recoverability; and the missing-index case. Each verified failing before its fix. --------- Co-authored-by: Archon Maintainer Bot <maintainer-implementer@archon.local>
…chon (#2299) * feat(workflows): unify run output under one per-project tree in ~/.archon Makes "the repo holds SOURCE, ~/.archon/workspaces/<project>/ holds OUTPUT" a structural invariant instead of a prompt convention. One key, one resolver. The identity -> storage-paths rule lived in three places at three levels of correctness: the executor handled repo/_local/_folder, the CLI's `continue` handled repo/_local, and the two HTTP artifact routes handled `owner/repo` only. That last gap is the headline defect — for a folder project or a no-remote local repo `parseOwnerRepo(codebase.name)` returns null, so GET /api/runs/:runId/artifacts answered 200 with an empty list: indistinguishable from "the run wrote nothing". `resolveProjectStorageKey` + `getProjectStoragePaths` now live in @archon/paths and all four call sites delegate. The list route 404s with an explicit message when nothing resolves, and the console renders that distinctly from an empty list. $STATE_DIR. `.archon/state/` was a bash-agent convention with zero engine support — `mkdir -p .archon/state` relative to cwd. Inside an isolated run that is the worktree, so the cross-run memory died at cleanup; in a user's repo it was stageable (Archon never writes a .gitignore). $STATE_DIR is a pre-created sibling of $ARTIFACTS_DIR at <projectRoot>/state/, scoped per PROJECT so cooperating workflows share one ledger. Referencing it without a resolved value throws, mirroring $BASE_BRANCH. A legacy directory produces exactly one WARN with a literal `mv`; nothing is ever moved automatically. A durable pointer, not a durable key. workflow_runs.output_root records the resolved project root once at run start (never on resume). Readers prefer it and re-derive only when it is NULL, which decouples historical artifact lookup from the codebase-identity redesign in #1192 without waiting for it. BREAKING: a run with no resolvable codebase used to write artifacts and logs to <cwd>/.archon/ — the engine itself writing output into the repository. It now resolves to ~/.archon/workspaces/_cwd/<basename>/ like every other project kind. No migration: existing in-repo output stays where it is and is no longer looked up. Authors who want output in git use the copy-node escape hatch documented in the authoring guide. Also lands the constitution case-law entry and 8 lock tests for `workflow:`'s spawn-time target resolution, so a well-meaning "make it consistent with include:" load-time check cannot silently delete runtime sub-run authoring. Container runs are documented as a known limitation, not implemented: neither $ARTIFACTS_DIR nor $STATE_DIR is mounted, so those writes land in the container's ephemeral layer (readable via docker exec until cleanup). A third bind mount and `docker cp` are both recorded as rejected, with evidence. Closes #2200 * fix(workflows): make the state guard structural and the migration honest Addresses the three blockers from review of #2299. C1 — CI was red and the migration script could not run. scripts/ imports @archon/paths and @archon/core, but the root manifest declared only @archon/git and @archon/providers, and Bun links a workspace package into root node_modules/ only when the root manifest depends on it. Root type-check runs `tsc -p scripts/tsconfig.json`, so a clean checkout failed with two TS2307s while a long-lived local node_modules still carried the symlinks from an older install. Both packages are now declared. Verified by removing node_modules and running `bun install --frozen-lockfile` before validate, not by re-running in place. C2 — the first-run guard covered 1 of 5 state reads. The four it missed drive exactly the side effects it exists to prevent: closed-issue and closed-PR dedup comments, PR template comments, and stale nudges (stale-nudging had no guard at all). Repeating the prose four more times would leave a sixth state file unguarded by default, so the check is now a single deterministic `bash:` node, `state-preflight`, that every state-reading node hangs off — the sole root of the DAG, with the four former roots depending on it and closed-dedup-check covered transitively. It is deliberately directory-level and names no state file, so a sixth state file added later is covered by construction. It also asks a better question than the marker did: "does unmigrated legacy state exist while $STATE_DIR is empty?" That is the actual discriminator, which means a genuine first run on a fresh project no longer aborts and needs no manual setup step. Exercised against all four cases: fresh project, unmigrated legacy state, migrated, and explicitly marked. The stale "same rule as triage-issues step 1" claim is corrected; the per-file prose now points at the gate. C3 — the migration script marked partial migrations complete. A nested directory hit `continue`, then `.initialized` was written unconditionally and the summary reported the pre-skip count — success message, marker, and leftover state, which told the guard to proceed with an incomplete state dir. The pre-flight now decides the entire migration before moving a byte and refuses (exit 2, nothing moved, nothing marked) on either a destination collision or a nested directory, reporting both together. The count is the real moved count. The inverse gap is closed too: the no-op paths (no legacy dir, empty legacy dir) now mark the destination under --apply, so an operator who correctly runs the migration on a project with nothing to migrate is not left with an unusable $STATE_DIR. Also from the review: - The legacy-state warning latched process-wide while probing a per-project path, so on a server the first run of any project permanently suppressed detection for every other one. Keyed by cwd now, with a regression test. - updateWorkflowRun writes output_root via COALESCE, making write-once structural rather than doc-only. - A failed output_root persist logs at error, not warn: it is never retried, so that run stays on the re-derive path for its whole lifetime. - maybeWarnLegacyStatePath takes the resolved stateDir instead of re-deriving it, removing the last hard-coded 'state' segment outside archon-paths.ts. - 8 subprocess-driven tests for the migration script, covering exit codes and the marker's presence or absence on every path. - A test that STATE_DIR reaches bash and script subprocesses as an env var — the one delivery channel the fail-fast cannot protect, since a dropped env entry is silent. - repo-init.md no longer teaches .archon/state/ as the place for cross-run state; that file is copied into user projects. - Doc fixes: $WS is not a substitution variable; the script has no --dry-run flag (dry run is the default); $STATE_DIR added to the variables summary list and the availability table. * test(workflows): thread stateDir into the executeDagWorkflow call added by #2205 The #2205 test landed on dev while this branch was changing executeDagWorkflow's signature, so after the rebase its positional args shifted by one and config arrived undefined. Test files are excluded from tsconfig, so this surfaced only at runtime — on CI's merge commit, not on the branch head. * test(workflows): keep the preamble suite out of the real ARCHON_HOME The @archon/paths mock in executor-preamble.test.ts is partial, so unlisted exports fall through to the real module and the real storage resolver runs. These cases use cwd '/tmp', which now resolves to _cwd/tmp — so the executor pre-created artifacts/ and state/ inside the developer's actual ~/.archon. Redirect ARCHON_HOME to a temp dir per case, matching subrun.test.ts. Pre-existing in kind (the old fallback wrote to /tmp/.archon) but the relocated fallback moved it into $HOME, which is worse. * fix(workflows): re-check the state gate on resume state-preflight is a precondition, not a work step. Without always_run a resumed run serves it from the resume cache (prior_success) and proceeds against whatever $STATE_DIR looks like now rather than what it looked like when the gate passed. The check is a pure read with no side effects, so re-running it costs nothing. * test: make the new #2200 tests portable on Windows Windows CI ran to completion for the first time on this branch (earlier runs were cancelled when ubuntu failed first) and caught a POSIX-separator assumption in a test I added: join() produces backslashes on win32, so `paths.root.startsWith('/custom/archon')` was false. Both assertions now build their expectations with join(). The paths package aborted the parallel run, so workflows/server/cli/core and the scripts tests never executed on Windows — fixing only the reported failure would have bought another red cycle. Hardened the other two Windows-risky spots I introduced at the same time: - api.workflow-runs.test.ts hard-coded '/tmp/.archon' while actually creating and reading files there; '/tmp/...' is not an absolute path on Windows. The paths mock now derives from a mutable mockArchonHome, and the two artifact describes point it at a real mkdtemp dir, torn down per case. - The STATE_DIR env-delivery test's bash node used "$STATE_DIR", which the engine substitutes textually — putting a backslash-laden Windows path into the script body. It now reads "${STATE_DIR}", which survives substitution (the engine replaces the exact string $STATE_DIR) and is expanded by the shell from the env bag. That also tests the env channel more precisely, which is the point of the test. * test(workflows): build resolveProjectPaths expectations with join() Second instance of the same Windows failure, one layer down: the @archon/paths fakes in executor.test.ts composed paths with template literals while production composeRunPaths uses join(), so every expectation was forward-slashed and failed on win32. The fakes now use join() too, so they behave like the helpers they stand in for, and expectations are built through a wsPath() helper. Also replaced a negative assertion (!startsWith('/some/cwd')) that passed trivially on win32 with the positive form. Audited every test file this branch touches for the same class — the remaining POSIX literals are all either join() inputs, verbatim pass-throughs, or pre-existing values the test itself supplied. * fix(scripts): reject malformed migration args; close the latch and DRY gaps Round-3 review fixes. scripts/migrate-state-dir.ts — two defects, both in the silent-success family this PR exists to prevent: - `--cwd --apply` swallowed the flag as a path, resolved to <pwd>/--apply, found no legacy state, wrote a junk .initialized marker and exited 0 reporting success. argv is now parsed strictly: --cwd requires a non-flag value, unknown arguments are rejected, and either exits 1 having written nothing. `--dry-run` is rejected too — it looks plausible since dry run is the default, so accepting it silently would teach a wrong invocation that happens to work. - `--apply` printed `move <name>` for every entry BEFORE the copy loop, so a run that died midway claimed moves it never made. Progress is reported after each successful move; the dry-run wording is a separate branch. state-migration.ts — the cwd latch was still set before the probe, so a cwd with no legacy directory consumed its own latch and a directory created later in the same process stayed silent for it forever. The latch is written only on an actual warning, with an in-flight promise map preserving the concurrent-start dedupe the eager set was really buying. The new test kills the mutant: restoring the pre-probe set fails it. S3 — the rule-of-three decline rested on a miscount (I counted 2; it is 6: three inside @archon/paths and three outside, including the WRITER in executor.ts). Writer and readers composing <artifactsRoot>/runs/<id> by hand is #2200's own bug one level down, so getRunArtifactsDirForRoot() is now the single composition point for the persisted-root branch, shared by the executor, the artifact routes, and the CLI. S6 — the COALESCE write-once clause was new hand-concatenated SQL with no tests. Three added: clause shape, placeholder numbering alongside other fields, and absence when output_root is omitted. S11 — the archon-paths docblock asserted a complete tree with a closing branch while this PR adds a fifth child; it now documents all four project-key kinds and state/. Also a third instance of the Windows separator class, visible only once the earlier failures stopped aborting the run: join()-ifying the getScopeArtifactsPath mock broke pre-existing resolveScopeArtifactsDir expectations built from template literals. * fix(scripts): anchor migration source and destination together (C5) `migrate-state-dir.ts` could silently disarm the state-preflight gate. `findCodebaseByPathPrefix` matches any SUBDIRECTORY of a registered project, so the destination climbed to the project root while the source stayed at the literal cwd (`join(CWD, '.archon', 'state')`). Run from `<project>/packages/foo` and the two pointed at different things: no legacy dir under the subdirectory → "nothing to migrate" → `.initialized` written into the REAL project's state root, exit 0. That marker is the gate's latch, so the abort became unreachable and a run would re-post dedup comments and re-nudge stale issues at contributors — the tool defeating the gate, via the documented happy path, with no malformed input. Reproduced before fixing. Both now derive from one `anchor` (the codebase's `default_cwd` when one matched, else the cwd), which makes the disagreement unrepresentable rather than special-cased. The climb is reported explicitly. When the invocation cwd ALSO holds legacy state the script refuses (exit 2, nothing moved, nothing marked) — migrating one while marking would leave the other unmigrated behind a satisfied marker, which is the same bug one level down. Two more entry-point defects in the same family: - `--cwd <nonexistent>` was a confident no-op success that wrote a junk marker; the cwd is now validated before anything else. - a repeated `--cwd` silently used the last; it now exits 1, since a copy-paste slip would otherwise operate on a different project than the one the operator is reading in their shell. S16 — the `output_root` containment guard existed on 1 of 3 consumers (the API routes). A relative or whitespace root would make a run mkdir its artifacts AND its shared state under the server's cwd and report success. `isInsideArchonHome` moves to @archon/paths as the single definition and now gates the executor's WRITE path (ignore + re-derive + log at error, so a corrupt row cannot brick a run) and the CLI reader (drop the candidate) as well as the routes. S17 — the docstring claimed run-id subdirectories keep colliding projects apart. True for artifacts and logs; false for `state/`, which this PR added to the same root and which has no run-id segment, so two local repos both named `api` share one `triage-state.json`. Corrected in both docstrings and documented in variables.md. Also: argv tests now assert the destination tree is ABSENT rather than empty — `listOrEmpty` swallows ENOENT, so `toEqual([])` could not tell a refusal that created nothing from one that created an empty tree. * fix(server): keep the artifact tree relocatable across an ARCHON_HOME move I1 — a moved or restored ARCHON_HOME permanently un-browsed every stamped run. The containment guard sat AFTER the output_root branch, so a persisted root that no longer resolves inside ARCHON_HOME produced a hard 400 with no fallthrough. Move the home — machine migration, restored backup, the documented ARCHON_DATA split — and every stamped root is out-of-tree at once, so every run stamped since #2200 becomes unbrowsable even though its artifacts are re-derivable and physically present under the new home. output_root is write-once via COALESCE, so the app could never clear the column to recover; raw SQL was the only repair. On dev today this just works, because paths are derived at read time and are therefore relocatable — the column introduced the regression, and the guard placement is what turned a cache into an authority. The guard now sits INSIDE the branch, matching continue.ts and the executor: an untrusted root is ignored and the location re-derived. The outer check stays as defense-in-depth for the DERIVED path, which is its real job — a crafted codebase name must still not escape the tree. I2 — the shipped OpenAPI description still documented the behaviour this PR deleted ("Returns `{ files: [] }` when the run has no codebase or the codebase name is not in `owner/repo` form"), live in /api/openapi.json and baked into the checked-in api.generated.d.ts. Rewritten and regenerated. The regeneration also picks up `output_root` on both run schemas — the web types never knew about the column this PR adds — plus pre-existing drift from other merged PRs (evidence_policy, settingSources) that nobody had regenerated. Also corrects a doc claim of mine: `worktree: enabled: false` does not serialize a second run, it REJECTS it outright ("This worktree is in use"). Verified at executor.ts:1253-1258. * test(workflows): reconcile dev test call sites with the stateDir signature Rebasing onto dev surfaced 19 call sites that predate this branch's insertion of the required positional `stateDir` parameter into executeDagWorkflow (between `artifactsDir` and `logDir`). - dag-executor.test.ts: 17 calls passed the old argument order, silently shifting logDir/baseBranch/docsDir/config by one. Added the missing `join(testDir, 'state')` argument, matching the 230 call sites in the same file this branch already updated. - executor.test.ts: 4 positional-index assertions on the executeDagWorkflow mock still used pre-insertion indices — baseBranch 10 -> 11, priorCompletedNodes 15 -> 16, priorTokenUsage 23 -> 24. Every argument involved is a string, and test files are excluded from type-check (packages/workflows/tsconfig.json), so neither tsc nor lint could catch this — only the suite did. * fix(workflows): retry a transient codebase lookup, and detect legacy repo-local artifacts Two additions on top of the rebase, both narrow. #2304: a transient getCodebase fault dropped the run onto the _cwd/<basename> pseudo-project, and because output_root is write-once that location was pinned for the run's whole life — including its $STATE_DIR, so a stateful workflow silently read an empty state directory. A transient fault is the only thing that produces this, so the lookup is retried once rather than compensated for downstream. Failing the run instead was rejected: the fallback exists precisely so a registry blip does not kill a run. Whether an unresolved identity should be recorded on the row stays open in #2304. #2311: the .archon/state relocation Archon never caused got a detector; the artifacts/logs relocation the engine itself caused on the unregistered-cwd fallback got nothing, so the case Archon caused was the silent one. Adds the matching one-time WARN, latched separately so a repo with one legacy directory and not the other still reports both. Deliberately not a migration: for an isolated run cwd is the worktree, so those files already died at teardown; run in place they survive in the user's own repo, unlinked rather than deleted. Also reconciles four dag-executor test call sites added by #2388 after this branch inserted the required positional stateDir parameter — they passed the old argument order, shifting logDir/baseBranch/docsDir/config by one and leaving config undefined. Same class the branch already fixed once, and an argument for #2302. * chore: drop test-log debris committed during the rebase Six .jsonl files were swept in by a `git add -A` in 42690a8. They are artifacts OF the bug that commit fixed: before the stateDir argument was threaded, `'main'` landed in executeDagWorkflow's logDir slot, so the executor wrote run logs to a relative `main/` directory in the test's cwd. With the argument order corrected the directory is no longer created — verified by deleting the files and re-running the suite (34 pass, no `main/` recreated). Nothing references them. Found by an independent probe of this PR, not by the rebase itself. * fix(workflows): use the run's actual isolation posture for the legacy warns Three corrections from an adversarial probe of this PR. 1. `isolated` read `workflow.worktree?.enabled !== false` — the workflow's DECLARED policy, not the run's actual posture. The real decision is `pinnedEnabled ?? (!resume && !noWorktree)`, resolved in the CLI, so a workflow that leaves `worktree` unset and is run with --no-worktree executes in place while the declared policy still reads as isolated. That inverts the message in the one case where it is actionable: the legacy files are sitting in the user's real repository, and they were being told the files 'are deleted with the worktree, so nothing is lost'. Folder projects took the same path. A managed worktree always lives under ARCHON_HOME and an in-place checkout never does, so the cwd answers what the policy cannot. 2. The #2304 retry comment claimed 'the ONLY thing that produces this is a transient fault'. True in kind, misleading for the default install: on SQLite, busy_timeout=5000 means SQLITE_BUSY cannot throw until five seconds of sustained contention have elapsed, so what reaches us is by construction not transient. The retry earns its place on Postgres, where a stale pooled connection is exactly the fault it clears. Comment now says so per dialect rather than overclaiming. 3. A .gitignore entry for `main/`, so the stray run logs removed in 0edd6a7 cannot come back from a future mis-threaded logDir argument. Batches 1, 17 and 37 green in isolation (449, 81, 13 pass). --------- Co-authored-by: Archon Maintainer Bot <maintainer-implementer@archon.local>
The bundled idea-to-pr / plan-to-pr family could sweep repo-local .archon/artifacts/, .archon/logs/, and .archon/state/ into a target repo's PR (#1407 landed 1,821 lines of telemetry in a user's repo). Run artifacts now live outside the repo, but repo-local .archon/ dirs still exist in target repos (older layouts, the .archon/state/ convention, user workflows) and no bundled default guarded against committing them. - archon-implement-tasks: add a Repository Hygiene section — any .gitignore the run creates or modifies must include the three .archon telemetry paths, and they must never be staged - extend every existing "Never stage" blocklist (finalize-pr, create-pr, fix-issue, implement-issue, implement-review-fixes, simplify-changes, auto-fix-review, self-fix-all, fix-github-issue, piv-loop, ralph-dag, refactor-safely) with the repo-local .archon telemetry paths - regenerate bundled-defaults.generated.ts Closes #1407. Supersedes the telemetry slice of #1408. Co-authored-by: Archon Maintainer Bot <maintainer-implementer@archon.local> Co-authored-by: 2-Chengs <ivan.cheng011099@gmail.com>
…orrect the interactive hint Unknown YAML keys were silently stripped by Zod. They are now reported as non-blocking warnings across every surface an author actually looks at — `archon validate workflows` (human and `--json`), chat, the console workflow picker, and the API — each naming the node and the key. Adds per-mode ignored-field lists so an AI field on a non-AI node (`model:` on `bash:`, the include/workflow ignored sets) is warned rather than dropped. Warn, never reject: rejecting would break workflows that load today. Persists the warnings to the audit trail as a `workflow_parse_warnings` event emitted in the engine beside `workflow_started`, so the record exists whatever surface started the run and survives a failed delivery. No schema change — `workflow_events` already stores type + JSONB. Closes #2213 Closes #2255 Follow-up: #2478 (mode-exclusive keys on the wrong node mode are still dropped in silence).
Adds `with:` on `include:` nodes and the `$INPUTS.<name>` load-time macro, so a shared sub-DAG can be parameterised instead of forked. Phase 1 of the workflow signature plan: expansion resolves fully at load time, so the executor still sees a flat static DAG and load-time validation, resume and the audit trail are unaffected. A missing input fails the load rather than substituting silently — `Object.hasOwn` is used for the lookup so an unsupplied `$INPUTS.toString` is reported missing instead of splicing an inherited member into the prompt. `$INPUTS` is walked across every model-facing string surface, including the ones the output-ref rewrite skips (`systemPrompt`, `agents.*`, `approval.on_reject.prompt`), because load time is the only pass that resolves it. Command files referenced by an include block are scanned for refs that namespacing would break and for unsatisfiable `$INPUTS` parameters; a file discovery could not resolve warns and continues, since that is an incomplete-information state rather than an unsafe one. Closes #2466 Follow-ups: #2475 (`on_reject` refs unvalidated and unrenamed), #2476 (`systemPrompt`/`agents` runtime substitution decision), #2477 (`loop_group` body command scan).
…tespace cannot degrade the error hasTruncationMarker() was anchored exact-tail, so anything appended after the marker — a single trailing newline is enough — made it return false. The effect is not broken behaviour but worse advice: a clipped-output failure silently reports the generic "not a JSON object" error instead of naming the truncation. Nothing appends after the marker today, which is exactly why this is worth pinning. The regression is invisible by construction: the first normalisation step anyone adds to persisted node output would swap a precise diagnostic for a vague one, with no test failing. Match against output.trimEnd(), and extend the existing round-trip test in output-ref.test.ts with the trailing-whitespace case. Verified failing before the change and passing after. hasTruncationMarker has one caller (output-ref.ts:137), on the error-classification path, so the extra allocation is not on any hot path. Closes #2465
The deterministic tier covered bash/script/bun/uv/timeout and two trigger rules. It did not cover join semantics against a skipped upstream, until_bash termination, output_type, or fan-out at all — so the composition primitives merged in #2223/#2224/#2467 had no unattended regression test. Four workflows, no AI nodes, all assertions in bash: e2e-joins trigger_rule all_done + none_failed_min_one_success against a SKIPPED upstream, output_type, and a loop_group terminating on until_bash e2e-echo-child one bash node; the cheapest possible fan-out child e2e-fanout-alldone fan_out over a literal list with one failing child; all_done must aggregate it as {error,status} e2e-fanout-allsuccess same list, join: all_success — must FAIL the node. Wired as a negative test, so a zero exit is the regression and CI inverts the assertion Each runs in seconds and costs nothing, which is what makes them viable on every push rather than as a deliberate exercise. Verified with --no-worktree, the form CI uses: the first two exit 0, the third exits 1. Also adds rasmus-tests/ for the AI-driven probes these were derived from. Those need a funded provider and minutes per run, so they stay out of CI — t8-cascade additionally needs a concurrent abandon to observe cascade-cancel and cannot run unattended without a driver script. The split is now readable from either side: unattended work lives in test-workflows/, subscription work in rasmus-tests/. Two engine findings came out of running them: #2494 and #2495.
#2499) * fix(workflows): stop the issue-fix workflow on a missing specification Run 42acf940 spent a `model: large` implement node and four review-tail nodes on a run that had already lost its specification, then posted a public comment on the issue announcing it was blocked. The investigate node delegated to an installed skill instead of running the command it was given. That skill routes on the leading verb of its input; $ARGUMENTS was "fix issue 2473", so it selected implement-mode, demanded the investigation artifact that only this command produces, and declined. An AI node that declines still exits 0, so the refusal read downstream as a completed investigation. - bridge-artifacts: exit 1 instead of printing WARNING. It holds the only cheap deterministic view of the precondition, so it is where the run has to stop. - archon-investigate-issue: execute the command directly rather than delegating to a skill, and strip a leading intent verb from the input. The trigger message is the user's whole phrasing, so the verb reaches the command as if it were a mode switch. It is not one. Does not address the review tail still firing after an upstream failure (synthesize's all_done cannot distinguish a by-design skip from a dead run) — that needs its own gate. * fix(workflows): gate the review tail on a PR actually existing The review tail ran to completion on run 42acf940 after the implementation phase had already failed — every reviewer skipped, synthesize/self-fix/simplify/report all fired, and report posted a public comment on the issue announcing the run was blocked. synthesize uses trigger_rule: all_done so it waits for every reviewer to reach a terminal state including the ones the classifier skipped by design. all_done cannot distinguish that from reviewers skipped because the run died upstream, and no trigger_rule can — so the decision moves into a node that computes it and a when: that reads it. pr-exists reports whether .pr-number was written (capture-pr-number exits 1 when no PR resolves, so its absence is authoritative) and runs under all_done itself so the tail always has a signal. synthesize skipping propagates to self-fix -> simplify -> report through the default all_success.
…default (#2500) #2499 fixed bridge-artifacts in the experimental copy only. The bundled archon-fix-github-issue.yaml — the one users actually get — carried the identical warn-and-continue branch, so the same refusal-reads-as-success path was still shipped. Caught in review of #2499. Also clear .pr-number/.pr-url at the top of capture-pr-number in the experimental workflow. pr-exists treats the file's presence as authoritative, so the write must be the product of the validation that just ran, not of an earlier attempt. The bundled workflow's review tail does not need the pr-exists gate: its synthesize uses trigger_rule: one_success, so a run that dies before create-pr leaves every reviewer skipped, zero successes, and the tail skips on its own.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
POWERFULMOVES
added a commit
to POWERFULMOVES/PMOVES-Archon
that referenced
this pull request
Aug 10, 2026
) * fix(submodules): drop 7 orphan gitlinks with no .gitmodules mapping `1e02907a` ("sync: merge upstream/main into PMOVES.AI-Edition-Hardened", 2026-08-06) added 7 gitlink tree entries and no `.gitmodules` file. There is no `.gitmodules` in this branch at all, so every one of them is unmapped: external/PMOVES-Agent-Zero external/PMOVES-BoTZ external/PMOVES-Deep-Serch external/PMOVES-HiRAG pmoves_multi_agent_pro_pack/PMOVES-BotZ-gateway pmoves_multi_agent_pro_pack/PMOVES-tensorzero pmoves_multi_agent_pro_pack/docling Any `git submodule` traversal dies on the first one: $ git submodule status fatal: no submodule mapping found in .gitmodules for path 'external/PMOVES-Agent-Zero' This is a merge resurrection, not an intentional addition. Both parents of `1e02907a` carry zero gitlinks — `38cfd912` (this branch, pre-merge) and `1213b808` (upstream Release 0.8.0). The merge introduced all 7 against both sides, which is why no `.gitmodules` came with them. No code in this repository references any of the 7 paths, so removing the tree entries restores exactly what both parents had. The alternative — authoring a `.gitmodules` with 7 entries — would declare submodules neither parent wanted. Downstream effect: PMOVES.AI consumes this branch at two gitlink paths (`PMOVES-Archon` and `pmoves/integrations/archon`). `actions/checkout` runs `git submodule foreach --recursive` for its sshCommand cleanup even with `submodules: false`, so any runner holding a recursive checkout of this repo fails that step with exit 128. That is the observed `emit lifecycle trail` failure on PMOVES.AI PRs coleam00#2501/coleam00#2502. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(git): test asserted clone argv without the `--` separator the impl passes `Test Suite` has been red on PMOVES.AI-Edition-Hardened since 2026-08-06 with a single failure: (fail) git utilities > cloneRepository > clones successfully without token `repo.ts:397` runs `['clone', '--', cloneUrl, targetPath]`. The `--` is the CodeQL js/second-order-command-line-injection barrier — an argument separator so a URL can never be read as an option. The test still expected the pre-barrier argv, so it asserted against the vulnerable shape. A sibling test in the same block was updated when the barrier landed — line 2228 already documents `args: ['clone', '--', <url>, <target>]` in a comment. This one was missed, and the 2026-08-06 upstream sync carried the gap onto the branch. Fixing the test, not the implementation: the `--` is the security fix and must stay. `repo.ts` also keeps the leading-dash guard above it, which is defence-in-depth rather than a substitute — CodeQL wants the separator. Second commit on this branch because it is a second defect, but the same origin: `1e02907a` is also what added the 7 orphan gitlinks. One bad sync, two failures, and this one has been blocking every PR against the branch since. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[0.8.0] - 2026-08-06
Run output moves out of the repository for good — see Breaking below before upgrading. Alongside it, workflow composition grows up: sub-run nodes can now fan out over a runtime list and take their own worktree, and include blocks accept parameters. Plus a batch of fixes for failures that were previously silent.
Breaking
.archon/state/is no longer read, and is never migrated automatically. Cross-run state now lives at$STATE_DIR(~/.archon/workspaces/<project>/state/). A run that finds a legacy directory emits exactly one warning containing the literalmvcommand and then proceeds with an empty state directory — so a workflow depending on prior state will not error, it will behave as though it is running for the first time. Move it before upgrading, or on the first warning. From a source checkout,bun run scripts/migrate-state-dir.tsreports what would move (dry run by default) and--applyperforms it; binary installs should use themvprinted in the warning. (feat(workflows): unify run output under one per-project tree in ~/.archon #2299)<cwd>/.archon/. The engine's fallback previously placed its own output inside the working directory — inside a user's repository, where it was stageable. Output now resolves under~/.archon/workspaces/for every run. Anything reading run artifacts from a repo-relative path must be repointed at$ARTIFACTS_DIR. (feat(workflows): unify run output under one per-project tree in ~/.archon #2299)GET /api/runs/:runId/artifactsreturns 404 instead of an empty list when a run's project storage cannot be resolved. It previously answered HTTP 200 with{ files: [] }for folder projects and local repos without a remote, which was indistinguishable from a run that wrote nothing. Consumers treating an empty list as "no artifacts" must now also handle 404. (feat(workflows): unify run output under one per-project tree in ~/.archon #2299)Added
workflow:sub-run nodes.fan_out: { items, max_parallel, join }expands one governed child run per item of a runtime list, bounded by a sliding concurrency window and joined byall_done(default) orall_success. Results aggregate as a JSON array in item order and thread back as$nodeId.output. Previously a sub-run node was strictly 1:1 with its width fixed in YAML, so the orchestrator-worker pattern had no encoding short of abash:dispatcher spawning detached children with hand-rolled polling — out of process, with no native await, cost roll-up, or run tree. (feat(workflows): dynamic fan-out for workflow sub-run nodes (slice 2, PR-C) #2224)workflow:sub-run nodes. A sub-run node may declareisolation: worktreeto get its own checkout and branch instead of sharing the parent's. Isolation is explicit-only and never inferred fromfan_out— concurrent children on a shared checkout are refused by a spawn-time preflight rather than silently given a worktree. (feat(workflows): per-child worktree isolation for workflow: sub-run nodes (slice-2 PR-A) #2223)with:on aninclude:node plus the$INPUTS.<name>macro let one shared sub-DAG be reused with different values instead of forked. Substitution resolves entirely at load time, so the executor still sees a flat static DAG and load-time validation, resume, and the audit trail are unaffected. An unsupplied input fails the load rather than substituting silently. (feat: add load-time inputs for include nodes #2467)$STATE_DIRfor cross-run state. A per-project directory alongside$ARTIFACTS_DIR, pre-created by the executor and living outside the repository. It replaces the.archon/state/convention, which had no engine support at all — prompts didmkdir -p .archon/staterelative to cwd, so inside an isolated run the "cross-run memory" wrote to the worktree and died at cleanup, and in a user's repo it was stageable. A legacy directory produces one warning with the exactmvand is never moved. (feat(workflows): unify run output under one per-project tree in ~/.archon #2299)Changed
continue, and the two HTTP artifact routes — which is what allowed the artifact routes to silently fail for two of the three project kinds Archon can register. A singleresolveProjectStorageKeyin@archon/pathsnow backs all four call sites. (feat(workflows): unify run output under one per-project tree in ~/.archon #2299)output_rootpointer is recorded once at run start and never rewritten on resume, so historical runs keep resolving to the tree they actually wrote to even if the codebase is later renamed. (feat(workflows): unify run output under one per-project tree in ~/.archon #2299)archon validate workflows(human and--json), chat, the console workflow picker, and the API — each naming the node and the key, and persisted to the audit trail as aworkflow_parse_warningsevent. Warn rather than reject, so workflows that load today keep loading. (fix(workflows): surface unknown-key warnings where authors are, and correct the interactive hint #2455)Fixed
output_formatno longer silences an unparseable output. "No parseable object at all" was treated as a declared-optional field and resolved to empty on the declared-schema path while the schemaless path threw — so declaringoutput_formatmade a broken producer quieter than declaring nothing. It bit hardest onworkflow:sub-run nodes, where a child returning prose instead of JSON turned every declared field into an empty string with no error or warning. Both paths now fail. Genuine leniency is untouched: a field missing from a payload that actually parsed still resolves to empty. (fix(workflows): stop a declared output_format from silencing an unparseable output #2460).archon/artifacts/,.archon/logs/, and.archon/state/are documented as never belonging in git, but no bundled default told the agent to ignore them or refuse to stage them. Every "never stage" blocklist in the bundled defaults now lists them, and runs that create or modify a.gitignoremust include them. (fix(workflows): keep .archon telemetry out of target-repo PRs #2199)Merging this PR releases 0.8.0 to main.