feat(providers): add GitHub Copilot community provider (@github/copilot-sdk)

[popemkt]

Summary

• Problem: GitHub Copilot shipped a first-party SDK (@github/copilot-sdk) in 2025 that drives the Copilot CLI through a supported JSON-RPC bridge. Archon had no way to use it — the only path to Copilot would have been screen-scraping the interactive TUI.
• Why it matters: Copilot CLI gives users access to gpt-5, gpt-5-mini, and cross-family models (Claude via Copilot) under an existing GitHub billing relationship. This is the second community provider after Pi (feat(providers): add Pi community provider (@mariozechner/pi-coding-agent) coleam00/Archon#1270) and validates that the community-provider seam scales.
• What changed: New packages/providers/src/community/copilot/ wrapping @github/copilot-sdk, registered via registerCopilotProvider() with builtIn: false. Seven phased commits: initial scaffold → tool restrictions → MCP → skills → structured output → hardening fixes from CodeRabbit/Devin → sub-agents. Nine of the thirteen capability flags are wired end-to-end.
• What did NOT change (scope boundary): No hooks, no fallback model, no sandbox, no promotion to builtIn: true. Those remain deliberate follow-ups (plan preserved at .claude/archon/plans/copilot-provider-parity.md). Claude and Codex code paths untouched. Pi provider's behavior is preserved byte-for-byte via re-exports.

Architecture Diagram

Before

  @archon/workflows
        │
        │ IAgentProvider
        ▼
  @archon/providers ──┬── claude/                  ─── @anthropic-ai/claude-agent-sdk
  (registry)          ├── codex/                   ─── @openai/codex-sdk
                      └── community/pi/            === @mariozechner/pi-coding-agent

  registerCommunityProviders() — pi (builtIn: false)

After

  @archon/workflows
        │
        │ IAgentProvider  (unchanged contract)
        ▼
  @archon/providers ──┬── claude/                  ─── @anthropic-ai/claude-agent-sdk
  (registry)          ├── codex/                   ─── @openai/codex-sdk
                      ├── community/pi/            === @mariozechner/pi-coding-agent
                      ├── [+] community/copilot/   === @github/copilot-sdk
                      └── [+] shared/              ─── skills.ts, structured-output.ts
                                                       (extracted from pi/, pi re-exports)

  registerCommunityProviders() — pi + [+] copilot (both builtIn: false)

Shared extractions (no behavior change to Pi):

Extracted to	Source	Back-compat
shared/skills.ts::resolveSkillDirectories	pi/options-translator.ts::resolvePiSkills	Pi re-exports under the original name; all 14 existing Pi skill tests pass unchanged
shared/structured-output.ts::augmentPromptForJsonSchema	pi/provider.ts	Pi re-exports; no Pi test changes
shared/structured-output.ts::tryParseStructuredOutput	pi/event-bridge.ts	Pi re-exports; all 9 existing Pi JSON-parse tests pass unchanged

MCP reuse (no extraction): Copilot imports Claude's existing public loadMcpConfig re-export (packages/providers/src/claude/index.ts:4) directly. Env-var expansion and missing-var detection behave identically across providers.

Capability matrix

Field	Copilot	Notes
sessionResume	✅	Returns sessionId, reused on resume
envInjection	✅	Codebase env vars merged into spawned CLI environment
effortControl	✅	`effort: low
thinkingControl	✅	String form of thinking: accepted; object form warns (Claude-specific)
toolRestrictions	✅	allowed_tools → availableTools, denied_tools → excludedTools
mcp	✅	nodeConfig.mcp JSON → SessionConfig.mcpServers, $VAR expanded
skills	✅	skills: [name] → absolute dirs containing SKILL.md, missing names warn
structuredOutput	✅	Best-effort prompt-engineering (same pattern as Pi coleam00#1297)
agents	✅	nodeConfig.agents → customAgents; Claude-specific fields (model, disallowedTools, skills, maxTurns) warn per agent
hooks	❌	Copilot's SessionHooks event vocabulary diverges from Archon's Claude-shaped hook schema; deferred to avoid partial mapping
fallbackModel	❌	No native SDK field; one-shot retry would be adapter-level — deferred
costControl	❌	Not wired (no maxBudgetUsd enforcement)
sandbox	❌	Copilot's onPermissionRequest is policy control, not sandbox semantics; intentionally not declared equivalent

Implementation (7 commits)

1. 9546ea75 initial provider — CopilotProvider + parseCopilotConfig + config-aware resolveCopilotCliPath (env > config > vendor in binary mode > PATH > dev-undefined > throw). Auth: copilot login, COPILOT_GITHUB_TOKEN, GH_TOKEN, GITHUB_TOKEN. Registration wired at registerCommunityProviders(). SAFE_ASSISTANT_FIELDS gains copilot: ['model'].

2. f412b835 refactor + tool restrictions — Extracted single-source-of-truth buildSessionConfig() with a ProviderWarning[] collector, so each subsequent phase plugs in as one applyX(sessionConfig, …, warnings) call. applyToolRestrictions passes allowed_tools / denied_tools through verbatim (SDK tool names are canonical).

3. 94b7f472 MCP — applyMcpServers reuses Claude's public loadMcpConfig. Missing env vars surface as a consolidated system-warning chunk; IO / JSON errors propagate. enableConfigDiscovery stays false by default (see trust-boundary note in docs).

4. a19829064 skills — Extracted Pi's resolvePiSkills → shared/skills.ts::resolveSkillDirectories. Pi re-exports under the historical name so every existing Pi test passes unchanged. Copilot's applySkills maps names → absolute paths → SessionConfig.skillDirectories; unresolved names warn.

5. d7719bba structured output (best-effort) — Extracted Pi's augmentPromptForJsonSchema + tryParseStructuredOutput → shared/structured-output.ts. Same prompt-engineering + JSON-parse approach as Pi feat(providers/pi): best-effort structured output via prompt engineering coleam00/Archon#1297. Parse failure leaves structuredOutput unset so the executor's existing dag.structured_output_missing path handles degradation.

6. 8a3504d7 PR-review hardening — Addresses CodeRabbit + Devin findings:

   • Abort-guard: checks abortSignal.aborted before awaiting sendAndWait, since addEventListener('abort', ...) is a no-op on already-aborted signals and would otherwise enter the 24-hour timeout path after cancel.
   • Cleanup: session.disconnect() and client.stop() each in their own try/catch-log so a cleanup throw can't replace the primary result / friendly error.
   • Flips sawAssistantContent = true when emitting the final-message fallback, preventing a spurious session.error warning from double-firing.
   • Trims whitespace on model strings before assigning to SessionConfig.model.
   • Replaces existsSync() with isExecutableFile() (isFile + exec bit on posix, isFile on win32) so env/config/vendor overrides fail early.
   • Binary-resolver test switched to per-test mkdtempSync + chmod 0o755 — no shared /tmp/.archon state.
   • Docs: trust-boundary note added for enableConfigDiscovery.

7. 31d94d4d sub-agents — applyAgents maps nodeConfig.agents to SessionConfig.customAgents with name/description/prompt/tools (allowlist) passed through. Archon fields Copilot can't represent (model, disallowedTools, skills, maxTurns) warn per-agent. Intentionally does NOT set SessionConfig.agent — Archon invokes sub-agents via the Task tool, not by switching session-level agent.

Validation Evidence

bun run validate    # ← fully green end-to-end

• Static analysis: bun run type-check — all 10 packages exit 0; bun run lint — 0 errors 0 warnings; bun run format:check — clean.
• Tests: bun --filter @archon/providers test passes every per-file batch.
• New Copilot tests (9 files, 52 tests):

Test file	Tests
community/copilot/config.test.ts	3 (defensive parsing)
community/copilot/binary-resolver.test.ts	10 (env/config/vendor/PATH precedence + isExecutableFile)
community/copilot/provider.test.ts	6 (happy path, streaming, resume, reasoning warning, auth error)
community/copilot/tool-restrictions.test.ts	5 (pass-through, resume path)
community/copilot/mcp-translation.test.ts	4 (omit/load/env-expand/missing-file)
community/copilot/skills-translation.test.ts	4 (absent/resolved/missing/nothing-resolves)
community/copilot/structured-output.test.ts	7 (prompt augment, valid JSON, fences, unparseable, absent, fallback-msg parse)
community/copilot/agents-translation.test.ts	7 (pass-through, per-agent warning, ordering, Task-tool contract)
community/copilot/provider-hardening.test.ts	6 (abort-before-start, model trim, fallback flag, cleanup non-masking)

Plus 6 new tests in registry.test.ts exercising Copilot registration, idempotence, capability flags, and isModelCompatible. All 12 Pi tests that exercise the extracted shared utilities via the Pi re-exports pass unchanged (byte-for-byte identical function bodies).

Not yet validated (manual): end-to-end run of .archon/workflows/e2e-copilot-smoke.yaml with a real Copilot CLI auth. The smoke workflow asserts the happy path returns COPILOT_OK; first real-timing exercise happens when a reviewer runs it locally after copilot login (or setting COPILOT_GITHUB_TOKEN / GH_TOKEN / GITHUB_TOKEN).

Security Impact

• New permissions/capabilities? No — Copilot runs with the same process privileges as Claude / Codex / Pi (user-level, no elevation).
• New external network calls? Yes — the Copilot CLI calls GitHub's Copilot API. Equivalent surface to the other providers' SDK calls.
• Secrets/tokens handling changed? Tokens read from the per-request env + process.env (COPILOT_GITHUB_TOKEN, GH_TOKEN, GITHUB_TOKEN) and passed to CopilotClient({ githubToken, ... }). Not persisted by Archon. useLoggedInUser: true is the default when no token env var is present, delegating auth to the Copilot CLI's existing credential store.
• File-system access scope changed? No — the Copilot CLI operates under cwd like other providers. enableConfigDiscovery: false is Archon's default, preventing the CLI from auto-loading repo .mcp.json / skill dirs outside nodeConfig.mcp / nodeConfig.skills (documented as a trust boundary).
• Binary resolution hardening: isExecutableFile validates the resolved CLI path is a regular file with the exec bit set (posix), not a directory or non-executable, so env/config/vendor overrides fail early rather than passing a bad path to the SDK.

Compatibility / Migration

• Backward compatible? Yes — purely additive. Existing workflows untouched.
• Config/env changes? Optional — users may add assistants.copilot.model to .archon/config.yaml and supply auth via copilot login or one of three env vars. Nothing required for existing installs.
• Database migration needed? No.
• Shared-utility extractions are back-compat via re-exports — pi/options-translator.ts and pi/provider.ts / pi/event-bridge.ts re-export the extracted functions under their historical names. No external import paths change.

Human Verification

Verified scenarios (unit tests against mocked Copilot SDK + real filesystem/env-var fixtures):

• Assistant / reasoning / tool / tool_result / result chunk translation
• Session resume (reuses resumeSession path)
• Effort mapping: low|medium|high|xhigh|max → reasoningEffort; string thinking warns; off disables
• System prompt pass-through
• Env-var merge with caller-wins semantics
• Binary resolver precedence: env > config > vendor-in-binary-mode > PATH > dev-undefined > throw
• isExecutableFile rejects directories, missing paths, and non-executable files on posix
• allowed_tools / denied_tools pass-through on both create and resume paths
• MCP JSON load, $VAR expansion, missing-var warning, relative-path resolution, malformed-file error
• Skill resolution from .agents/skills/ / .claude/skills/ (project or home), missing-skill warning
• Best-effort JSON parse: valid JSON, json / fences, prose-wrapped → no structuredOutput, absent outputFormat → never set
• Sub-agent mapping: description + prompt + tools pass through; model / disallowedTools / skills / maxTurns warn; empty object / absent → omitted; SessionConfig.agent never set
• Abort-before-start rejects with AbortError and never enters sendAndWait
• Model string trim on both requestOptions.model and assistantConfig.model
• Cleanup (disconnect + stop) failures logged and swallowed, never mask the primary result or the friendly auth/model-access error
• Final-message fallback flips sawAssistantContent, preventing double-emission of a session.error warning

What was NOT verified:

• Live end-to-end run with real Copilot CLI auth (smoke workflow staged, waiting on reviewer)
• Behavior against every Copilot model (gpt-5, gpt-5-mini, Claude-via-Copilot aliases)
• Copilot SDK's hook / permission semantics under an actual permission-request event (we default to approveAll, matching Claude / Codex's bypassPermissions default)

Side Effects / Blast Radius

• Affected subsystems: only workflows / chat sessions that opt in via provider: copilot. Claude, Codex, and Pi code paths are untouched at runtime. The two Pi shared-utility call sites go through re-exports, so Pi's observable behavior is byte-identical.
• Bundle size: adds @github/copilot-sdk (~few MB, includes the CLI bridge types). Loaded only when CopilotProvider.sendQuery runs.
• Registry surface: GET /api/providers returns 4 entries (claude, codex, copilot, pi). Web UI already iterates generically.
• Logs added: copilot.binary_resolved, copilot.mcp_loaded, copilot.mcp_env_vars_missing (warn), copilot.skills_resolved, copilot.agents_registered, copilot.abort_failed (warn), copilot.disconnect_failed (warn), copilot.client_stop_errors (warn), copilot.client_stop_threw (warn). All under the provider.copilot Pino module, matching the {domain}.{action}_{state} convention.

Rollback Plan

• Fast rollback: revert the PR merge commit on dev. Seven commits, purely additive, no schema or interface changes — revert is clean.
• User-side rollback: remove provider: copilot from workflow YAML or assistants.copilot from .archon/config.yaml.
• Observable failure symptoms:
  • copilot.*_failed / copilot.*_errors / copilot.*_threw log events under provider.copilot
  • User-visible ⚠️ system-chunk messages for missing skills, missing MCP env vars, unsupported thinking / effort shapes, and ignored per-agent fields
  • Workflow node failures on provider: copilot nodes with friendly messages for auth errors, model-access errors, or binary-not-found

Risks and Mitigations

• Risk: @github/copilot-sdk is pre-1.0 (v0.2.2) — breaking changes possible on minor bumps.
  • Mitigation: pinned ^0.2.2. The 52 new Copilot tests plus type-check gate any API-surface drift before merge.
• Risk: Generator abandonment without an AbortSignal can leave a Copilot session holding resources until sendAndWait resolves (up to 24 h).
  • Mitigation: the dag-executor (primary caller) always threads an AbortSignal, limiting blast radius. A follow-up can add an internal AbortController wired into the generator's finally to cover the abandonment path. Tracked in a review comment on this PR.
• Risk: Best-effort structured output depends on model instruction-following, not SDK enforcement.
  • Mitigation: same Pi feat(providers/pi): best-effort structured output via prompt engineering coleam00/Archon#1297 pattern — fenced-code stripper catches the common compliance slip; parse failure degrades via the executor's existing dag.structured_output_missing warning. Documented as "best-effort" in the capability matrix and docs.
• Risk: enableConfigDiscovery: true would let the Copilot CLI load repo-level config (MCP servers, skill dirs) outside Archon's workflow validation surface.
  • Mitigation: defaults to false. Docs carry an explicit trust-boundary note (packages/docs-web/.../ai-assistants.md).
• Risk: Cross-provider coupling — loadMcpConfig is imported directly from Claude's module instead of extracted to shared/.
  • Mitigation: loadMcpConfig is a public re-export in packages/providers/src/claude/index.ts, not an internal detail. By CLAUDE.md's Rule of Three, extraction to shared/mcp.ts can wait until a third provider needs it.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• New Features

  • Added GitHub Copilot as a supported AI assistant
  • Introduced script nodes for deterministic TypeScript/Python execution in workflows
  • Approval gates now capture optional rejection reasons passed to rejection prompts
  • Workflow tags enable organization and categorization
  • Web UI node deletion via context menu and header button

• Bug Fixes

  • Improved worktree registration error messages for stale symlinks
  • Discord adapter failures no longer block server startup

• Documentation

  • Added Copilot provider setup guide and script nodes documentation
  • Expanded workflow authoring best practices and troubleshooting guides

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This release introduces GitHub Copilot as a new community provider with full SDK integration, refactors the Pi provider for lazy loading, adds shared structured-output and skills utilities, extends DAG workflows with script/approval/cancel nodes, implements approval auto-resume on web, and includes extensive documentation updates and release infrastructure improvements. Version bumped to 0.3.9.

Changes

Cohort / File(s)	Summary
GitHub Copilot Community Provider packages/providers/src/community/copilot/*, packages/providers/package.json, packages/providers/src/index.ts	New CopilotProvider implementation with binary resolver, session management, streaming chunks, tool restrictions, MCP/skills translation, structured output, and capabilities matrix. Adds @github/copilot-sdk dependency and exports Copilot provider API surface.
Copilot Provider Tests packages/providers/src/community/copilot/*.test.ts	Comprehensive test coverage for binary resolution, config parsing, provider streaming, MCP/skills/agents/tool-restriction translation, structured output, provider hardening, and session resumption scenarios.
Pi Provider Refactoring packages/providers/src/community/pi/provider.ts, packages/providers/src/community/pi/options-translator.ts, packages/providers/src/community/pi/event-bridge.ts	Lazy-loads Pi SDK via dynamic imports at sendQuery() time, adds package dir shim for compiled deployments, delegates model lookup to dynamic ModelRegistry, extracted shared utilities, and improves auth handling with structured logging.
Pi Provider Lazy-Load Verification packages/providers/src/community/pi/provider-lazy-load.test.ts	New regression test ensuring Pi SDK packages are not eagerly imported during module registration.
Shared Structured-Output & Skills Utilities packages/providers/src/shared/structured-output.ts, packages/providers/src/shared/skills.ts	New shared modules for JSON schema prompt augmentation and structured-output parsing, plus provider-agnostic skill directory resolution by name, extracted for reuse across providers.
Provider Registry & Tests packages/providers/src/registry.ts, packages/providers/src/registry.test.ts, packages/providers/src/claude/binary-resolver.*, packages/providers/src/codex/binary-resolver.*	Added Copilot provider registration, extended Claude/Codex binary resolvers to support home-directory autodetection, added comprehensive resolver tests for multiple fallback paths.
Workflow Schema Extensions packages/workflows/src/schemas/workflow.ts, packages/workflows/src/loader.ts, packages/workflows/src/executor-shared.ts, packages/workflows/src/dag-executor.ts	Added script node support, approval node support, cancel node support, workflow-level tags, completion signal detection for multiple formats, MCP failure filtering, streaming policy for paused status, and updated loop handling.
Workflow Tests packages/workflows/src/loader.test.ts, packages/workflows/src/dag-executor.test.ts, packages/workflows/src/executor-shared.test.ts, packages/workflows/src/defaults/bundled-defaults.test.ts	Extended test coverage for script/approval/cancel node parsing, tag normalization, MCP filtering, completion signals, loop/DAG finalization, and bundled workflow validation.
Orchestrator & Auto-Resume packages/core/src/orchestrator/orchestrator*.ts, packages/server/src/routes/api.ts, packages/server/src/routes/api.workflow-runs.test.ts	Added parentConversationId to workflow execution for web approval auto-resume, implemented tryAutoResumeAfterGate() helper for orchestrator dispatch on approval/rejection, extended API approval/reject handlers to support optional rejection reasons with on-reject prompt wiring.
Web UI Approval & Node Management packages/web/src/components/chat/WorkflowProgressCard.tsx, packages/web/src/components/dashboard/*, packages/web/src/hooks/useBuilderKeyboard.*	Updated rejection flows to capture optional reason text, added ConfirmRunActionDialog for rejection reason input, implemented node context-menu deletion in WorkflowCanvas, added keyboard shortcut support (Delete/Backspace) with input-target awareness, extracted testable keyboard handler functions.
Workflow Metadata & Tags packages/web/src/lib/workflow-metadata.*, packages/web/src/components/workflows/WorkflowCard.tsx, packages/web/src/lib/api.generated.d.ts	Added explicit tags field to workflow definitions, updated getWorkflowTags to honor explicit tags with inference fallback, updated generated OpenAPI types for workflow tags and health-response platforms list.
Release & CI Improvements scripts/build-binaries.sh, .github/workflows/release.yml, CHANGELOG.md, homebrew/archon.rb	Removed bytecode generation flag from binary builds, updated smoke tests to use --no-worktree flag, added release notes for 0.3.9/0.3.8/0.3.7, updated Homebrew formula checksums.
Release Skill & Workflow Documentation .claude/skills/release/SKILL.md, .claude/skills/test-release/SKILL.md	Enhanced release skill with compiled-binary smoke test (Step 1.5), improved CI failure detection and rerun logic, added recovery procedures; updated test-release skill with archon-stable symlink preservation for brew installations.
Maintainer Workflows & Commands .archon/workflows/maintainer/*, .archon/commands/maintainer-*, .archon/scripts/maintainer-standup-*, .archon/maintainer-standup/*	New maintainer-review-pr workflow with gating, aspect-specific reviews, synthesis, and auto-commenting; new maintainer-standup workflow with git/GitHub data gathering, triage, and state persistence; supporting scripts and documentation.
Workflow-Builder & Node Documentation packages/docs-web/src/content/docs/guides/script-nodes.md, packages/docs-web/src/content/docs/book/dag-workflows.md, .claude/skills/archon/*, .archon/workflows/defaults/archon-*.yaml	Comprehensive guides for script nodes (bun/uv execution), approval gates, cancellation, tags, good practices, parameter matrix, troubleshooting, and repo init; updated default workflow templates with model references and artifact management.
E2E Test Workflows .archon/workflows/test-workflows/e2e-copilot-*.yaml, .archon/workflows/test-workflows/e2e-minimax-smoke.yaml	New Copilot smoke/feature test workflows, updated Pi provider E2E tests for model validation and session verification.
Configuration & Miscellaneous package.json, packages/*/package.json, eslint.config.mjs, .gitignore, CLAUDE.md, CONTRIBUTING.md, packages/server/src/index.ts, packages/web/src/routes/DashboardPage.tsx	Version bumps to 0.3.9 across all packages, axios override, ESLint ignores, maintainer-standup artifacts in .gitignore, PR template guidance, Discord adapter fault tolerance with continued-operation mode, rejection handler refactoring.

Sequence Diagram(s)

sequenceDiagram
    participant Web as Web UI
    participant API as API Server
    participant Orch as Orchestrator
    participant Exec as Workflow<br/>Executor

    Web->>API: PUT /approve (with reason?)
    activate API
    API->>API: Record approval event
    API->>API: Update workflow state
    
    alt Has parent_conversation_id
        API->>API: Fetch parent conversation
        alt Parent is web platform
            API->>Orch: Dispatch resume message<br/>(/workflow run ...)
            Note over API,Orch: tryAutoResumeAfterGate()
            Orch->>Exec: Execute workflow<br/>(with parent context)
            Exec-->>Orch: Results
            Orch-->>API: Dispatch complete
            API-->>Web: "Resuming workflow."
        else Non-web parent
            API-->>Web: "Send a message to continue."
        end
    else No parent context
        API-->>Web: "Send a message to continue."
    end
    deactivate API

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

This PR introduces substantial new functionality (GitHub Copilot provider with 600+ lines of implementation and tests), refactors existing critical provider code (Pi lazy-loading), extends workflow schema with new node types and capabilities, modifies approval/orchestration flow for auto-resume, updates Web UI components for node management and rejection workflow, and includes extensive documentation and infrastructure changes. The changes are heterogeneous, affecting multiple subsystems with varying complexity and density (provider internals, workflow engine, server API, Web UI, documentation). The variety of file types and reasoning patterns (provider SDK integration, workflow DSL schema, server orchestration, React components, Bash scripts, documentation, YAML workflows) compounds review effort significantly.

Poem

🐰 A copilot now soars through workflows so grand,
Scripts run without AI, by developer's hand.
Approvals auto-resume with reason and care,
Tags float on workflows—oh what a fair affair!
From Pi's lazy loading to Opus's new call,
This release hops forward, delighting us all! 🌟

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch emdash/add-copilot-2er

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (5)

packages/providers/package.json (1)

22-22: Test script is getting unwieldy — consider consolidating.

The test script now chains ~25 individual bun test <file> invocations with &&, which fails-fast on the first failing suite (so later suites won't run and give feedback) and keeps growing with each new test file. Bun supports passing multiple file arguments or a glob in a single invocation.

♻️ Suggested simplification

-    "test": "bun test src/claude/provider.test.ts && bun test src/codex/provider.test.ts && bun test src/registry.test.ts && bun test src/codex/binary-guard.test.ts && bun test src/codex/binary-resolver.test.ts && bun test src/codex/binary-resolver-dev.test.ts && bun test src/claude/binary-resolver.test.ts && bun test src/claude/binary-resolver-dev.test.ts && bun test src/community/pi/model-ref.test.ts && bun test src/community/pi/config.test.ts && bun test src/community/pi/event-bridge.test.ts && bun test src/community/pi/options-translator.test.ts && bun test src/community/pi/session-resolver.test.ts && bun test src/community/pi/provider.test.ts && bun test src/community/copilot/config.test.ts && bun test src/community/copilot/binary-resolver.test.ts && bun test src/community/copilot/provider.test.ts && bun test src/community/copilot/tool-restrictions.test.ts && bun test src/community/copilot/mcp-translation.test.ts && bun test src/community/copilot/skills-translation.test.ts && bun test src/community/copilot/structured-output.test.ts && bun test src/community/copilot/provider-hardening.test.ts && bun test src/community/copilot/agents-translation.test.ts",
+    "test": "bun test src/**/*.test.ts",

If the original sequencing matters (e.g., registry tests relying on other files' mocks having run first), the per-file flow can be preserved while still running the whole set via a single bun process.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/providers/package.json` at line 22, The test script in package.json
currently chains many individual "bun test <file>" commands with &&; replace
this with a single bun invocation that accepts multiple files or a glob to run
all tests in one process (e.g., use bun test "src/**/*.test.ts" or pass multiple
file args to bun test) to simplify maintenance and avoid the long && chain;
update the "test" npm script entry (the "test" property) to use that single bun
test command while preserving any required test ordering if necessary.

packages/providers/src/community/pi/event-bridge.ts (1)

154-158: Hoist the import to the top of the file.

Same note as in packages/providers/src/community/pi/provider.ts: the mid-file import { tryParseStructuredOutput } from '../../shared/structured-output'; is purely stylistic — ES imports are hoisted — but it tends to trip lint rules and makes the module's dependency graph harder to scan. Prefer a top-of-file import plus either a separate export { tryParseStructuredOutput } there or a single export { tryParseStructuredOutput } from '../../shared/structured-output'; re-export to preserve the back-compat surface.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/providers/src/community/pi/event-bridge.ts` around lines 154 - 158,
Move the inline import of tryParseStructuredOutput to the top of the module and
replace the mid-file import+export with a top-level re-export; specifically,
remove the mid-file "import { tryParseStructuredOutput } ..." and instead add
either a top-of-file "import { tryParseStructuredOutput }" followed by an
"export { tryParseStructuredOutput }" or a single top-level "export {
tryParseStructuredOutput } from '...';" so the symbol tryParseStructuredOutput
is declared/re-exported at the module root for linters and clearer dependency
scanning.

packages/providers/src/community/pi/provider.ts (1)

68-72: Move the import to the top of the file.

Placing import { augmentPromptForJsonSchema } from '../../shared/structured-output'; in the middle of the module is non-idiomatic — ES module imports are hoisted anyway, so there's no semantic benefit from the placement, and most lint configs (import/first, @typescript-eslint conventions) will flag it. Move the import up with the other imports and keep the export { augmentPromptForJsonSchema }; re-export alongside the comment at the current location if you want the "shared-but-re-exported" signal, or convert to a single top-level export { augmentPromptForJsonSchema } from '../../shared/structured-output';.

♻️ Proposed refactor

At the top of the file, alongside the existing imports:

 import { createArchonUIBridge, createArchonUIContext } from './ui-context-stub';
+import { augmentPromptForJsonSchema } from '../../shared/structured-output';

And replace lines 68-72 with a single re-export:

-// Structured-output prompt augmentation is shared across providers. Import
-// once for local use and re-export so existing callers and tests keep their
-// import path stable; new providers should import from `../../shared/structured-output`.
-import { augmentPromptForJsonSchema } from '../../shared/structured-output';
-export { augmentPromptForJsonSchema };
+// Structured-output prompt augmentation is shared across providers. Re-exported
+// here so existing Pi callers/tests keep their import path stable; new providers
+// should import from `../../shared/structured-output`.
+export { augmentPromptForJsonSchema } from '../../shared/structured-output';

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/providers/src/community/pi/provider.ts` around lines 68 - 72, Move
the import of augmentPromptForJsonSchema to the top with the other imports and
replace the mid-file import with a re-export; specifically, remove the inline
"import { augmentPromptForJsonSchema } from '../../shared/structured-output';"
from the middle of the module, add the import or single-line re-export at the
top alongside other imports, and keep or add "export {
augmentPromptForJsonSchema }" (or use "export { augmentPromptForJsonSchema }
from '../../shared/structured-output';") to preserve the public API; update
references to augmentPromptForJsonSchema accordingly.

packages/providers/src/shared/structured-output.ts (1)

51-54: Fence stripping accepts only json or unlabeled fences.

The regex ^\``(?:json)?\s*\n?strips ```` ``` ```` and ```` ```json ```` but leaves other labels (e.g. ```` ```javascript ````, ```` ```ts ````) in place, which will causeJSON.parseto fail and returnundefined. That's a safe degradation, but if instruction-following drift produces labels like javascriptortypescript` it will silently miss the structured output. Consider widening the match to any identifier, since the fence context already implies the content is supposed to be the JSON:

♻️ Suggested tweak

-    .replace(/^```(?:json)?\s*\n?/i, '')
+    .replace(/^```[\w-]*\s*\n?/i, '')

This accepts any single-word language tag (including empty) without compromising the boundary anchoring.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/providers/src/shared/structured-output.ts` around lines 51 - 54, The
opening-fence strip in the cleaned variable only matches unlabeled or "json"
fences; update the first .replace used to build cleaned (the one stripping the
opening ``` fence) so it accepts any single-word language tag
(letters/digits/underscore/hyphen) or no tag instead of only "json", while
preserving the start-anchor and optional trailing newline/whitespace; keep the
existing closing-fence strip unchanged so the overall trimming still removes
fenced blocks before JSON.parse.

packages/providers/src/community/copilot/tool-restrictions.test.ts (1)

110-123: Add a test case for overlapping allow/deny lists to verify documented SDK precedence.

The current test uses disjoint lists and doesn't validate the documented behavior where availableTools takes precedence when a tool appears in both lists. Add a case with the same tool in both allowed_tools and denied_tools to ensure the provider correctly propagates this conflict to the SDK, which will handle it per its documented contract.

Suggested test case

   test('passes both through when both present (SDK enforces availableTools precedence)', async () => {
     await drain(
       new CopilotProvider().sendQuery('hi', '/repo', undefined, {
         model: 'gpt-5',
         nodeConfig: {
           allowed_tools: ['read_file'],
           denied_tools: ['shell'],
         },
       })
     );

     const cfg = capturedSessionConfigs[0] ?? {};
     expect(cfg.availableTools).toEqual(['read_file']);
     expect(cfg.excludedTools).toEqual(['shell']);
   });
+
+  test('defines behavior when the same tool is both allowed and denied', async () => {
+    await drain(
+      new CopilotProvider().sendQuery('hi', '/repo', undefined, {
+        model: 'gpt-5',
+        nodeConfig: {
+          allowed_tools: ['shell', 'read_file'],
+          denied_tools: ['shell'],
+        },
+      })
+    );
+
+    const cfg = capturedSessionConfigs[0] ?? {};
+    expect(cfg.availableTools).toEqual(['shell', 'read_file']);
+    expect(cfg.excludedTools).toEqual(['shell']);
+  });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/providers/src/community/copilot/tool-restrictions.test.ts` around
lines 110 - 123, Add a new test that sends a query via CopilotProvider.sendQuery
with the same tool in both nodeConfig.allowed_tools and nodeConfig.denied_tools
(e.g., 'read_file') and then assert that the capturedSessionConfigs entry for
that session still contains both availableTools and excludedTools entries
listing that tool (e.g., expect(cfg.availableTools).toEqual(['read_file']) and
expect(cfg.excludedTools).toEqual(['read_file'])); this verifies the provider
propagates overlapping allow/deny lists to the SDK which enforces the documented
precedence.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/providers/src/community/copilot/provider-hardening.test.ts`:
- Around line 108-123: Test currently only ensures sendAndWait is not called but
doesn't ensure the provider avoids creating an external Copilot session; update
CopilotProvider.sendQuery to check abortSignal?.aborted at the very start and
throw an AbortError before any session setup or external calls, and add/adjust
the test to assert that any session-creation mock (e.g., mockCreateSession or
whatever function initializes a Copilot session) is not called in addition to
mockSendAndWait remaining at 0 calls.

In `@packages/providers/src/community/copilot/skills-translation.test.ts`:
- Around line 123-127: The assertions currently use toContain which allows
partial matches; tighten them to assert exact absolute paths by replacing the
two toContain checks with exact equality checks against resolved absolute paths
(e.g., compute expectedAlpha = path.resolve(join('.agents','skills','alpha'))
and expectedBeta = path.resolve(join('.claude','skills','beta')) and then assert
dirs?.[0] === expectedAlpha and dirs?.[1] === expectedBeta using toBe or
toEqual). Update the test variables capturedSessionConfigs, cfg.skillDirectories
and the two expectations accordingly so the test fails on relative-path
regressions.

In `@packages/providers/src/shared/skills.ts`:
- Around line 62-74: The loop that builds candidate = join(root, rawName) in the
skill resolution (inside the for...of skillNames loop, using seen, roots,
candidate, existsSync(join(candidate, 'SKILL.md'))) is vulnerable to
path-traversal via rawName; validate/sanitize rawName before joining: reject or
skip names that are absolute or contain ".." or path separators that would
escape the root, or safer — compute candidate = resolve(root, rawName) and then
ensure path.relative(root, candidate) does not start with ".." and candidate is
not absolute; only then call existsSync(join(candidate, 'SKILL.md')). Apply this
check where candidate is constructed so malicious rawName values cannot probe
outside the intended roots.

---

Nitpick comments:
In `@packages/providers/package.json`:
- Line 22: The test script in package.json currently chains many individual "bun
test <file>" commands with &&; replace this with a single bun invocation that
accepts multiple files or a glob to run all tests in one process (e.g., use bun
test "src/**/*.test.ts" or pass multiple file args to bun test) to simplify
maintenance and avoid the long && chain; update the "test" npm script entry (the
"test" property) to use that single bun test command while preserving any
required test ordering if necessary.

In `@packages/providers/src/community/copilot/tool-restrictions.test.ts`:
- Around line 110-123: Add a new test that sends a query via
CopilotProvider.sendQuery with the same tool in both nodeConfig.allowed_tools
and nodeConfig.denied_tools (e.g., 'read_file') and then assert that the
capturedSessionConfigs entry for that session still contains both availableTools
and excludedTools entries listing that tool (e.g.,
expect(cfg.availableTools).toEqual(['read_file']) and
expect(cfg.excludedTools).toEqual(['read_file'])); this verifies the provider
propagates overlapping allow/deny lists to the SDK which enforces the documented
precedence.

In `@packages/providers/src/community/pi/event-bridge.ts`:
- Around line 154-158: Move the inline import of tryParseStructuredOutput to the
top of the module and replace the mid-file import+export with a top-level
re-export; specifically, remove the mid-file "import { tryParseStructuredOutput
} ..." and instead add either a top-of-file "import { tryParseStructuredOutput
}" followed by an "export { tryParseStructuredOutput }" or a single top-level
"export { tryParseStructuredOutput } from '...';" so the symbol
tryParseStructuredOutput is declared/re-exported at the module root for linters
and clearer dependency scanning.

In `@packages/providers/src/community/pi/provider.ts`:
- Around line 68-72: Move the import of augmentPromptForJsonSchema to the top
with the other imports and replace the mid-file import with a re-export;
specifically, remove the inline "import { augmentPromptForJsonSchema } from
'../../shared/structured-output';" from the middle of the module, add the import
or single-line re-export at the top alongside other imports, and keep or add
"export { augmentPromptForJsonSchema }" (or use "export {
augmentPromptForJsonSchema } from '../../shared/structured-output';") to
preserve the public API; update references to augmentPromptForJsonSchema
accordingly.

In `@packages/providers/src/shared/structured-output.ts`:
- Around line 51-54: The opening-fence strip in the cleaned variable only
matches unlabeled or "json" fences; update the first .replace used to build
cleaned (the one stripping the opening ``` fence) so it accepts any single-word
language tag (letters/digits/underscore/hyphen) or no tag instead of only
"json", while preserving the start-anchor and optional trailing
newline/whitespace; keep the existing closing-fence strip unchanged so the
overall trimming still removes fenced blocks before JSON.parse.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 147adf7f-4bce-4022-b6a6-1e57d6268099

📥 Commits

Reviewing files that changed from the base of the PR and between 9546ea7 and e0b57a8.

📒 Files selected for processing (18)

• packages/docs-web/src/content/docs/getting-started/ai-assistants.md
• packages/providers/package.json
• packages/providers/src/community/copilot/agents-translation.test.ts
• packages/providers/src/community/copilot/binary-resolver.test.ts
• packages/providers/src/community/copilot/binary-resolver.ts
• packages/providers/src/community/copilot/capabilities.ts
• packages/providers/src/community/copilot/mcp-translation.test.ts
• packages/providers/src/community/copilot/provider-hardening.test.ts
• packages/providers/src/community/copilot/provider.ts
• packages/providers/src/community/copilot/skills-translation.test.ts
• packages/providers/src/community/copilot/structured-output.test.ts
• packages/providers/src/community/copilot/tool-restrictions.test.ts
• packages/providers/src/community/pi/event-bridge.ts
• packages/providers/src/community/pi/options-translator.ts
• packages/providers/src/community/pi/provider.ts
• packages/providers/src/registry.test.ts
• packages/providers/src/shared/skills.ts
• packages/providers/src/shared/structured-output.ts

✅ Files skipped from review due to trivial changes (1)

• packages/providers/src/community/copilot/capabilities.ts

🚧 Files skipped from review as they are similar to previous changes (4)

• packages/docs-web/src/content/docs/getting-started/ai-assistants.md
• packages/providers/src/community/copilot/binary-resolver.test.ts
• packages/providers/src/community/copilot/binary-resolver.ts
• packages/providers/src/community/copilot/provider.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Assert already-aborted requests do not create a Copilot session.

This currently only proves sendAndWait is skipped. For a true early-abort guard, the provider should avoid the external session setup path too.

Proposed test assertion

     expect(error).toBeDefined();
     expect(error?.name).toBe('AbortError');
+    expect(mockCreateSession).toHaveBeenCalledTimes(0);
     // sendAndWait must NOT have been entered
     expect(mockSendAndWait).toHaveBeenCalledTimes(0);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	test('rejects early when abortSignal is already aborted', async () => {
	const controller = new AbortController();
	controller.abort();
	const { error } = await collect(
	new CopilotProvider().sendQuery('hi', '/repo', undefined, {
	model: 'gpt-5',
	abortSignal: controller.signal,
	})
	);
	expect(error).toBeDefined();
	expect(error?.name).toBe('AbortError');
	// sendAndWait must NOT have been entered
	expect(mockSendAndWait).toHaveBeenCalledTimes(0);
	});
	test('rejects early when abortSignal is already aborted', async () => {
	const controller = new AbortController();
	controller.abort();
	const { error } = await collect(
	new CopilotProvider().sendQuery('hi', '/repo', undefined, {
	model: 'gpt-5',
	abortSignal: controller.signal,
	})
	);
	expect(error).toBeDefined();
	expect(error?.name).toBe('AbortError');
	expect(mockCreateSession).toHaveBeenCalledTimes(0);
	// sendAndWait must NOT have been entered
	expect(mockSendAndWait).toHaveBeenCalledTimes(0);
	});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/providers/src/community/copilot/provider-hardening.test.ts` around
lines 108 - 123, Test currently only ensures sendAndWait is not called but
doesn't ensure the provider avoids creating an external Copilot session; update
CopilotProvider.sendQuery to check abortSignal?.aborted at the very start and
throw an AbortError before any session setup or external calls, and add/adjust
the test to assert that any session-creation mock (e.g., mockCreateSession or
whatever function initializes a Copilot session) is not called in addition to
mockSendAndWait remaining at 0 calls.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Assert exact absolute skill directories.

This test says it verifies absolute paths, but toContain(...) would also pass for relative suffixes. Pin the full expected paths so SDK-breaking relative-path regressions fail.

Proposed assertion tightening

     const cfg = capturedSessionConfigs[0] ?? {};
     const dirs = cfg.skillDirectories as string[] | undefined;
     expect(dirs).toHaveLength(2);
-    expect(dirs?.[0]).toContain(join('.agents', 'skills', 'alpha'));
-    expect(dirs?.[1]).toContain(join('.claude', 'skills', 'beta'));
+    expect(dirs).toEqual([
+      join(workDir, '.agents', 'skills', 'alpha'),
+      join(tmpRoot, 'home', '.claude', 'skills', 'beta'),
+    ]);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const cfg = capturedSessionConfigs[0] ?? {};
	const dirs = cfg.skillDirectories as string[] | undefined;
	expect(dirs).toHaveLength(2);
	expect(dirs?.[0]).toContain(join('.agents', 'skills', 'alpha'));
	expect(dirs?.[1]).toContain(join('.claude', 'skills', 'beta'));
	const cfg = capturedSessionConfigs[0] ?? {};
	const dirs = cfg.skillDirectories as string[] | undefined;
	expect(dirs).toHaveLength(2);
	expect(dirs).toEqual([
	join(workDir, '.agents', 'skills', 'alpha'),
	join(tmpRoot, 'home', '.claude', 'skills', 'beta'),
	]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/providers/src/community/copilot/skills-translation.test.ts` around
lines 123 - 127, The assertions currently use toContain which allows partial
matches; tighten them to assert exact absolute paths by replacing the two
toContain checks with exact equality checks against resolved absolute paths
(e.g., compute expectedAlpha = path.resolve(join('.agents','skills','alpha'))
and expectedBeta = path.resolve(join('.claude','skills','beta')) and then assert
dirs?.[0] === expectedAlpha and dirs?.[1] === expectedBeta using toBe or
toEqual). Update the test variables capturedSessionConfigs, cfg.skillDirectories
and the two expectations accordingly so the test fails on relative-path
regressions.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Skill name is joined without path-traversal sanitization.

join(root, rawName) will happily resolve .. segments (e.g. rawName = "../../etc" → candidate outside any intended root). Because the existence check is on join(candidate, 'SKILL.md') the practical impact is bounded — an attacker would need a SKILL.md to already exist at the traversed path — but:

1. It still lets a crafted skills: entry probe the filesystem outside the four documented roots, which leaks information via the warning string (missing list names).
2. If a future change ever starts reading files from the resolved paths beyond SKILL.md (or passes them to a model/shell), this becomes a real traversal vector.

Skill names come from workflow YAML (usually trusted), so this is minor, but an early reject is cheap insurance:

🛡️ Suggested validation

   for (const rawName of skillNames) {
     if (typeof rawName !== 'string' || rawName.length === 0) continue;
+    // Reject path separators and traversal segments — skill names must be
+    // simple directory names within the configured roots.
+    if (rawName.includes('/') || rawName.includes('\\') || rawName === '..' || rawName === '.') {
+      missing.push(rawName);
+      continue;
+    }
     if (seen.has(rawName)) continue;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	for (const rawName of skillNames) {
	if (typeof rawName !== 'string' || rawName.length === 0) continue;
	if (seen.has(rawName)) continue;
	seen.add(rawName);
	let found: string | undefined;
	for (const root of roots) {
	const candidate = join(root, rawName);
	if (existsSync(join(candidate, 'SKILL.md'))) {
	found = candidate;
	break;
	}
	}
	for (const rawName of skillNames) {
	if (typeof rawName !== 'string' || rawName.length === 0) continue;
	// Reject path separators and traversal segments — skill names must be
	// simple directory names within the configured roots.
	if (rawName.includes('/') || rawName.includes('\\') || rawName === '..' || rawName === '.') {
	missing.push(rawName);
	continue;
	}
	if (seen.has(rawName)) continue;
	seen.add(rawName);
	let found: string | undefined;
	for (const root of roots) {
	const candidate = join(root, rawName);
	if (existsSync(join(candidate, 'SKILL.md'))) {
	found = candidate;
	break;
	}
	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/providers/src/shared/skills.ts` around lines 62 - 74, The loop that
builds candidate = join(root, rawName) in the skill resolution (inside the
for...of skillNames loop, using seen, roots, candidate,
existsSync(join(candidate, 'SKILL.md'))) is vulnerable to path-traversal via
rawName; validate/sanitize rawName before joining: reject or skip names that are
absolute or contain ".." or path separators that would escape the root, or safer
— compute candidate = resolve(root, rawName) and then ensure path.relative(root,
candidate) does not start with ".." and candidate is not absolute; only then
call existsSync(join(candidate, 'SKILL.md')). Apply this check where candidate
is constructed so malicious rawName values cannot probe outside the intended
roots.

[popemkt]

Status of @Wirasm's asks on coleam00#1351

His comment (link) asks for four things before merge. Here's what's done locally and what's still on you.

1. Models tested

Live, end-to-end on Linux + Copilot CLI 1.0.36 (/stuff/WorkSpace/Archon/node_modules/.bin/copilot):

Model	Workflow	Result
gpt-5-mini	e2e-copilot-smoke	PASS — 12s
gpt-5-mini	e2e-copilot-all-features	PASS — 25s, all 4 capabilities

The PR description also lists three earlier scenarios on Windows + CLI 1.0.31 with gpt-5-mini. So we have evidence on two OSes / two CLI minor versions for gpt-5-mini.

I did not run live tests for gpt-5 or a Claude-via-Copilot model — happy to add those if you give the go-ahead, will burn ~30s of quota each.

2. Configs used

Auth on this Linux box: copilot login (logged-in user — no env var). The provider falls back to useLoggedInUser: true when no token env is set. Other supported envs (COPILOT_GITHUB_TOKEN, GH_TOKEN, GITHUB_TOKEN) are documented in packages/docs-web/src/content/docs/getting-started/ai-assistants.md.

No MCP, skills, or extension config staged for the live runs — those are exercised by the unit tests (52 tests in packages/providers/src/community/copilot/) but I haven't shipped a live-MCP smoke yet. Worth doing if Wirasm pushes back.

enableConfigDiscovery defaults to false (Archon trust boundary).

3. Results — what was tested

e2e-copilot-all-features exercises four capabilities in one DAG:

── results ──
hello       = PONG          ← basic chat round-trip
reasoning   = 391           ← effort: high → reasoningEffort='high' (17×23)
restricted  = DENIED_OK     ← denied_tools: [shell, write] passthrough
json.model  = gpt-5-mini    ← output_format JSON via shared/structured-output
json.ok     = true
──────────────
PASS: all four capabilities exercised end-to-end

Per-node timings (Pino logs from the run):

• hello: 10.4s
• reasoning (effort: high): 10.9s
• tool_restricted (denied_tools): 9.8s
• structured (output_format): 14.6s — slowest, expected for prompt-augmented JSON
• assert (bash): 12ms

Workflow run id: e08af89220eb0b96e85c0240473478bc (locally — not persisted in any shared artifact yet).

Capability matrix recap (from PR body, validated by where):

Capability	Live	Unit-tested	Notes
sessionResume	indirect	✅	provider.test.ts covers resume path
envInjection	indirect	✅	provider.test.ts; live use of useLoggedInUser
effortControl	✅	✅	e2e-copilot-all-features (391 = 17×23)
thinkingControl	—	✅	string→reasoningEffort tested; object→warn
toolRestrictions	✅	✅	e2e-copilot-all-features (DENIED_OK)
mcp	—	✅	mcp-translation.test.ts
skills	—	✅	skills-translation.test.ts
structuredOutput	✅	✅	e2e-copilot-all-features ({model, ok})
agents	—	✅	agents-translation.test.ts

4. Smoke workflow with run output captured

Two files now under .archon/workflows/test-workflows/ (the upstream-renamed location from coleam00#1431, which was part of the conflict):

• e2e-copilot-smoke.yaml — single-prompt connectivity check. Mirrors e2e-claude-smoke / e2e-pi-smoke.
• e2e-copilot-all-features.yaml (NEW — added in 06c3f9e) — four-capability smoke. Mirrors e2e-minimax-smoke. Each scenario is documented inline so the file doubles as adoption docs.

To reproduce:

copilot login                                    # one-time
bun run cli workflow run e2e-copilot-smoke
bun run cli workflow run e2e-copilot-all-features

Conflict status — resolved

Upstream dev advanced past the previous PR head with two relevant landings:

• feat(pi): Use ModelRegistry & allow empty auth to support custom models/providers. coleam00/Archon#1284 feat(pi): use ModelRegistry to support custom models — touched the same region in pi/provider.ts where we'd extracted augmentPromptForJsonSchema to shared/.
• chore(workflows): group smoke-test workflows under test-workflows/ + add e2e-minimax-smoke coleam00/Archon#1431 chore(workflows): group smoke-test workflows under test-workflows/ — moved every e2e-*.yaml smoke into test-workflows/.

Resolution in commit 06c3f9e:

• pi/provider.ts — kept the shared/ extraction (single source of truth for both Pi and Copilot), re-export the function from pi/provider.ts under the original name so existing Pi callers and tests stay byte-for-byte.
• Dropped a stale lookupPiModel/GetModelFn helper that had zero callers (leftover from an earlier merge attempt; superseded by upstream's ModelRegistry).
• Moved e2e-copilot-smoke.yaml to test-workflows/ to match the new convention.
• Added e2e-copilot-all-features.yaml alongside.

Upstream PR mergeability after push: MERGEABLE / CLEAN (was DIRTY / CONFLICTING).

What I haven't done yet — your call

1. Reply to Wirasm. Have NOT posted on feat(providers): add GitHub Copilot community provider (builtIn: false) coleam00/Archon#1351 per your instruction. The reply for him is essentially this comment with the items 1-4 + a pointer to commit 06c3f9e and an offer to add live MCP/skills smokes. Want me to draft + post when you say go?
2. Video / screenshot evidence. Wirasm called video "if possible". The two smoke files give you a clean reproducible setup to record:
   • CLI capture (simplest): bun run cli workflow run e2e-copilot-all-features in a terminal — the Pino log lines + final ── results ── block fit on one screen.
   • Web UI capture: bun run dev, browse to http://localhost:5173, pick e2e-copilot-all-features from the workflow list, click run, screenshot the streaming DAG.
     I can prep a cleaner terminal recording shell-script if you want a single command that auto-clears + runs + tails the log to a file you can attach.
3. Live MCP / skills smokes. Both are unit-tested but not yet exercised against a live Copilot session. If Wirasm pushes back, I can stage a minimal MCP server config + a .agents/skills/foo/SKILL.md and add e2e-copilot-mcp-smoke.yaml / e2e-copilot-skills-smoke.yaml.
4. Wider model coverage. gpt-5 and a Claude-via-Copilot model — quick to add (one extra workflow file per model), only matters if Wirasm asks.

Recommendation: post the reply + offer (2)(3)(4) on demand. The current evidence already exceeds what the Pi PRs landed with at the same review stage.

[coderabbitai]

Actionable comments posted: 9

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

🟡 Minor comments (12)

.claude/skills/archon/references/authoring-commands.md-9-21 (1)

9-21: ⚠️ Potential issue | 🟡 Minor

Add a language to the fenced block to satisfy markdownlint.

Line 9 opens a fenced code block without a language (MD040). Please mark it as text since this is a directory tree.

Suggested fix

-```
+```text
 <repoRoot>/.archon/commands/     # 1. Repo-scoped (wins)
 ├── my-command.md                #    Custom command for this repo
 ├── archon-assist.md             #    Overrides the bundled archon-assist
 └── triage/                      #    Subfolders allowed, 1 level deep
     └── review.md                #    Resolves as 'review', not 'triage/review'
 
 ~/.archon/commands/              # 2. Home-scoped (user-level, shared across all repos)
 ├── review-checklist.md          #    Personal helper available in every repo
 └── pr-style-guide.md
 
 <bundled defaults>                # 3. Shipped with Archon (archon-assist, etc.)

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @.claude/skills/archon/references/authoring-commands.md around lines 9 - 21,
The fenced code block that begins with "/.archon/commands/ # 1.
Repo-scoped (wins)" is missing a language tag (MD040); update the opening fence
from totext so the directory-tree block is marked as text (i.e., replace
the triple backticks that start the block with ```text).

</details>

</blockquote></details>
<details>
<summary>packages/docs-web/src/content/docs/getting-started/ai-assistants.md-240-257 (1)</summary><blockquote>

`240-257`: _⚠️ Potential issue_ | _🟡 Minor_

**Document the compiled-mode vendor fallback for Copilot CLI resolution.**

The Copilot section currently documents env/config override paths but not the compiled-binary vendor fallback path/behavior described in this PR. That can mislead users into unnecessary global installs.



<details>
<summary>📝 Proposed docs patch</summary>

```diff
 For compiled Archon binaries, install the Copilot CLI yourself and point Archon at it if needed:
@@
 Optional override paths:
@@
 assistants:
   copilot:
     copilotCliPath: /absolute/path/to/copilot
```

```diff
+In compiled binary mode, Archon also checks its vendor Copilot binary location before falling back to PATH. If you ship Archon with a vendored Copilot CLI, no global install is required.
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@packages/docs-web/src/content/docs/getting-started/ai-assistants.md` around
lines 240 - 257, The docs omit describing the compiled-mode vendor fallback used
to resolve the Copilot CLI, which can cause users to unnecessarily install the
global `@github/copilot` package; update the Getting Started / Copilot section to
document the compiled-binary vendor fallback behavior and the exact resolution
order (env var COPILOT_CLI_PATH, YAML config assistants.copilot.copilotCliPath,
then the compiled-mode vendor fallback path), and indicate the typical vendor
path used by compiled Archon binaries and when a global install is actually
required; reference the Copilot CLI override examples already present so readers
understand precedence.
```

</details>

</blockquote></details>
<details>
<summary>.archon/commands/maintainer-review-code-review.md-115-117 (1)</summary><blockquote>

`115-117`: _⚠️ Potential issue_ | _🟡 Minor_

**Add language specifier to fenced code block.**

The fenced code block is missing a language specifier, which triggers a markdownlint warning. Since this is an example output format (not executable code), specify `text` as the language.



<details>
<summary>📝 Suggested fix</summary>

```diff
-```
+```text
 Code review complete. <N> CRITICAL, <N> HIGH, <N> MEDIUM, <N> LOW findings. Verdict: <ready|fixes-needed|blocking>.
 ```
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @.archon/commands/maintainer-review-code-review.md around lines 115 - 117,
The fenced code block in the example output (the triple-backtick block
containing "Code review complete. CRITICAL...") lacks a language specifier;
update that fenced block to include the text language (i.e., change ``` to

non-code text.

packages/providers/src/codex/binary-resolver.test.ts-90-104 (1)

90-104: ⚠️ Potential issue | 🟡 Minor

Use os.homedir() to stay aligned with production.

The probe path is built from process.env.HOME, but the resolver builds it from homedir() (node:os). On POSIX they usually agree, but if HOME is unset/overridden in CI, homedir() falls back to getpwuid_r, the mock won't match the real probe path, and the test will fail with a misleading "not found" error rather than a clear assertion failure. Source the expected path the same way the production code does.

♻️ Suggested change

+import { homedir } from 'node:os';
@@
   test('autodetects npm global install at ~/.npm-global/bin/codex (POSIX)', async () => {
     if (process.platform === 'win32') return; // POSIX-only probe
-    const home = process.env.HOME ?? '/Users/test';
-    const expected = `${home}/.npm-global/bin/codex`;
+    const expected = `${homedir()}/.npm-global/bin/codex`;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/providers/src/codex/binary-resolver.test.ts` around lines 90 - 104,
The test "autodetects npm global install at ~/.npm-global/bin/codex (POSIX)"
builds the expected path from process.env.HOME which can diverge from
production's os.homedir(); update the test to derive the expected path using
homedir() (imported from node:os or referenced the same way production does) so
the mocked resolver.fileExists check (fileExistsSpy on resolver.fileExists)
matches the path used by resolveCodexBinaryPath, ensuring the assertion and mock
hit consistently.

.archon/workflows/defaults/archon-piv-loop.yaml-398-403 (1)

398-403: ⚠️ Potential issue | 🟡 Minor

TASK_COUNT fallback can swallow the friendly “no tasks” error.

grep -c writes its count to stdout before exiting 1 when there are no matches, so on a malformed plan the substitution captures "0" from grep and then "0" from the || echo "0" fallback. With set -e (Line 368) and the resulting multi-line TASK_COUNT="0\n0", the next [ "$TASK_COUNT" -eq 0 ] errors with integer expression required and the script aborts before the ERROR: No '### Task N:' sections found … message is printed, defeating the whole point of this guard.

🛠️ Suggested fix

-      TASK_COUNT=$(grep -c "^### Task [0-9]" "$PLAN_FILE" 2>/dev/null || echo "0")
-      if [ "$TASK_COUNT" -eq 0 ]; then
+      TASK_COUNT=$(grep -c "^### Task [0-9]" "$PLAN_FILE" 2>/dev/null) || TASK_COUNT=0
+      if [ "$TASK_COUNT" -eq 0 ]; then
         echo "ERROR: No '### Task N:' sections found in $PLAN_FILE. Plan may be malformed."
         exit 1
       fi

This keeps the original intent (default to 0 when grep fails or finds nothing) while ensuring TASK_COUNT is a single integer so the friendly error path actually runs.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.archon/workflows/defaults/archon-piv-loop.yaml around lines 398 - 403, The
TASK_COUNT assignment can produce a multi-line value and break the integer
check; change the capture to avoid the fallback echo adding a second "0" and
ensure TASK_COUNT is a single integer: replace the line that sets TASK_COUNT
(the grep -c ... || echo "0") with a safer sequence such as capturing grep's
output but allowing failure (TASK_COUNT=$(grep -c "^### Task [0-9]" "$PLAN_FILE"
2>/dev/null || true)), then normalize it to a single integer (e.g. trim any
newline and default to 0: TASK_COUNT=${TASK_COUNT%%$'\n'*};
TASK_COUNT=${TASK_COUNT:-0}) before the [ "$TASK_COUNT" -eq 0 ] check so the
friendly error for PLAN_FILE is reliably reached.

packages/workflows/src/schemas/workflow.ts-71-71 (1)

71-71: ⚠️ Potential issue | 🟡 Minor

Trim tag strings at schema validation to reject whitespace-only entries.

z.string().min(1) allows values like " ". That diverges from loader behavior (trim/drop), so the same workflow can validate in one path and normalize differently in another.

Proposed fix

-  tags: z.array(z.string().min(1)).optional(),
+  tags: z.array(z.string().trim().min(1)).optional(),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/schemas/workflow.ts` at line 71, The tags schema
currently uses z.array(z.string().min(1)).optional(), which accepts
whitespace-only strings; update the tags item schema so each tag is trimmed and
empty results are rejected (or filtered) at validation. Specifically, modify the
tags array element (the symbol "tags" in the workflow schema) to transform each
string by trimming (e.g., .transform(s => s.trim())) and then require non-empty
(e.g., .min(1) or a .refine check) so whitespace-only entries fail validation
(or are removed) and behavior matches the loader normalization.

packages/workflows/src/loader.ts-364-381 (1)

364-381: ⚠️ Potential issue | 🟡 Minor

tags dedupe is still case-sensitive, so semantic duplicates can leak through.

Set only dedupes exact strings. Inputs like ["GitLab", "gitlab"] remain two tags despite the block’s stated normalization intent.

Proposed fix

-    if (Array.isArray(raw.tags)) {
-      tags = [
-        ...new Set(
-          raw.tags
-            .filter((t): t is string => typeof t === 'string')
-            .map(t => t.trim())
-            .filter(t => t.length > 0)
-        ),
-      ];
+    if (Array.isArray(raw.tags)) {
+      const byCanonical = new Map<string, string>();
+      for (const rawTag of raw.tags) {
+        if (typeof rawTag !== 'string') continue;
+        const trimmed = rawTag.trim();
+        if (trimmed.length === 0) continue;
+        const canonical = trimmed.toLowerCase();
+        if (!byCanonical.has(canonical)) {
+          byCanonical.set(canonical, trimmed);
+        }
+      }
+      tags = [...byCanonical.values()];
     } else if (raw.tags !== undefined) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/loader.ts` around lines 364 - 381, The dedupe step for
tags is currently case-sensitive (Set on the trimmed strings), so semantic
duplicates like "GitLab" vs "gitlab" pass through; change the
normalization/dedupe pipeline used when computing tags from raw.tags to perform
case-insensitive deduplication: after filtering and trimming each string (the
existing raw.tags filter/map), compute a casefold key (e.g., t.toLowerCase() or
t.toLocaleLowerCase()) and use a Map keyed by that lowercased value to keep the
first-seen original-cased trimmed tag, then set tags to the Map.values() array;
update the code paths that currently use new Set(...) so they use this
case-insensitive dedupe while preserving first-seen casing and still warn via
getLog() on invalid raw.tags.

.archon/workflows/defaults/archon-adversarial-dev.yaml-120-120 (1)

120-120: ⚠️ Potential issue | 🟡 Minor

Model alias opus[1m] will work, but the YAML parsing has a subtle risk.

Good catch replacing claude-opus-4-6[1m (missing closing bracket — would not resolve) with the documented Claude Code alias opus[1m]. Per Anthropic's model-config docs, opus resolves to the latest Opus and the [1m] suffix opts into the 1M context variant on supported plans.

Two confirmed points and one risk to address:

1. Model resolver handles [1m] correctly — The Claude provider's isModelCompatible() function accepts any string (no bracket stripping). The model string passes through to the Claude SDK/CLI as-is, bracket suffix intact. ✅

2. Schema validation doesn't trim model field — Unlike the provider field (which has .trim() in the schema), the model field is defined as z.string().optional() with no trimming. This is safe for bracket suffixes; however, it represents an inconsistency vs. provider handling that may be intentional. ✅

3. Unquoted YAML scalar opus[1m] is valid but risky — Bun's native YAML parser (used in the loader) correctly parses model: opus[1m] as the string "opus[1m]" in block context, following YAML 1.2 plain-scalar rules. However, the eight workflows already using this syntax show it works in practice. If the codebase ever switches YAML loaders or moves to flow-style mappings, the unquoted brackets become flow indicators and parsing breaks. Quoting (model: "opus[1m]") would be safer for long-term resilience.

Recommendation: Keep the current fix (opus[1m] unquoted) since it matches eight existing workflows and is documented in Anthropic's model-config. For robustness across potential future loader changes, consider quoting it as a follow-up refactor, but this is not critical.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.archon/workflows/defaults/archon-adversarial-dev.yaml at line 120, The YAML
model scalar "opus[1m]" should be quoted to avoid future YAML parser/flow-style
ambiguity: update the workflow YAML so the model value is written as a quoted
string (e.g., "opus[1m]") instead of an unquoted plain scalar, and ensure any
loader/schema expectations still accept the quoted value (note the schema uses
z.string().optional() for model and provider trimming is done elsewhere with
provider.trim(), while isModelCompatible() passes the model through unchanged);
this change is purely formatting and should not require code changes other than
ensuring the YAML loader continues to produce the same string.

packages/docs-web/src/content/docs/book/quick-reference.md-145-145 (1)

145-145: ⚠️ Potential issue | 🟡 Minor

Update documentation: agents is supported by both Claude and Copilot, not Claude-only.

The codebase shows Copilot's capabilities include agents: true with a dedicated translation layer (applyAgents() in packages/providers/src/community/copilot/provider.ts). Archon's inline YAML agents: definitions map to Copilot's SessionConfig.customAgents, though with field constraints (Copilot cannot represent model, disallowedTools, skills, or maxTurns and surfaces these as warnings). Update the table to clarify support is provider-specific but present, or add a note explaining the data shape difference.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/docs-web/src/content/docs/book/quick-reference.md` at line 145,
Update the table entry so it no longer says "Claude only" — change the provider
support note for `agents` to indicate both Claude and Copilot support it (e.g.,
"Supported by Claude and Copilot (provider-specific)"). Also add a short
clarifying note that Copilot maps inline YAML `agents:` into
SessionConfig.customAgents via applyAgents() and that Copilot cannot represent
`model`, `disallowedTools`, `skills`, or `maxTurns` (these are surfaced as
warnings), so the data shape differs by provider.

packages/providers/src/community/pi/provider.ts-195-207 (1)

195-207: ⚠️ Potential issue | 🟡 Minor

Catch message conflates AuthStorage and ModelRegistry init failures.

The try covers both AuthStorage.create() (line 198) and ModelRegistry.create(authStorage) (line 199), but the thrown error hardcodes Pi auth storage init failed and points the operator at ~/.pi/agent/auth.json. Per the doc comment above (lines 192–194), ModelRegistry.create() is expected not to throw on models.json load errors, but if it ever does (e.g. unexpected I/O failure, future SDK change), the operator gets sent to the wrong file to debug. Either narrow each call to its own try/catch with a tailored message, or generalize the wording to "Pi SDK init failed" and mention both files.

🛡️ Proposed fix — split the two init calls

-    let authStorage: ReturnType<typeof piCodingAgent.AuthStorage.create>;
-    let modelRegistry: ReturnType<typeof piCodingAgent.ModelRegistry.create>;
-    try {
-      authStorage = piCodingAgent.AuthStorage.create();
-      modelRegistry = piCodingAgent.ModelRegistry.create(authStorage);
-    } catch (err) {
-      const e = err as Error;
-      getLog().error({ err: e, piProvider: parsed.provider }, 'pi.auth_storage_init_failed');
-      throw new Error(
-        `Pi auth storage init failed: ${e.message}. Check that ~/.pi/agent/auth.json ` +
-          '(or $PI_CODING_AGENT_DIR/auth.json) is valid JSON and readable.'
-      );
-    }
+    let authStorage: ReturnType<typeof piCodingAgent.AuthStorage.create>;
+    try {
+      authStorage = piCodingAgent.AuthStorage.create();
+    } catch (err) {
+      const e = err as Error;
+      getLog().error({ err: e, piProvider: parsed.provider }, 'pi.auth_storage_init_failed');
+      throw new Error(
+        `Pi auth storage init failed: ${e.message}. Check that ~/.pi/agent/auth.json ` +
+          '(or $PI_CODING_AGENT_DIR/auth.json) is valid JSON and readable.'
+      );
+    }
+    let modelRegistry: ReturnType<typeof piCodingAgent.ModelRegistry.create>;
+    try {
+      modelRegistry = piCodingAgent.ModelRegistry.create(authStorage);
+    } catch (err) {
+      const e = err as Error;
+      getLog().error({ err: e, piProvider: parsed.provider }, 'pi.model_registry_init_failed');
+      throw new Error(
+        `Pi model registry init failed: ${e.message}. Check that ~/.pi/agent/models.json ` +
+          '(or $PI_CODING_AGENT_DIR/models.json) is valid JSON.'
+      );
+    }

Note that the existing test at lines 285–308 only exercises the AuthStorage.create() failure path — if you adopt the split, the second branch should get its own test as well.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/providers/src/community/pi/provider.ts` around lines 195 - 207, The
catch currently covers both piCodingAgent.AuthStorage.create() and
piCodingAgent.ModelRegistry.create(authStorage) but logs a hardcoded "Pi auth
storage init failed" pointing to auth.json; split the calls into two separate
try/catch blocks (one around AuthStorage.create() and one around
ModelRegistry.create(authStorage)) so each catch logs a specific error and
guidance (AuthStorage errors mention ~/.pi/agent/auth.json or
$PI_CODING_AGENT_DIR/auth.json and ModelRegistry errors mention models.json /
SDK init issues) and use getLog().error with the caught Error and
parsed.provider; alternatively, if you prefer a single catch, change the message
to a generalized "Pi SDK init failed" that references both auth.json and
models.json and includes the actual error details.

.archon/scripts/maintainer-standup-gh-data.ts-38-44 (1)

38-44: ⚠️ Potential issue | 🟡 Minor

Frontmatter regex captures surrounding quotes verbatim.

/^gh_handle:\s*(\S+)\s*$/m will capture "popemkt" (including the quotes) for the common YAML form gh_handle: "popemkt", after which every gh invocation that interpolates ghHandle (lines 104, 112, 121, 174) will pass a literal quoted string and return zero results. Strip optional surrounding quotes:

🛡️ Proposed fix

-  const match = profile.match(/^gh_handle:\s*(\S+)\s*$/m);
-  if (match) ghHandle = match[1];
+  const match = profile.match(/^gh_handle:\s*["']?([^"'\s]+)["']?\s*$/m);
+  if (match) ghHandle = match[1];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.archon/scripts/maintainer-standup-gh-data.ts around lines 38 - 44, The
frontmatter regex currently captures surrounding quotes into ghHandle causing
quoted values like "\"popemkt\"" to be used; update the match and assignment in
the block that sets ghHandle (the const profile / match logic) so it either uses
a regex that allows optional surrounding single or double quotes and captures
the inner handle (e.g. change the capture group to exclude surrounding quotes)
or strip surrounding quotes from match[1] before assigning to ghHandle; ensure
the variable ghHandle used later in gh invocations (references around lines
where ghHandle is interpolated) contains the raw handle without quotes.

.archon/commands/maintainer-review-report.md-60-60 (1)

60-60: ⚠️ Potential issue | 🟡 Minor

Edge case: .label-applied may be absent if post-decline aborted before the label step.

The instruction reads .label-applied and falls back to .label-error, but if gh pr comment itself failed on line 287 of the workflow, the script exits before either file is written. In that case neither file exists, and the LLM has no instruction for what to surface. Consider adding a "if neither file exists, the post-decline node failed before reaching the label step — say so" clause.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.archon/commands/maintainer-review-report.md at line 60, The current logic
that reads `$ARTIFACTS_DIR/.label-applied` and falls back to
`$ARTIFACTS_DIR/.label-error` must also handle the case where neither file
exists; update the label-reporting code (the block that reads `.label-applied` /
`.label-error`) to detect missing files and output a clear one-line message like
“no, post-decline node failed before reaching the label step — no
`.label-applied` or `.label-error` present” so maintainers know the labeling
step was never attempted; keep the existing behavior that if `.label-applied`
exists and equals `skipped` you produce “no, label add failed: <reason>” using
the contents of `.label-error`, and do not report “yes” in skipped/error cases.

🧹 Nitpick comments (23)

packages/workflows/src/dag-executor.test.ts (2)

6086-6087: Avoid hardcoding source line numbers in comments.

The reference to dag-executor.ts ~line 2956 will silently drift on every refactor. A symbol-based hint (e.g. the variable name anyFailed or function name) is enough and won't go stale.

♻️ Proposed tweak

-  // succeeded. This covers the anyFailed branch in executeDagWorkflow
-  // (dag-executor.ts ~line 2956), which had no direct test coverage.
+  // succeeded. This covers the `anyFailed` branch in executeDagWorkflow,
+  // which had no direct test coverage.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/dag-executor.test.ts` around lines 6086 - 6087, The
comment hardcodes a source line number ("dag-executor.ts ~line 2956") which will
become stale; update the comment to remove the numeric line reference and
instead point to the relevant symbol(s) — e.g., mention the function
executeDagWorkflow and the anyFailed variable/branch — so the intent stays
accurate after refactors.

---

5832-5930: Hoist these dynamic imports to the top of the file.

parseMcpFailureServerNames and loadConfiguredMcpServerNames are imported via await import('./dag-executor') inside every individual it block (≈11 call sites across these two describes, plus 7 more in the shouldContinueStreamingForStatus describe at lines 6043–6077). Since all other helpers from ./dag-executor are imported statically at the top (lines 35–40), aligning these for consistency reduces noise and avoids repeated module-cache lookups in tests.

♻️ Proposed refactor

 import {
   buildTopologicalLayers,
   checkTriggerRule,
   substituteNodeOutputRefs,
   executeDagWorkflow,
+  parseMcpFailureServerNames,
+  loadConfiguredMcpServerNames,
+  shouldContinueStreamingForStatus,
 } from './dag-executor';

Then drop the in-test await import('./dag-executor') calls and reference the symbols directly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/dag-executor.test.ts` around lines 5832 - 5930, The
tests repeatedly call await import('./dag-executor') inside each it block for
parseMcpFailureServerNames and loadConfiguredMcpServerNames; hoist those dynamic
imports to the top of the test file and import the two symbols statically (e.g.,
add import { parseMcpFailureServerNames, loadConfiguredMcpServerNames } from
'./dag-executor' alongside the existing static imports), then remove the in-test
await import calls and reference the functions directly in all it blocks
(including the other describe for shouldContinueStreamingForStatus).

packages/providers/src/claude/binary-resolver.ts (1)

102-112: Optional: align autodetect probe with Copilot resolver's isExecutableFile check.

The Copilot binary resolver uses a stricter isExecutableFile check (validates both file type and executable permissions) across all resolution paths. In contrast, Claude's autodetect at ~/.local/bin/claude relies on plain existsSync via fileExists(), which returns true for directories or non-executable files. Because autodetect runs without explicit user opt-in (unlike env/config sources where users get immediate errors), a stale or wrong-typed entry at that path would be silently returned and only fail later at spawn time.

Not blocking — the env and config paths already throw on missing files, providing user feedback; this is an optional improvement to fail-fast on autodetect as well.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/providers/src/claude/binary-resolver.ts` around lines 102 - 112,
Autodetect currently returns nativeInstallerPath when
fileExists(nativeInstallerPath) is true, which can be a directory or
non-executable file; change this to use the stricter executable check (the same
isExecutableFile used in the Copilot resolver) when probing ~/.local/bin/claude
so the resolver only returns a path that is a regular file with execute
permission; update the check in binary-resolver.ts to call
isExecutableFile(nativeInstallerPath) (or perform equivalent stat+mode checks)
and only log/return nativeInstallerPath when that check passes, otherwise
continue resolution.

.archon/commands/maintainer-review-code-review.md (2)

113-113: Consider hyphenating compound adjective for clarity.

The phrase "single line summary" would be more grammatically correct as "single-line summary" when used as a compound adjective modifying "summary."

📝 Suggested fix

-Return a single line summary as your response:
+Return a single-line summary as your response:

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.archon/commands/maintainer-review-code-review.md at line 113, Replace the
unhyphenated phrase "single line summary" with the hyphenated compound adjective
"single-line summary" wherever it appears (e.g., the line containing "Return a
single line summary as your response:") to improve grammatical clarity; update
the text to read "Return a single-line summary as your response:".

---

22-24: Clarify fallback behavior if CLAUDE.md is missing.

The instruction assumes CLAUDE.md exists in the repo. Consider adding guidance on what to do if it's missing (e.g., skip compliance checks, use defaults, or fail with a clear message).

📝 Suggested clarification

 ### Read the project's rules
 
-Read the repo's `CLAUDE.md` (project-level). It's the source of truth for engineering principles, type-safety rules, eslint policy, error-handling conventions, and forbidden patterns.
+Read the repo's `CLAUDE.md` (project-level) if it exists. It's the source of truth for engineering principles, type-safety rules, eslint policy, error-handling conventions, and forbidden patterns.
+
+```bash
+if [[ -f "CLAUDE.md" ]]; then
+  cat CLAUDE.md
+else
+  echo "Note: CLAUDE.md not found. Skipping project-specific compliance checks."
+fi
+```

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.archon/commands/maintainer-review-code-review.md around lines 22 - 24, The
"Read the project's rules" step assumes CLAUDE.md exists; update this section to
specify fallback behavior when CLAUDE.md is missing by adding a clear
instruction (e.g., skip project-specific checks, apply default rules, or abort
with a descriptive error) and show the expected handling pattern referencing
CLAUDE.md and the "Read the project's rules" step so reviewers know to check for
the file and act accordingly; make the preferred default behavior explicit (for
example: if CLAUDE.md is absent, echo a note and proceed with default compliance
rules or fail with a clear message).

packages/providers/src/codex/binary-resolver.test.ts (1)

124-135: Nit: assert the resolution source for parity with sibling tests.

The other two autodetect tests (lines 100-103, 118-121) verify that mockLogger.info is called with source: 'autodetect'. This one only checks the returned path, so a regression that resolves /usr/local/bin/codex but mislabels its source would slip through. Mirror the same logger assertion here.

♻️ Suggested addition

     const result = await resolver.resolveCodexBinaryPath();
     expect(result).toBe('/usr/local/bin/codex');
+    expect(mockLogger.info).toHaveBeenCalledWith(
+      { binaryPath: '/usr/local/bin/codex', source: 'autodetect' },
+      'codex.binary_resolved'
+    );
   });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/providers/src/codex/binary-resolver.test.ts` around lines 124 - 135,
Add the same logger assertion as the sibling autodetect tests: after calling
resolver.resolveCodexBinaryPath() in the test that checks
'/usr/local/bin/codex', assert that mockLogger.info was called with an object
indicating the resolved path and source: 'autodetect' (matching the pattern used
in the other tests). Locate the test using resolver.resolveCodexBinaryPath,
fileExistsSpy and mockLogger and add an expectation that mockLogger.info
received an argument with the path '/usr/local/bin/codex' and source:
'autodetect'.

packages/providers/src/codex/binary-resolver.ts (1)

145-165: Optional: probe linuxbrew / be defensive on unsupported platforms.

Two small follow-ups you may want to consider; neither is blocking:

• On Linux, /home/linuxbrew/.linuxbrew/bin/codex is a fairly common npm prefix when Node is installed via Homebrew on Linux. The deferred-cases doc-comment doesn't mention it; worth either adding a probe or calling it out alongside the other deferred items.
• getAutodetectPaths() falls into the POSIX branch for any non-win32 platform (e.g., FreeBSD, AIX), even though getVendorBinaryName() rejects them via SUPPORTED_PLATFORMS. Probing a couple of POSIX paths is harmless, but you could short-circuit on unsupported platforms for symmetry with tier 3 — your call.

No action required if you'd rather keep tier-4 minimal.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/providers/src/codex/binary-resolver.ts` around lines 145 - 165,
getAutodetectPaths() currently treats any non-win32 as POSIX and misses the
common Linuxbrew prefix; update getAutodetectPaths() to (1) add
'/home/linuxbrew/.linuxbrew/bin/codex' to the Linux-specific probe (i.e., when
process.platform === 'linux') so the resolver will find Node/Homebrew installs,
and (2) optionally short-circuit and return an empty array if process.platform
is not listed in SUPPORTED_PLATFORMS (used by getVendorBinaryName()) to keep
behavior symmetric with platform support checks; reference getAutodetectPaths(),
SUPPORTED_PLATFORMS, and getVendorBinaryName() when making the change.

packages/server/src/index.ts (1)

403-403: Normalize Discord failure log event name to the standard convention.

discord.start_failed_continuing_without_adapter does not follow the {domain}.{action}_{state} shape used for consistent log filtering and alerting.

Proposed fix

-        getLog().error({ err, hint }, 'discord.start_failed_continuing_without_adapter');
+        getLog().error({ err, hint }, 'discord.start_failed');

As per coding guidelines: **/*.ts: Use structured logging with Pino; event naming format: {domain}.{action}_{state}.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/server/src/index.ts` at line 403, Replace the nonconforming event
string in the logging call so it follows the {domain}.{action}_{state}
convention: update the getLog().error invocation that currently uses
'discord.start_failed_continuing_without_adapter' to a normalized name such as
'discord.start_failed_without_adapter' (keep existing structured payload { err,
hint } intact); change only the event-name string in the getLog().error call so
log filtering/alerting uses the normalized symbol.

packages/docs-web/src/content/docs/reference/variables.md (1)

67-69: Strengthen the safety guidance — embedding raw values in a template literal is also unsafe.

The current text correctly tells authors to use JSON.parse rather than shell interpolation, but the example workflows in this PR (dag-workflow.yaml, archon-workflow-builder.yaml) recommend wrapping the substitution in a String.raw`...` template. That's still a JS interpolation surface — ${...} is evaluated and backticks terminate the string regardless of String.raw. Since this is the canonical reference, it's worth calling out the pitfall here so the example workflows follow.

♻️ Suggested addition

-`$nodeId.output` values are **auto shell-quoted** (single-quoted, with embedded `'` escaped) when substituted into `bash:` scripts, so the value is always safe to embed in a shell command. They are **not** shell-quoted when substituted into `script:` bodies — the raw value is embedded as-is. For script nodes, treat substituted values as untrusted input and parse them with language features (e.g. `JSON.parse`), not by interpolating into shell syntax.
+`$nodeId.output` values are **auto shell-quoted** (single-quoted, with embedded `'` escaped) when substituted into `bash:` scripts, so the value is always safe to embed in a shell command. They are **not** quoted or escaped in any way when substituted into `script:` bodies — the raw bytes are pasted directly into the source.
+
+For script nodes, treat substituted values as untrusted input. **Do not** embed them inside string literals or template literals (including `` String.raw`...` ``): a backtick or `${...}` in upstream output will terminate the string or evaluate as a JS/Python expression. Instead, read the upstream value from `stdin` (recommended) or pass it through a JSON-encoded substitution form, then `JSON.parse` / `json.loads` it.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/docs-web/src/content/docs/reference/variables.md` around lines 67 -
69, Update the safety guidance to explicitly warn that embedding substituted
values into JavaScript template literals (e.g. using String.raw`...` or backtick
interpolation like `${...}`) is unsafe for script: nodes because `${...}` is
evaluated and backticks can be terminated by the value; state that
$nodeId.output is only auto-quoted for bash: and is raw for script:, and
instruct authors to treat substituted values as untrusted and parse them with
language parsers (e.g. JSON.parse) or pass them through a safe
serialization/deserialization API instead of interpolating into template
literals (mention String.raw, template literal/backtick interpolation, and
JSON.parse by name so readers can find the relevant code/examples to change).

.claude/skills/archon/references/troubleshooting.md (1)

9-11: Add a language tag to the plain-text fenced blocks (markdownlint MD040).

Static analysis flags lines 9 and 42 as missing a code-block language. Use text to keep the lint clean:

♻️ Diff

-```
+```text
 ~/.archon/workspaces/<owner>/<repo>/logs/<run-id>.jsonl

```diff
-```
+```text
 ~/.archon/workspaces/<owner>/<repo>/artifacts/runs/<run-id>/

</details>




Also applies to: 42-44

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @.claude/skills/archon/references/troubleshooting.md around lines 9 - 11, The
two plain-text fenced code blocks in troubleshooting.md are missing a language
tag (markdownlint MD040); update both fenced blocks that show paths like
"/.archon/workspaces///logs/.jsonl" and
"/.archon/workspaces///artifacts/runs//" to use the "text"
language tag (i.e., change totext for those blocks) so the markdown
linter no longer flags them.

</details>

</blockquote></details>
<details>
<summary>.claude/skills/archon/references/repo-init.md (1)</summary><blockquote>

`90-94`: **Add a language tag to the log-output fenced block (markdownlint MD040).**

<details>
<summary>♻️ Diff</summary>

```diff
-```
+```text
 [archon] loaded N keys from ~/.archon/.env
 [archon] loaded M keys from /path/to/repo/.archon/.env
 [archon] stripped K keys from /path/to/repo (ANTHROPIC_API_KEY, OPENAI_API_KEY, ...)
 ```
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @.claude/skills/archon/references/repo-init.md around lines 90 - 94, The
fenced code block containing the sample log output (the triple-backtick block
that starts with "[archon] loaded N keys...") is missing a language tag and
triggers markdownlint MD040; update that fence to include a language tag (e.g.,
change totext) for the block showing the log lines and search for any
other unlabeled fenced blocks in the same document to add appropriate language
tags as well.

</details>

</blockquote></details>
<details>
<summary>.claude/skills/archon/references/cli-commands.md (1)</summary><blockquote>

`35-46`: **Slight inconsistency between the `--resume` flag description and the auto-resume note.**

Line 35 describes `--resume` as "Resume the last failed run of this workflow at this cwd (skips completed nodes)", but line 45 then explains that *auto-resume already does that* and `--resume` is only needed to "force resume a specific failed run or to reuse the worktree from that run." Readers will be unclear on what `--resume` actually adds over the default behavior.

<details>
<summary>♻️ Suggested phrasing</summary>

```diff
-| `--resume` | Resume the last failed run of this workflow at this cwd (skips completed nodes) |
+| `--resume` | Force-resume a prior failed run (and reuse its worktree). Most workflows auto-resume without this flag — see note below |
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In @.claude/skills/archon/references/cli-commands.md around lines 35 - 46, The
`--resume` flag description conflicts with the "Auto-resume without `--resume`"
note; update the `--resume` line to clearly state that it forces resuming a
specific prior failed run or reuses that run's worktree (e.g., "Force resume a
specific failed run at this cwd or reuse its worktree; skips completed nodes"),
and leave the "Auto-resume without `--resume`" paragraph to explain the default
automatic skipping of completed nodes for the most recent failed invocation;
change either occurrence (the `--resume` bullet and the "Auto-resume without
`--resume`" heading) so both consistently convey that normal invocations
auto-resume the last failed run but `--resume` targets a specific run/worktree.
```

</details>

</blockquote></details>
<details>
<summary>.claude/skills/archon/references/good-practices.md (1)</summary><blockquote>

`224-239`: **Consider clarifying the MCP wildcard anti-pattern.**

This example is labeled "BAD" but then explains that `allowed_tools: []` actually works because "Archon auto-adds mcp__<server>__* wildcards." The anti-pattern is "forgetting and manually adding Read/Write/Bash" when you only want MCP.

This might confuse readers who see "BAD" but then read that it works. Consider restructuring to make the actual anti-pattern clearer.



<details>
<summary>Clarification suggestion</summary>

Consider rewording the comment to lead with what works, then explain what the anti-pattern is:

```diff
 # BAD — no tools available, including MCP
 - id: analyze
   prompt: "Use the Postgres MCP to query users"
   mcp: .archon/mcp/postgres.json
-  allowed_tools: []          # OOPS — disables EVERYTHING, including MCP tools
+  allowed_tools: [Read, Write, Bash]  # OOPS — forgot that mcp: auto-adds MCP tools
+                                       # Now you've allowed file ops when you only wanted MCP

-# FIXED — Archon auto-adds mcp__<server>__* wildcards when mcp: is set,
-# so this actually works out of the box. The anti-pattern is forgetting
-# and manually adding Read/Write/Bash/etc. when you only want MCP.
+# CORRECT — Archon auto-adds mcp__<server>__* wildcards when mcp: is set
 - id: analyze
   prompt: "Use Postgres MCP to query users"
   mcp: .archon/mcp/postgres.json
-  allowed_tools: []          # correct — MCP tools auto-attached
+  allowed_tools: []          # MCP tools auto-attached, everything else denied
```

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In @.claude/skills/archon/references/good-practices.md around lines 224 - 239,
The example labeled "BAD" is confusing because it shows allowed_tools: [] with
mcp: set but then states Archon auto-adds mcp__<server>__* wildcards so the
config actually works; update the text and examples around the analyze entry
(id: analyze, mcp: .archon/mcp/postgres.json, allowed_tools) to clearly show the
real anti-pattern: demonstrate the incorrect manual addition of Read/Write/Bash
tools when only MCP is intended and mark that as BAD, and mark the version that
relies on Archon's auto-added mcp__<server>__* wildcards (i.e., allowed_tools:
[] with only mcp set) as the correct/fixed example, adjusting comments to
explicitly state that Archon auto-attaches MCP wildcards.
```

</details>

</blockquote></details>
<details>
<summary>.claude/skills/archon/references/parameter-matrix.md (1)</summary><blockquote>

`155-155`: **Minor style: Add period after "etc."**

In American English, abbreviations like "etc." require a period.



<details>
<summary>Style fix</summary>

```diff
-| Variable reference (`$ARGUMENTS`, `$ARTIFACTS_DIR`, etc) | `variables.md`                                                   |
+| Variable reference (`$ARGUMENTS`, `$ARTIFACTS_DIR`, etc.) | `variables.md`                                                   |
```

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In @.claude/skills/archon/references/parameter-matrix.md at line 155, Update the
markdown table cell that currently reads "| Variable reference (`$ARGUMENTS`,
`$ARTIFACTS_DIR`, etc) | `variables.md` |" to add the missing period after "etc"
so it reads "etc."; edit the line containing the table entry (the string
"Variable reference (`$ARGUMENTS`, `$ARTIFACTS_DIR`, etc)") and replace "etc"
with "etc." to conform to American English abbreviation style.
```

</details>

</blockquote></details>
<details>
<summary>packages/docs-web/src/content/docs/book/dag-workflows.md (1)</summary><blockquote>

`240-240`: **Minor formatting: Remove trailing space in code example.**

The script node syntax has a trailing space before the closing backtick: `script: "..." ` — should be `script: "..."`.



<details>
<summary>Minor fix</summary>

```diff
-| **Script** | `script: "..." ` + `runtime: bun \| uv` | Run TypeScript/JavaScript (bun) or Python (uv) without AI. Inline code or named reference to `.archon/scripts/`. Stdout captured as `$nodeId.output`. See [Script Nodes](/guides/script-nodes/). |
+| **Script** | `script: "..."` + `runtime: bun \| uv` | Run TypeScript/JavaScript (bun) or Python (uv) without AI. Inline code or named reference to `.archon/scripts/`. Stdout captured as `$nodeId.output`. See [Script Nodes](/guides/script-nodes/). |
```

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@packages/docs-web/src/content/docs/book/dag-workflows.md` at line 240, In the
markdown table row containing the script node example, remove the stray space at
the end of the inline code token `script: "..." ` so it reads `script: "..."`
(i.e., delete the trailing space before the closing backtick) to fix the
formatting in the docs; update the table cell where the snippet `script: "..." `
+ `runtime: bun | uv` appears.
```

</details>

</blockquote></details>
<details>
<summary>packages/providers/src/community/pi/provider.test.ts (2)</summary><blockquote>

`216-229`: **Shim regression test — well-targeted.**

Driving the assertion through the fast-fail "no model" path is a nice trick: it sidesteps any need to script a successful Pi run while still proving that `ensurePiPackageDirShim()` runs before the no-model error. One small thing — `process.env.PI_PACKAGE_DIR` is left set after this test completes; later tests don't read it so there's no functional consequence today, but consider adding a `delete process.env.PI_PACKAGE_DIR` to the `beforeEach` block (alongside the existing `GEMINI_API_KEY` / `ANTHROPIC_API_KEY` deletes) to keep the test environment hermetic against future tests that might assert on its absence.

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@packages/providers/src/community/pi/provider.test.ts` around lines 216 - 229,
Add cleanup for PI_PACKAGE_DIR in the test suite setup: in the existing
beforeEach block that clears GEMINI_API_KEY / ANTHROPIC_API_KEY, also delete
process.env.PI_PACKAGE_DIR so the test "sendQuery installs PI_PACKAGE_DIR shim
before Pi SDK loads" does not leave PI_PACKAGE_DIR set for subsequent tests;
reference the beforeEach setup and the test name/sendQuery and the environment
variable process.env.PI_PACKAGE_DIR when locating where to add the deletion.
```

</details>

---

`389-402`: **Test header comment is slightly inaccurate.**

The comment "the adapter rejects unknown providers" overstates what the adapter does — there's no provider allowlist; an unknown `provider` segment just causes `modelRegistry.find()` to return `undefined`, which is the same path being exercised here. The reason to use `mockImplementationOnce(() => undefined)` against a *mapped* provider like `google` is so that the credential-resolution path (which depends on `PI_PROVIDER_ENV_VARS[parsed.provider]`) gets exercised in earlier tests with real fixtures, not because the adapter rejects unknown providers. Consider tightening the comment to reflect this.

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@packages/providers/src/community/pi/provider.test.ts` around lines 389 - 402,
The test header comment is inaccurate: update it to say that the adapter does
not reject unknown providers but that an unknown provider segment causes
ModelRegistry.find to return undefined, so we use
mockModelRegistryFind.mockImplementationOnce(() => undefined) with a mapped
provider like 'google' to exercise the credential-resolution path (which depends
on PI_PROVIDER_ENV_VARS[parsed.provider]) rather than to simulate an
adapter-level provider rejection; mention PiProvider.sendQuery,
mockModelRegistryFind, and the 'google/unknown-model-id' modelId to make the
intent clear.
```

</details>

</blockquote></details>
<details>
<summary>packages/providers/src/community/pi/provider.ts (1)</summary><blockquote>

`97-101`: **Move the value import to the top of the file.**

While the import placement itself is not idiomatic (imports should appear at the top of the file), the code organization concern is valid. However, the `import/first` ESLint rule referenced in the original suggestion is not enabled in this repository—`eslint-plugin-import` is not installed or configured. Keep the `export { augmentPromptForJsonSchema };` re-export at its current location with the documentation comment for clarity, but move the binding import up with the other imports for consistency with standard module organization.

<details>
<summary>♻️ Proposed refactor</summary>

```diff
 import { existsSync, mkdirSync, writeFileSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';

 import { createLogger } from '@archon/paths';

 import type {
   IAgentProvider,
   MessageChunk,
   ProviderCapabilities,
   SendQueryOptions,
 } from '../../types';

 import { PI_CAPABILITIES } from './capabilities';
 import { parsePiConfig } from './config';
 import { parsePiModelRef } from './model-ref';
+import { augmentPromptForJsonSchema } from '../../shared/structured-output';
@@
-// Structured-output prompt augmentation is shared across providers. Import
-// once for local use and re-export so existing callers and tests keep their
-// import path stable; new providers should import from `../../shared/structured-output`.
-import { augmentPromptForJsonSchema } from '../../shared/structured-output';
-export { augmentPromptForJsonSchema };
+// Re-export the shared structured-output helper so existing Pi callers/tests
+// keep their import path stable; new providers should import directly from
+// `../../shared/structured-output`.
+export { augmentPromptForJsonSchema };
```

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@packages/providers/src/community/pi/provider.ts` around lines 97 - 101, Move
the binding import for augmentPromptForJsonSchema up into the existing
top-of-file import block (so add "import { augmentPromptForJsonSchema } from
'../../shared/structured-output';" alongside the other imports), but leave the
re-export line "export { augmentPromptForJsonSchema };" and its documentation
comment exactly where it is; this keeps module organization consistent while
preserving the current exported API and existing tests/callers.
```

</details>

</blockquote></details>
<details>
<summary>.archon/scripts/maintainer-standup-git-status.ts (1)</summary><blockquote>

`20-27`: **Surface captured stderr when `git` invocations fail.**

The helper pipes stderr to a buffer but discards it on the catch path. When `pull_status` ends up `pull_failed` (line 52) or `current_dev_sha` is empty because `git rev-parse origin/dev` failed (line 55), the user has no diagnostic to act on. Forwarding the error message to `process.stderr` keeps the function's `{ stdout, ok }` contract intact while making failures debuggable.



<details>
<summary>♻️ Proposed refactor</summary>

```diff
 function git(args: string[]): { stdout: string; ok: boolean } {
   try {
     const out = execFileSync('git', args, { stdio: ['ignore', 'pipe', 'pipe'] }).toString();
     return { stdout: out, ok: true };
-  } catch {
+  } catch (e) {
+    const stderr = (e as { stderr?: Buffer }).stderr?.toString() ?? (e as Error).message;
+    process.stderr.write(`git ${args.join(' ')} failed: ${stderr}\n`);
     return { stdout: '', ok: false };
   }
 }
```

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In @.archon/scripts/maintainer-standup-git-status.ts around lines 20 - 27, The
git helper currently swallows stderr on failure; update the git(args: string[])
function so that in the catch path it surfaces the captured stderr (and/or error
message) to process.stderr before returning { stdout: '', ok: false }—preserve
the return contract but write err.stderr (or err.message if stderr is absent) to
process.stderr (e.g., process.stderr.write(...)) so callers like pull_status and
the current_dev_sha lookup (git rev-parse origin/dev) get useful diagnostics
when ok is false.
```

</details>

</blockquote></details>
<details>
<summary>.archon/workflows/maintainer/maintainer-review-pr.yaml (2)</summary><blockquote>

`199-233`: **Mandatory `code-review` is only enforced by prompt — workflow can silently dead-end the review branch.**

The classifier prompt (Line 159) asserts code-review is mandatory, but the workflow gates `code-review` with `when: "$review-classify.output.run_code_review == 'true'"`. If the LLM ever returns `run_code_review: 'false'` (LLM disobedience, schema-coercion edge case, etc.), every aspect node is skipped, `synthesize-review`'s `trigger_rule: one_success` cannot fire, `post-review` never runs, and `report` is also skipped because none of `[post-review, post-decline, approve-unclear]` succeeded. The review path silently produces no output.

Drop the `when:` on `code-review` so the workflow contract matches the prompt. The other four aspects are genuinely conditional and should keep theirs.

<details>
<summary>♻️ Suggested fix</summary>

```diff
   - id: code-review
     command: maintainer-review-code-review
     depends_on: [review-classify]
-    when: "$review-classify.output.run_code_review == 'true'"
     context: fresh
```

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In @.archon/workflows/maintainer/maintainer-review-pr.yaml around lines 199 -
233, The workflow declares code-review mandatory in the classifier but gates the
node with when: "$review-classify.output.run_code_review == 'true'", which can
silently skip all downstream steps; remove the when condition from the node with
id "code-review" (command "maintainer-review-code-review") so that "code-review"
always runs while leaving the other aspect nodes (error-handling, test-coverage,
comment-quality, docs-impact) and their when conditions untouched; ensure
"code-review" still depends_on "review-classify" and retains context "fresh" so
"synthesize-review" (id "synthesize-review") can trigger as expected with its
trigger_rule.
```

</details>

---

`65-82`: **Diff truncation is invisible to the gate — append a marker when capped.**

`head -2500` silently drops lines past the cap. The gate then produces a verdict on a partial diff with no signal that truncation happened, which contradicts the comment on Line 70 ("would produce a confident verdict on no evidence") — same risk applies to "confident verdict on partial evidence." Cheap to fix: emit a trailing marker only when the diff exceeds the cap.

<details>
<summary>♻️ Suggested fix</summary>

```diff
   - id: fetch-diff
     bash: |
       PR_NUM=$(cat "$ARTIFACTS_DIR/.pr-number")
-      # Don't redirect stderr — let auth / network / deleted-PR failures surface
-      # as a node failure rather than feeding an empty diff to the gate (which
-      # would produce a confident verdict on no evidence).
       if ! diff_output=$(gh pr diff "$PR_NUM"); then
         echo "ERROR: gh pr diff failed for PR #$PR_NUM" >&2
         exit 1
       fi
-      # Cap at 2500 lines to keep prompt size bounded; gate cares about shape, not every line.
       if [ -z "$diff_output" ]; then
         echo "(empty diff — PR has no changes)"
       else
-        echo "$diff_output" | head -2500
+        total_lines=$(printf '%s\n' "$diff_output" | wc -l)
+        printf '%s\n' "$diff_output" | head -2500
+        if [ "$total_lines" -gt 2500 ]; then
+          echo "... (diff truncated: showing 2500 of ${total_lines} lines — gate verdict should treat this PR as 'too broad to assess from diff alone')"
+        fi
       fi
```

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In @.archon/workflows/maintainer/maintainer-review-pr.yaml around lines 65 - 82,
The fetch-diff step currently truncates output with `echo "$diff_output" | head
-2500` without signaling truncation; change it to detect when the diff exceeds
the cap and append a visible marker (e.g., "--TRUNCATED--: diff exceeds 2500
lines") only in that case. Concretely, after populating `diff_output` (and
before printing), compute the line count (e.g., via `wc -l` or similar) and if
it is greater than 2500 print the first 2500 lines and then the marker;
otherwise print the full `diff_output` as now. Keep identifiers `fetch-diff`,
`PR_NUM`, and `diff_output` unchanged so the logic is easy to locate.
```

</details>

</blockquote></details>
<details>
<summary>.archon/workflows/maintainer/maintainer-standup.yaml (1)</summary><blockquote>

`150-162`: **Narrow `err` before reading `.message`.**

In the catch on Line 150, `err` is `unknown` under strict TypeScript / Bun. `err.message` will either fail typecheck or, if loosened, evaluate to `undefined` for non-Error throws (e.g., a thrown string), making the recovery line less useful right at the moment it matters most.

<details>
<summary>♻️ Suggested fix</summary>

```diff
       } catch (err) {
-        // Synthesis (Sonnet, ~5 min) is the expensive part. If persist fails
-        // (disk full, read-only fs, permission), dump the brief + state to
-        // stderr so the run isn't a total loss — they're recoverable from logs.
-        process.stderr.write(`PERSIST FAILED: ${err.message}\n`);
+        // Synthesis (Sonnet, ~5 min) is the expensive part. If persist fails
+        // (disk full, read-only fs, permission), dump the brief + state to
+        // stderr so the run isn't a total loss — they're recoverable from logs.
+        const message = err instanceof Error ? err.message : String(err);
+        process.stderr.write(`PERSIST FAILED: ${message}\n`);
         process.stderr.write('--- BEGIN brief_markdown (recoverable from logs) ---\n');
```

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In @.archon/workflows/maintainer/maintainer-standup.yaml around lines 150 - 162,
The catch currently accesses err.message unsafely; change the catch to accept
err: unknown and narrow it before reading .message (e.g., use if (err instanceof
Error) { message = err.message } else { message = String(err) }) and then use
that safe message in the process.stderr.write call(s) instead of err.message;
locate the catch block handling persistence failure (the one writing PERSIST
FAILED and using process.stderr.write with data.brief_markdown and
JSON.stringify(data.next_state)) and apply this narrowing so non-Error throws
don’t produce undefined.
```

</details>

</blockquote></details>
<details>
<summary>.archon/commands/maintainer-review-report.md (1)</summary><blockquote>

`26-30`: **Disambiguate `decline` vs `needs_split` in branch detection.**

Phase 1 lists three possibilities, but Phase 2 reports four branches (`review | decline | needs_split | unclear`). In the workflow both `decline` and `needs_split` produce `decline-comment.md` and run `post-decline`, so artifact existence alone cannot tell them apart — the report has to read the verdict from `gate-decision.md` to pick the correct label. Worth calling out explicitly here so the LLM doesn't always emit `decline`.

<details>
<summary>📝 Suggested clarification</summary>

```diff
 1. **Review branch ran**: `$ARTIFACTS_DIR/review/synthesis.md` exists.
-2. **Decline branch ran**: `$ARTIFACTS_DIR/decline-comment.md` exists with non-placeholder content; the post-decline bash node already posted to GitHub.
+2. **Decline / needs_split branch ran**: `$ARTIFACTS_DIR/decline-comment.md` exists with non-placeholder content and the post-decline bash node already posted to GitHub. Read the verdict from `$ARTIFACTS_DIR/gate-decision.md` to distinguish `decline` from `needs_split` — both share this path.
 3. **Unclear branch ran**: gate verdict was `unclear` and the maintainer was prompted to decide manually.
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In @.archon/commands/maintainer-review-report.md around lines 26 - 30, The
branch-detection currently infers labels from artifact presence (e.g.,
review/synthesis.md and decline-comment.md) which cannot distinguish "decline"
vs "needs_split"; update the report generation logic in maintainer-review-report
to read the gate verdict from gate-decision.md and use that value to choose
between "decline" and "needs_split" (emit "needs_split" only when
gate-decision.md contains that verdict, otherwise emit "decline"), while still
treating the presence of review/synthesis.md as "review" and preserving
"unclear" when the gate verdict is `unclear`.
```

</details>

</blockquote></details>

</blockquote></details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Add error handling for missing or empty PR number file.

If $ARTIFACTS_DIR/.pr-number doesn't exist or is empty, the workflow will fail later with unclear errors when attempting to use $PR_NUMBER. Add an explicit check and fail-fast message.

🛡️ Suggested fix with error handling

 ```bash
-PR_NUMBER=$(cat $ARTIFACTS_DIR/.pr-number)
+if [[ ! -f "$ARTIFACTS_DIR/.pr-number" ]]; then
+  echo "Error: $ARTIFACTS_DIR/.pr-number not found. Gate step may have failed."
+  exit 1
+fi
+PR_NUMBER=$(cat "$ARTIFACTS_DIR/.pr-number")
+if [[ -z "$PR_NUMBER" ]]; then
+  echo "Error: PR number is empty in $ARTIFACTS_DIR/.pr-number"
+  exit 1
+fi

</details>

Based on learnings: "Fail Fast + Explicit Errors: prefer throwing early with a clear error for unsupported or unsafe states — never silently swallow errors."

<!-- suggestion_start -->

<details>
<summary>📝 Committable suggestion</summary>

> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

```suggestion

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.archon/commands/maintainer-review-code-review.md around lines 18 - 20, The
script currently reads PR_NUMBER directly from $ARTIFACTS_DIR/.pr-number without
validating it; add a pre-check to ensure the file exists and is non-empty before
assigning PR_NUMBER: test for the existence of "$ARTIFACTS_DIR/.pr-number" and
exit with a clear error message if missing, then read PR_NUMBER and validate it
is not empty (exit with a clear error if it is). Ensure you reference and use
the same variables PR_NUMBER and ARTIFACTS_DIR used in the script so callers
fail fast with an explicit message rather than proceeding with an unset/empty
PR_NUMBER.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Add error handling for missing gate decision file.

If the gate step failed or was skipped, $ARTIFACTS_DIR/gate-decision.md may not exist. Add an explicit check to fail fast with a clear message.

🛡️ Suggested fix with error handling

 ```bash
+if [[ ! -f "$ARTIFACTS_DIR/gate-decision.md" ]]; then
+  echo "Error: $ARTIFACTS_DIR/gate-decision.md not found. Gate step may not have completed."
+  exit 1
+fi
 cat $ARTIFACTS_DIR/gate-decision.md

</details>

Based on learnings: "Fail Fast + Explicit Errors: prefer throwing early with a clear error for unsupported or unsafe states — never silently swallow errors."

<!-- suggestion_start -->

<details>
<summary>📝 Committable suggestion</summary>

> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

```suggestion

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.archon/commands/maintainer-review-code-review.md around lines 28 - 30, Add
an explicit existence check before attempting to read the gate decision file:
verify that "$ARTIFACTS_DIR/gate-decision.md" exists and if not emit a clear
error message and exit non‑zero, then only run the existing cat
$ARTIFACTS_DIR/gate-decision.md; specifically update the snippet that currently
calls cat to first test [[ -f "$ARTIFACTS_DIR/gate-decision.md" ]] (or
equivalent) and fail fast with a descriptive message if the file is missing so
the script does not silently continue.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Add error handling for gh command failures and clarify sampling strategy.

Two issues:

1. The gh pr diff command can fail due to auth issues, network problems, or an invalid PR number. Add error handling to fail fast with a clear message.
2. The guidance on "too large to reason about cleanly" is subjective. Consider providing a concrete threshold (e.g., file count or line count) or suggesting a systematic sampling approach.

🛡️ Suggested fix with error handling

 ```bash
-gh pr diff $PR_NUMBER
+if ! gh pr diff "$PR_NUMBER" > /tmp/pr-diff.txt 2>&1; then
+  echo "Error: Failed to fetch PR diff for #$PR_NUMBER. Check gh auth status."
+  exit 1
+fi
+cat /tmp/pr-diff.txt

-If the diff is too large to reason about cleanly, sample: read the diff against each changed file individually with gh pr diff $PR_NUMBER -- <path>.
+If the diff is too large to reason about cleanly (e.g., > 50 changed files or > 5,000 lines), sample: read the diff against each changed file individually with gh pr diff "$PR_NUMBER" -- <path>. Prioritize files with the most changes or those in critical paths (e.g., packages/core, packages/workflow).

</details>

Based on learnings: "Fail Fast + Explicit Errors: prefer throwing early with a clear error for unsupported or unsafe states."

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @.archon/commands/maintainer-review-code-review.md around lines 36 - 41,
Update the docs/commands to add explicit failure handling and a concrete
sampling guideline: instruct the script/command that invokes gh pr diff to check
the command's exit status and fail fast with a clear error message when gh pr diff "$PR_NUMBER" fails (mentioning auth/network/invalid PR as possible causes)
and to capture its output to a temporary file for inspection; also replace the
vague "too large" guidance with a concrete threshold (e.g., >50 changed files or

5,000 lines) and recommend systematic sampling by running gh pr diff "$PR_NUMBER" -- <path> for files prioritized by change volume or critical
directories (e.g., packages/core, packages/workflow).

</details>

<!-- fingerprinting:phantom:triton:puma:2cb7b664-7e28-43c9-b2aa-9bbaf49fd757 -->

<!-- 4e71b3a2 -->

<!-- This is an auto-generated comment by CodeRabbit -->

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Ensure the review subdirectory exists before writing findings.

The instructions specify writing to $ARTIFACTS_DIR/review/code-review-findings.md, but there's no guarantee the review/ subdirectory exists. Add a directory creation step to avoid write failures.

Additionally, clarify that the template placeholder <n> in line 79 should be replaced with the actual $PR_NUMBER.

📁 Suggested fix to create directory

 ## Phase 3: WRITE FINDINGS
 
-Write `$ARTIFACTS_DIR/review/code-review-findings.md` with this structure:
+Create the review directory and write `$ARTIFACTS_DIR/review/code-review-findings.md` with this structure:
+
+```bash
+mkdir -p "$ARTIFACTS_DIR/review"
+```
 
 ```markdown
-# Code Review — PR #<n>
+# Code Review — PR #${PR_NUMBER}

Based on learnings: "Fail Fast + Explicit Errors: prefer throwing early with a clear error for unsupported or unsafe states."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.archon/commands/maintainer-review-code-review.md around lines 74 - 108, In
the WRITE FINDINGS step ensure the review output directory exists and the PR
placeholder is substituted: before writing
$ARTIFACTS_DIR/review/code-review-findings.md add mkdir -p
"$ARTIFACTS_DIR/review"; validate required variables (e.g., $ARTIFACTS_DIR and
$PR_NUMBER) and fail fast with a clear error if missing; and replace the
template header "# Code Review — PR #<n>" with "# Code Review — PR
#${PR_NUMBER}" so the actual PR number is written.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the profile.md schema referenced by other scripts/docs and confirm
# whether a git_author field already exists or only gh_handle.
fd -t f 'profile.md' .archon
rg -nP '^(gh_handle|git_author|git_name|git_email)\s*:' --type=md
rg -nP -C2 'gh_handle' .archon

Repository: popemkt/Archon

Length of output: 3502

---

🏁 Script executed:

#!/bin/bash
# 1. Check if there are actual profile.md files (not .example) in the repo
echo "=== Looking for actual profile.md files ==="
find .archon -name "profile.md" -not -name "*.example"

# 2. Check for tests related to maintainer-standup-gh-data.ts
echo -e "\n=== Looking for tests for gh-data script ==="
fd -t f "test\|spec" | grep -i "maintainer\|gh-data" | head -20

# 3. Check how my_recent_commits is used downstream
echo -e "\n=== Usage of my_recent_commits ==="
rg "my_recent_commits" --type ts --type js -C2

# 4. Check if there's any documentation or comments about git author vs gh_handle
echo -e "\n=== Comments about author/handle mapping ==="
rg -i "git.*author|author.*name|author.*email" .archon --type ts --type md | head -20

Repository: popemkt/Archon

Length of output: 326

---

🌐 Web query:

git log --author flag behavior documentation

💡 Result:

The --author flag in git log limits output to commits whose author header lines match the given regular expression pattern. Multiple --author= options select commits matching any pattern (logical OR). By default, it matches against canonical author names after .mailmap normalization; use --no-use-mailmap to match raw commit data instead. The pattern is a regex, not a substring search (e.g., --author=John matches commits with author "John Doe", but --author=d* matches all due to zero or more 'd's).

Citations:

• 1: https://git-scm.com/docs/git-log.html
• 2: https://git-scm.com/docs/git-log
• 3: https://git-scm.com/docs/git-log/2.37.0.html
• 4: https://www.kernel.org/pub/software/scm/git/docs/git-log.html
• 5: https://git-scm.com/docs/git-log/2.42.1.html
• 6: https://blog.plover.com/prog/git-author-address.html

---

🏁 Script executed:

#!/bin/bash
# 1. Check the full output structure of the gh-data script
echo "=== Final JSON output structure (lines 182-200) ==="
sed -n '182,200p' .archon/scripts/maintainer-standup-gh-data.ts

# 2. Search for usage of my_recent_commits in synthesis or output processing
echo -e "\n=== Check for synthesis or output processing of gh-data ==="
rg "recent_commits\|my_recent_commits" .archon --type ts --type js --type md -C1

# 3. Look for any examples or documentation showing the field being used
echo -e "\n=== Documentation examples ==="
rg -A10 "recent_commits\|my_recent_commits\|recent unlabeled" .archon --type md

Repository: popemkt/Archon

Length of output: 622

---

git log --author=<gh_handle> won't match commits — git expects author name/email, not GitHub handle.

--author in git log matches the commit author name/email via regex, not the GitHub username. For most contributors, the GH handle (e.g. popemkt) is not a substring of their git author name (e.g. Pope MKT <pope@example.com>), so my_recent_commits will silently be empty even when the maintainer has shipped commits.

A few options:

1. Read the maintainer's git author from profile.md frontmatter (git_author: separate from gh_handle:), and pass that to --author.
2. Drop --author here and have the synthesis step join commits to GH PRs by SHA via the gh data already collected.
3. Fall back to git config user.email / user.name when no explicit git_author is set, and log a warning if neither resolves.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.archon/scripts/maintainer-standup-gh-data.ts around lines 167 - 180, The
current git log call in the myRecentCommits block uses --author with the GitHub
handle (ghHandle), but git expects commit author name/email so this often yields
no results; fix by resolving a git author identifier (prefer profile frontmatter
git_author if present, otherwise fallback to git config user.name or user.email)
and pass that value to execFileSync's git log --author argument (use lastRunAt
as before), or alternatively remove the --author filter here and perform SHA→PR
joining later; update the myRecentCommits code path (where ghHandle, lastRunAt
and execFileSync are used) to use the resolved git_author fallback chain and log
a warning if no author info is found.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the approval-node executor / DAG runner to confirm reject-without-on_reject semantics.
rg -nP --type=ts -C5 '\bapproval\b.*\b(on_reject|reject|max_attempts)\b' -g '!**/test/**'
rg -nP --type=ts -C3 'capture_response|approval(Node|Executor|Result)' -g '!**/test/**'

Repository: popemkt/Archon

Length of output: 50373

---

🏁 Script executed:

# Find the complete executeApprovalNode function, especially what happens when rejection occurs without on_reject
rg -A 50 'if \(rejectionReason !== .* && node\.approval\.on_reject\)' packages/workflows/src/dag-executor.ts | head -100

Repository: popemkt/Archon

Length of output: 2034

---

🏁 Script executed:

# Search for what happens after the approval node handles rejection — the return statement and full control flow
rg -A 100 'async function executeApprovalNode' packages/workflows/src/dag-executor.ts | tail -80

Repository: popemkt/Archon

Length of output: 2953

---

🏁 Script executed:

# Look for test cases of approval node rejection WITHOUT on_reject to see expected behavior
rg -B 5 -A 15 'approval.*reject.*on_reject' packages/workflows/src/dag-executor.test.ts

Repository: popemkt/Archon

Length of output: 40

---

🏁 Script executed:

# Get the full executeApprovalNode function to see what happens when rejectionReason is set but on_reject is undefined
sed -n '2174,2360p' packages/workflows/src/dag-executor.ts

Repository: popemkt/Archon

Length of output: 6062

---

🏁 Script executed:

# Also check if there's any special handling for approval rejection in the DAG runner that treats rejection as failure
rg -B 5 -A 10 'rejection|pauseWorkflowRun' packages/workflows/src/dag-executor.ts | grep -A 10 'state.*completed\|state.*failed' | head -50

Repository: popemkt/Archon

Length of output: 959

---

🏁 Script executed:

# Check if there's any rejection count or max-attempts logic outside of the on_reject handling
rg -B 3 -A 5 'rejection_count|rejection_reason' packages/workflows/src/dag-executor.ts | grep -A 5 -B 5 'max_attempts\|cancel'

Repository: popemkt/Archon

Length of output: 586

---

🏁 Script executed:

# Search pauseWorkflowRun implementation to see if it has any auto-cancel logic for repeated rejections
rg -A 30 'pauseWorkflowRun' packages/workflows/src/stores/ | head -80

Repository: popemkt/Archon

Length of output: 129

---

🏁 Script executed:

# Find where store files are located
find . -type f -name '*.ts' -path '*/store*' | grep -v node_modules | grep -v test | head -10

Repository: popemkt/Archon

Length of output: 217

---

🏁 Script executed:

# Search for pauseWorkflowRun implementation across the codebase
rg -l 'pauseWorkflowRun' --type=ts | grep -v test | grep -v node_modules

Repository: popemkt/Archon

Length of output: 208

---

🏁 Script executed:

# Check pauseWorkflowRun in the main store implementation
rg -A 30 'pauseWorkflowRun' packages/workflows/src/store.ts | head -80

Repository: popemkt/Archon

Length of output: 1289

---

🏁 Script executed:

# Also check the store-adapter to see if there's rejection handling there
rg -A 30 'pauseWorkflowRun|rejection' packages/core/src/workflows/store-adapter.ts | head -80

Repository: popemkt/Archon

Length of output: 1181

---

🏁 Script executed:

# Find the actual pauseWorkflowRun implementation in the database layer
rg -A 40 'pauseWorkflowRun' packages/core/src/db/workflows.ts

Repository: popemkt/Archon

Length of output: 1628

---

🏁 Script executed:

# Also search for any rejection logic that might happen outside the approval node execution
rg -B 5 -A 10 'rejection_count|rejection_reason' packages/core/src/db/workflows.ts

Repository: popemkt/Archon

Length of output: 40

---

User-facing message claims cancellation without on_reject configured — but code only cancels when on_reject is present.

The message states "Reject = workflow is cancelled," but when on_reject is undefined, rejection triggers re-pause at the approval gate, not cancellation. The condition on line 2208 of dag-executor.ts is if (rejectionReason !== '' && node.approval.on_reject) — rejection handling only activates when both are true. Without on_reject, a rejected approval re-enters the standard pause logic indefinitely (until the max approval timeout or user approves), leaving the report node blocked.

Either configure on_reject with a rejection handler, or revise the message to reflect actual behavior: rejection pauses for human re-review, not immediate cancellation.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.archon/workflows/maintainer/maintainer-review-pr.yaml around lines 309 -
322, The approval block "approve-unclear" claims "Reject = workflow is
cancelled" but the runtime (see dag-executor.ts check using
node.approval.on_reject and the rejectionReason logic) only cancels when an
on_reject handler is configured; otherwise a reject returns the flow to the
approval pause. Fix by either adding an on_reject handler to the approve-unclear
approval (so rejection actually cancels) or update the approval.message text to
accurately state that "Reject" will re-pause the workflow for manual re-review
(not immediately cancel) and include guidance to configure on_reject if
cancellation is desired; reference the approval block id approve-unclear and the
node.approval.on_reject behavior in dag-executor.ts when making the change.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

allowed_tools is Claude-only — remove on Pi nodes.

The workflow runs against provider: pi, but allowed_tools: [] is set on each prompt node. Per coding guidelines, allowed_tools/denied_tools are Claude-only restrictions, so on Pi this is either silently ignored (misleading intent) or rejected at workflow load time. The prompts already enforce "no tools" textually (e.g., line 38), so the YAML key is redundant.

♻️ Proposed fix

   - id: hello
     prompt: 'What is 2+2? Answer with just the number, nothing else.'
-    allowed_tools: []
     effort: low
     idle_timeout: 60000
@@
   - id: identify
     prompt: 'Without using any tools, on a single short line, tell me which model and provider you are.'
-    allowed_tools: []
     idle_timeout: 60000
     depends_on: [hello]
@@
         - "ok":       always true (boolean)
-    allowed_tools: []
     idle_timeout: 60000
     depends_on: [hello]

As per coding guidelines: "allowed_tools/denied_tools per-node restrictions (Claude only)".

Also applies to: 39-39, 51-51

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.archon/workflows/test-workflows/e2e-minimax-smoke.yaml at line 28, Remove
the Claude-only per-node restriction keys from this Pi workflow: delete the
allowed_tools entries (and any denied_tools if present) on the prompt nodes
referenced (the entries currently set near the prompt nodes that also state "no
tools" textually), since provider: pi ignores these keys; leave the textual "no
tools" instructions in the prompt content intact and ensure no other Claude-only
keys remain in the same prompt nodes.

[popemkt]

Thanks @Wirasm — got everything you asked for plus a bit more. Latest commit is ef16bac; branch is mergeable against dev after the merge in 06c3f9e.

Models tested (live, Linux + Copilot CLI 1.0.36)

Model	Workflow	Result
gpt-5-mini	e2e-copilot-smoke	PASS — 12s
gpt-5-mini	e2e-copilot-all-features	PASS — ~110s, all 7 capabilities

The PR description also lists three earlier scenarios on Windows + Copilot CLI 1.0.31 with gpt-5-mini. So we have evidence on two OSes / two CLI minor versions.

Configs used

Auth. copilot login (logged-in user mode) — no env vars set on this machine.

A clarification I'd like to fold into your mental model: GitHub Copilot has only one auth model (GitHub OAuth, billed via your Copilot subscription). The four "options" in the docs (copilot login, COPILOT_GITHUB_TOKEN, GH_TOKEN, GITHUB_TOKEN) are different delivery paths for the same OAuth token, not separate auth schemes — there is no PAT-style standalone Copilot token. The docs page (packages/docs-web/src/content/docs/getting-started/ai-assistants.md) is updated in ef16bac to spell this out, with an explicit warning that GitHub Actions' workflow-scoped ${{ github.token }} doesn't carry Copilot scope.

MCP / skills / agents. Staged at runtime by the smoke workflow itself (no fixtures committed to the repo):

• MCP: setup_mcp_fixture writes .archon/test-fixtures/copilot-mcp.json pointing at npx -y @modelcontextprotocol/server-everything stdio. The smoke's mcp_demo node calls the server's add(2, 3) tool.
• Skills: setup_skills_fixture writes .agents/skills/copilot-smoke/SKILL.md with an unguessable random token (SK_a8f3kL2qZTOK). The smoke's skills_demo invokes the skill and the assert checks for the literal token.
• Agents: defined inline as a customAgents entry on the agents_demo node — task-responder with another unguessable random token (AG_n5k7HpT3wAGOK).

The unguessable tokens are deliberate — they make confabulation impossible. A model that didn't actually receive the SKILL.md body or the agent's prompt field cannot produce those strings from training data. Their presence in the response is mathematical proof the SDK loaded the capability content into the model's context.

A cleanup node with trigger_rule: all_done removes both fixtures whether the workflow passes or fails, so the repo stays tidy.

Results

Capability matrix (7 live, 9 unit-tested, 4 deliberately deferred)

Capability	Live evidence	Unit tests
Basic chat / sendQuery	hello node → PONG	provider.test.ts
Session resume (sessionId)	indirect (smoke runs cold; resume-path is in unit tests)	provider.test.ts
Env injection	indirect (auth env path)	provider.test.ts
effort: reasoning	reasoning node, effort: high → 391 (17×23)	provider.test.ts
thinking: (string form)	—	provider.test.ts
denied_tools	tool_restricted → DENIED_OK (model declined shell tools)	tool-restrictions.test.ts (5)
output_format JSON	structured → {model: "gpt-5-mini", ok: true} (best-effort, shared with Pi coleam00#1297)	structured-output.test.ts
mcp:	mcp_demo → 5 (model invoked everything server's add tool)	mcp-translation.test.ts (4)
skills:	skills_demo → SK_a8f3kL2qZTOK (unguessable, proves SKILL.md body loaded)	skills-translation.test.ts (4)
agents: (custom Task agents)	agents_demo → AG_n5k7HpT3wAGOK (unguessable, proves agent prompt loaded)	agents-translation.test.ts (7)
hooks	not wired	n/a — deferred
fallbackModel	not wired	n/a — deferred
costControl (maxBudgetUsd)	not wired	n/a — deferred
sandbox	not wired	n/a — deferred (Copilot's onPermissionRequest is policy, not sandbox)

Captured run output (bun run cli workflow run e2e-copilot-all-features)

Final assertion output (expand)

── results ──
hello       = PONG
reasoning   = 391
restricted  = DENIED_OK
json.model  = gpt-5-mini
json.ok     = true
mcp_demo    = 5
skills_demo = SK_a8f3kL2qZTOK
agents_demo = Running the registered task-responder agent to retrieve its
              fixed smoke-test token — calling the task-responder sub-agent
              via the task tool and reporting intent first.AG_n5k7HpT3wAGOK
              AG_n5k7HpT3wAGOK
──────────────
PASS: all seven capabilities exercised end-to-end

Pino capability-wiring events (expand)

INFO copilot.skills_resolved   { resolved: 1, missing: [] }
INFO copilot.mcp_loaded        { serverNames: ["everything"], missingVars: [] }
INFO copilot.agents_registered { count: 1, names: ["task-responder"] }

These three events fire at sendQuery time on each node that uses the corresponding capability. They're the deterministic Archon-side proof that translation reached the SDK; the unguessable-token presence in the response is the SDK-side proof.

Per-node timings (expand)

setup_skills_fixture   36ms       (bash, stage SKILL.md)
setup_mcp_fixture      36ms       (bash, stage mcp.json)
hello                  25.7s      (Copilot session start + first prompt)
reasoning              14.5s      (effort: high)
tool_restricted        13.6s      (denied_tools: [shell, write])
structured             19.4s      (output_format JSON)
skills_demo            25.2s      (skills: [copilot-smoke])
mcp_demo               28.6s      (mcp: ./.archon/test-fixtures/copilot-mcp.json)
agents_demo            42.1s      (agents: { task-responder: ... } via Task tool)
assert                 20ms       (bash, all checks)
cleanup                12ms       (bash, remove fixtures)
                       ────
total                  ~110s

Workflow run id: 831b271fdf29a84fee656dca4df846ab (local; not persisted to a shared store).

Simple connectivity smoke (bun run cli workflow run e2e-copilot-smoke)

Output (expand)

[simple] Started
[simple] Completed (10s)
[assert] Started
[assert] Completed (11ms)
PASS: simple=COPILOT_OK
Workflow completed successfully.

Video evidence

Smoke workflow files (committed under .archon/workflows/test-workflows/)

Per coleam00#1431's grouping convention. Both files are documented inline so they double as adoption docs:

• e2e-copilot-smoke.yaml — single-prompt connectivity check. Mirrors e2e-claude-smoke / e2e-pi-smoke.
• e2e-copilot-all-features.yaml — 7-capability multi-node smoke. Mirrors e2e-minimax-smoke. Each scenario carries its own setup/cleanup so it leaves no repo state behind.

To reproduce on your end:

copilot login                                       # one-time
bun run cli workflow cleanup 0                      # avoid auto-resume from any cached state
bun run cli workflow run e2e-copilot-smoke
bun run cli workflow run e2e-copilot-all-features

Side note — CodeRabbit pass

Worked through the 8 inline + 6 nitpick comments from the CodeRabbit review in bf8734b (10 applied, 3 dismissed with rationale on the threads). Summary at #1351 (comment).

Happy to add live MCP/skills smokes against alternate models, drop the gpt-5 (full) coverage on the matrix, or anything else if it'd help. Otherwise this is ready when you are.

[popemkt]

Thanks @Wirasm — got everything you asked for plus a bit more. Latest commit is ef16bac; branch is mergeable against dev after the merge in 06c3f9e.

Models tested (live, Linux + Copilot CLI 1.0.36)

Model	Workflow	Result
gpt-5-mini	e2e-copilot-smoke	PASS — 10.8s
gpt-5-mini	e2e-copilot-all-features	PASS — ~62s, all 7 capabilities

The PR description also lists three earlier scenarios on Windows + Copilot CLI 1.0.31 with gpt-5-mini. So we have evidence on two OSes / two CLI minor versions.

Configs used

Auth. copilot login (logged-in user mode) — no env vars set on this machine.

A clarification I'd like to fold into your mental model: GitHub Copilot has only one auth model (GitHub OAuth, billed via your Copilot subscription). The four "options" in the docs (copilot login, COPILOT_GITHUB_TOKEN, GH_TOKEN, GITHUB_TOKEN) are different delivery paths for the same OAuth token, not separate auth schemes — there is no PAT-style standalone Copilot token. The docs page (packages/docs-web/src/content/docs/getting-started/ai-assistants.md) is updated in ef16bac to spell this out, with an explicit warning that GitHub Actions' workflow-scoped ${{ github.token }} doesn't carry Copilot scope.

MCP / skills / agents. Staged at runtime by the smoke workflow itself (no fixtures committed to the repo):

• MCP: setup_mcp_fixture writes .archon/test-fixtures/copilot-mcp.json pointing at npx -y @modelcontextprotocol/server-everything stdio. The smoke's mcp_demo node calls the server's add(2, 3) tool.
• Skills: setup_skills_fixture writes .agents/skills/copilot-smoke/SKILL.md with an unguessable random token (SK_a8f3kL2qZTOK). The smoke's skills_demo invokes the skill and the assert checks for the literal token.
• Agents: defined inline as a customAgents entry on the agents_demo node — task-responder with another unguessable random token (AG_n5k7HpT3wAGOK).

The unguessable tokens are deliberate — they make confabulation impossible. A model that didn't actually receive the SKILL.md body or the agent's prompt field cannot produce those strings from training data. Their presence in the response is mathematical proof the SDK loaded the capability content into the model's context.

A cleanup node with trigger_rule: all_done removes both fixtures whether the workflow passes or fails, so the repo stays tidy.

Results

Capability matrix (7 live, 9 unit-tested, 4 deliberately deferred)

Capability	Live evidence	Unit tests
Basic chat / sendQuery	hello node → PONG	provider.test.ts
Session resume (sessionId)	indirect (smoke runs cold; resume-path is in unit tests)	provider.test.ts
Env injection	indirect (auth env path)	provider.test.ts
effort: reasoning	reasoning node, effort: high → 391 (17×23)	provider.test.ts
thinking: (string form)	—	provider.test.ts
denied_tools	tool_restricted → DENIED_OK (model declined shell tools)	tool-restrictions.test.ts (5)
output_format JSON	structured → {model: "gpt-5-mini", ok: true} (best-effort, shared with Pi coleam00#1297)	structured-output.test.ts
mcp:	mcp_demo → 5 (model invoked everything server's add tool)	mcp-translation.test.ts (4)
skills:	skills_demo → SK_a8f3kL2qZTOK (unguessable, proves SKILL.md body loaded)	skills-translation.test.ts (4)
agents: (custom Task agents)	agents_demo → AG_n5k7HpT3wAGOK (unguessable, proves agent prompt loaded)	agents-translation.test.ts (7)
hooks	not wired	n/a — deferred
fallbackModel	not wired	n/a — deferred
costControl (maxBudgetUsd)	not wired	n/a — deferred
sandbox	not wired	n/a — deferred (Copilot's onPermissionRequest is policy, not sandbox)

Captured run output (bun run cli workflow run e2e-copilot-all-features)

Final assertion output (expand)

── results ──
hello       = PONG
reasoning   = 391
restricted  = DENIED_OK
json.model  = gpt-5-mini
json.ok     = true
mcp_demo    = Computing 2 + 3 using the MCP add tool on the everything
              server. Running the add call now.5
skills_demo = SK_a8f3kL2qZTOK
agents_demo = Invoking the task-responder sub-agent to retrieve the fixed
              smoke-test token. Running it now.AG_n5k7HpT3wAGOKAG_n5k7HpT3wAGOK
──────────────
PASS: all seven capabilities exercised end-to-end

Pino capability-wiring events (expand)

INFO copilot.skills_resolved   { resolved: 1, missing: [] }
INFO copilot.mcp_loaded        { serverNames: ["everything"], missingVars: [] }
INFO copilot.agents_registered { count: 1, names: ["task-responder"] }

These three events fire at sendQuery time on each node that uses the corresponding capability. They're the deterministic Archon-side proof that translation reached the SDK; the unguessable-token presence in the response is the SDK-side proof.

Per-node timings (expand)

setup_skills_fixture   35ms       (bash, stage SKILL.md)
setup_mcp_fixture      35ms       (bash, stage mcp.json)
hello                  13.3s      (Copilot session start + first prompt)
reasoning              11.4s      (effort: high)
tool_restricted        14.0s      (denied_tools: [shell, write])
structured             14.6s      (output_format JSON)
skills_demo            25.4s      (skills: [copilot-smoke])
agents_demo            37.8s      (agents: { task-responder: ... } via Task tool)
mcp_demo               49.0s      (mcp: ./.archon/test-fixtures/copilot-mcp.json — slowest, npx cold-spawns the everything server)
assert                 22ms       (bash, all 7 checks)
cleanup                15ms       (bash, remove fixtures)
                       ─────
total                  ~62s       (parallel layers, real wall-clock)

Workflow run id: 72860ab9d699cdbb8e117153c8b91137. Layers 2 and 3 run in parallel, so wall-clock is much less than the sum of individual durations.

Simple connectivity smoke (bun run cli workflow run e2e-copilot-smoke)

Output (expand)

[simple] Started
[simple] Completed (10.8s)
[assert] Started
[assert] Completed (11ms)
PASS: simple=COPILOT_OK
Workflow completed successfully.

Run id: daf59b6774dcf92630100e98635a30c1.

Video evidence

Smoke workflow files (committed under .archon/workflows/test-workflows/)

Per coleam00#1431's grouping convention. Both files are documented inline so they double as adoption docs:

• e2e-copilot-smoke.yaml — single-prompt connectivity check. Mirrors e2e-claude-smoke / e2e-pi-smoke.
• e2e-copilot-all-features.yaml — 7-capability multi-node smoke. Mirrors e2e-minimax-smoke. Each scenario carries its own setup/cleanup so it leaves no repo state behind.

To reproduce on your end:

copilot login                                       # one-time
bun run cli workflow cleanup 0                      # avoid auto-resume from any cached state
bun run cli workflow run e2e-copilot-smoke
bun run cli workflow run e2e-copilot-all-features

Side note — CodeRabbit pass

Worked through the 8 inline + 6 nitpick comments from the CodeRabbit review in bf8734b (10 applied, 3 dismissed with rationale on the threads). Summary at #1351 (comment).

Happy to add live MCP/skills smokes against alternate models, drop the gpt-5 (full) coverage on the matrix, or anything else if it'd help. Otherwise this is ready when you are.

[popemkt]

Thanks @Wirasm — quick run-down on your four asks. Latest commit: ef16bac. Branch is mergeable.

Models tested: gpt-5-mini live on Linux + Copilot CLI 1.0.36, plus three earlier scenarios on Windows + CLI 1.0.31 (in PR description).

Configs: copilot login for auth (no env tokens). MCP / skills / agents all staged at runtime by the smoke workflow itself — no fixtures committed to the repo, cleanup node strips them after.

Results: one DAG covers all 7 wired capabilities — chat, effort: high, denied_tools, output_format JSON, mcp:, skills:, agents:. Skill and agent assertions use unguessable random tokens (SK_a8f3kL2qZTOK / AG_n5k7HpT3wAGOK) so the model can't confabulate them — their presence in the response is proof the SDK loaded the SKILL.md body / agent prompt into context.

Run output — bun run cli workflow run e2e-copilot-all-features (~62s)

── results ──
hello       = PONG
reasoning   = 391
restricted  = DENIED_OK
json.model  = gpt-5-mini
json.ok     = true
mcp_demo    = ...Running the add call now.5
skills_demo = SK_a8f3kL2qZTOK
agents_demo = ...AG_n5k7HpT3wAGOKAG_n5k7HpT3wAGOK
──────────────
PASS: all seven capabilities exercised end-to-end

# Pino capability-wiring events:
INFO copilot.skills_resolved   { resolved: 1, missing: [] }
INFO copilot.mcp_loaded        { serverNames: ["everything"], missingVars: [] }
INFO copilot.agents_registered { count: 1, names: ["task-responder"] }

Smoke workflow: .archon/workflows/test-workflows/e2e-copilot-{smoke,all-features}.yaml (per coleam00#1431's grouping). Both documented inline so they double as adoption docs.

One small ask while I have you: the docs page (packages/docs-web/.../ai-assistants.md) had four auth options listed flat — I tightened it in ef16bac to make clear Copilot has only one auth model (GitHub OAuth, subscription-billed); the four "options" are just delivery paths for the same OAuth token. Worth a quick eyeball if you have a sec.

[popemkt]

Thanks @Wirasm — quick run-down on your four asks.

Models tested: gpt-5-mini live on Linux + Copilot CLI 1.0.36, plus three earlier scenarios on Windows + CLI 1.0.31 (in PR description).

Configs: copilot login for auth (no env tokens). MCP / skills / agents staged at runtime by the smoke workflow itself — no fixtures committed, cleanup node strips them after.

Results: one DAG covers all 7 wired capabilities — chat, effort: high, denied_tools, output_format JSON, mcp:, skills:, agents:. All assertions are pretty self-evident, you can inspect the bash, skill and agent assertions use unguessable random tokens (SK_a8f3kL2qZTOK / AG_n5k7HpT3wAGOK), the model can't fake them.

── results ──
hello       = PONG
reasoning   = 391
restricted  = DENIED_OK
json.model  = gpt-5-mini, json.ok = true
mcp_demo    = ...Running the add call now.5
skills_demo = SK_a8f3kL2qZTOK
agents_demo = ...AG_n5k7HpT3wAGOKAG_n5k7HpT3wAGOK
──────────────
PASS: all seven capabilities exercised end-to-end

Smoke files: .archon/workflows/test-workflows/e2e-copilot-{smoke,all-features}.yaml (per coleam00#1431's grouping). Documented inline so they double as adoption docs.

Full run output — bun run cli workflow run e2e-copilot-all-features (~62s)

Dispatching workflow: **e2e-copilot-all-features**
{"level":30,"time":1777306037705,"pid":282416,"hostname":"fedora","module":"provider.claude","authMode":"global","msg":"using_global_auth"}
{"level":30,"time":1777306037714,"pid":282416,"hostname":"fedora","module":"workflow.executor","workflowName":"e2e-copilot-all-features","provider":"copilot","providerSource":"workflow definition","model":"gpt-5-mini","msg":"workflow_provider_resolved"}
{"level":30,"time":1777306037719,"pid":282416,"hostname":"fedora","module":"workflow.executor","workflowName":"e2e-copilot-all-features","workflowRunId":"72860ab9d699cdbb8e117153c8b91137","hasIssueContext":false,"issueContextLength":0,"msg":"workflow_starting"}
🚀 **Starting workflow**: `e2e-copilot-all-features`

> Copilot provider feature smoke — chat + effort + tool restrictions + structured output + MCP + skills + agents.
{"level":30,"time":1777306037734,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","workflowName":"e2e-copilot-all-features","nodeCount":11,"layerCount":4,"hasIssueContext":false,"issueContextLength":0,"msg":"dag_workflow_starting"}
{"level":30,"time":1777306037736,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"setup_skills_fixture","type":"bash","msg":"dag_node_started"}
{"level":30,"time":1777306037736,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"setup_mcp_fixture","type":"bash","msg":"dag_node_started"}
{"level":30,"time":1777306037738,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"hello","provider":"copilot","msg":"dag_node_started"}
[setup_skills_fixture] Started
[setup_mcp_fixture] Started
[hello] Started
{"level":30,"time":1777306037766,"pid":282416,"hostname":"fedora","module":"copilot-binary","binaryPath":"/stuff/WorkSpace/Archon/node_modules/.bin/copilot","source":"path","msg":"copilot.binary_resolved"}
{"level":30,"time":1777306037771,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"setup_skills_fixture","durationMs":35,"msg":"dag_node_completed"}
{"level":30,"time":1777306037771,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"setup_mcp_fixture","durationMs":35,"msg":"dag_node_completed"}
[setup_skills_fixture] Completed (35ms)
[setup_mcp_fixture] Completed (35ms)
{"level":40,"time":1777306043827,"pid":282416,"hostname":"fedora","module":"provider.claude","rateLimitInfo":{"status":"allowed","resetsAt":1777320600,"rateLimitType":"five_hour","overageStatus":"allowed","overageResetsAt":1777306200,"isUsingOverage":false},"msg":"claude.rate_limit_event"}
{"level":30,"time":1777306044469,"pid":282416,"hostname":"fedora","module":"service.title-generator","conversationDbId":"0053c6e4e8e0178b2c1dd9c9a593a260","title":"Run E2E Copilot All Features","msg":"title.generate_completed"}
P

ONG
{"level":30,"time":1777306051039,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"hello","durationMs":13301,"msg":"dag_node_completed"}
[hello] Completed (13.3s)
{"level":30,"time":1777306051044,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"reasoning","provider":"copilot","msg":"dag_node_started"}
{"level":30,"time":1777306051044,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"tool_restricted","provider":"copilot","msg":"dag_node_started"}
{"level":30,"time":1777306051044,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"structured","provider":"copilot","msg":"dag_node_started"}
{"level":30,"time":1777306051044,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"agents_demo","provider":"copilot","msg":"dag_node_started"}
{"level":30,"time":1777306051044,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"skills_demo","provider":"copilot","msg":"dag_node_started"}
[reasoning] Started
[tool_restricted] Started
[agents_demo] Started
[skills_demo] Started
[structured] Started
{"level":30,"time":1777306051071,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"mcp_demo","provider":"copilot","msg":"dag_node_started"}
{"level":30,"time":1777306051073,"pid":282416,"hostname":"fedora","module":"copilot-binary","binaryPath":"/stuff/WorkSpace/Archon/node_modules/.bin/copilot","source":"path","msg":"copilot.binary_resolved"}
[mcp_demo] Started
{"level":30,"time":1777306051083,"pid":282416,"hostname":"fedora","module":"copilot-binary","binaryPath":"/stuff/WorkSpace/Archon/node_modules/.bin/copilot","source":"path","msg":"copilot.binary_resolved"}
{"level":30,"time":1777306051084,"pid":282416,"hostname":"fedora","module":"copilot-binary","binaryPath":"/stuff/WorkSpace/Archon/node_modules/.bin/copilot","source":"path","msg":"copilot.binary_resolved"}
{"level":30,"time":1777306051085,"pid":282416,"hostname":"fedora","module":"provider.copilot","resolved":1,"missing":[],"msg":"copilot.skills_resolved"}
{"level":30,"time":1777306051086,"pid":282416,"hostname":"fedora","module":"provider.copilot","serverNames":["everything"],"missingVars":[],"msg":"copilot.mcp_loaded"}
{"level":30,"time":1777306051088,"pid":282416,"hostname":"fedora","module":"copilot-binary","binaryPath":"/stuff/WorkSpace/Archon/node_modules/.bin/copilot","source":"path","msg":"copilot.binary_resolved"}
{"level":30,"time":1777306051088,"pid":282416,"hostname":"fedora","module":"provider.copilot","count":1,"names":["task-responder"],"msg":"copilot.agents_registered"}
{"level":30,"time":1777306051091,"pid":282416,"hostname":"fedora","module":"copilot-binary","binaryPath":"/stuff/WorkSpace/Archon/node_modules/.bin/copilot","source":"path","msg":"copilot.binary_resolved"}
{"level":30,"time":1777306051092,"pid":282416,"hostname":"fedora","module":"copilot-binary","binaryPath":"/stuff/WorkSpace/Archon/node_modules/.bin/copilot","source":"path","msg":"copilot.binary_resolved"}
391
{"level":30,"time":1777306062470,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"reasoning","durationMs":11426,"msg":"dag_node_completed"}
[reasoning] Completed (11.4s)
DEN

IED

_OK
{"level":30,"time":1777306065092,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"tool_restricted","durationMs":14047,"msg":"dag_node_completed"}
[tool_restricted] Completed (14s)
{"model":"gpt-5-mini","ok":true}
{"level":30,"time":1777306065623,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"structured","durationMs":14579,"msg":"dag_node_completed"}
[structured] Completed (14.6s)
SK

_a

8

f

3

k

L

2

q

ZT

OK
{"level":30,"time":1777306076448,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"skills_demo","durationMs":25404,"msg":"dag_node_completed"}
[skills_demo] Completed (25.4s)
Inv

oking

 the

 task

-res

ponder

 sub

-agent

 to

 retrieve

 the

 fixed

 smoke

-test

 token

.

 Running

 it

 now

.

AG_n5k7HpT3wAGOK

AG

_n

5

k

7

Hp

T

3

w

AG

OK
{"level":30,"time":1777306088834,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"agents_demo","durationMs":37790,"msg":"dag_node_completed"}
[agents_demo] Completed (37.8s)
Comput

ing

 

2

 +

 

3

 using

 the

 MCP

 add

 tool

 on

 the

 everything

 server

.

 Running

 the

 add

 call

 now

.

5
{"level":30,"time":1777306100084,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"mcp_demo","durationMs":49039,"msg":"dag_node_completed"}
[mcp_demo] Completed (49s)
{"level":30,"time":1777306100089,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"assert","type":"bash","msg":"dag_node_started"}
[assert] Started
{"level":40,"time":1777306100107,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"assert","stderr":"── results ──\nhello       = PONG\nreasoning   = 391\nrestricted  = DENIED_OK\njson.model  = gpt-5-mini\njson.ok     = true\nmcp_demo    = Computing 2 + 3 using the MCP add tool on the everything server. Running the add call now.5\nskills_demo = SK_a8f3kL2qZTOK\nagents_demo = Invoking the task-responder sub-agent to retrieve the fixed smoke-test token. Running it now.AG_n5k7HpT3wAGOKAG_n5k7HpT3wAGOK\n──────────────\nPASS: all seven capabilities exercised end-to-end","msg":"bash_node_stderr"}
Bash node 'assert' stderr:

── results ──
hello       = PONG
reasoning   = 391
restricted  = DENIED_OK
json.model  = gpt-5-mini
json.ok     = true
mcp_demo    = Computing 2 + 3 using the MCP add tool on the everything server. Running the add call now.5
skills_demo = SK_a8f3kL2qZTOK
agents_demo = Invoking the task-responder sub-agent to retrieve the fixed smoke-test token. Running it now.AG_n5k7HpT3wAGOKAG_n5k7HpT3wAGOK
──────────────
PASS: all seven capabilities exercised end-to-end

{"level":30,"time":1777306100110,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"assert","durationMs":22,"msg":"dag_node_completed"}
[assert] Completed (22ms)
{"level":30,"time":1777306100114,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"cleanup","type":"bash","msg":"dag_node_started"}
[cleanup] Started
{"level":30,"time":1777306100129,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"cleanup","durationMs":15,"msg":"dag_node_completed"}
[cleanup] Completed (15ms)
{"level":30,"time":1777306100134,"pid":282416,"hostname":"fedora","module":"workflow.dag-executor","nodeCount":11,"anyCompleted":true,"anyFailed":false,"msg":"dag_workflow_finished"}
cleanup complete

Workflow completed successfully.

Full run output — bun run cli workflow run e2e-copilot-smoke (~11s)

Dispatching workflow: **e2e-copilot-smoke**
{"level":30,"time":1777306126378,"pid":284572,"hostname":"fedora","module":"provider.claude","authMode":"global","msg":"using_global_auth"}
{"level":30,"time":1777306126388,"pid":284572,"hostname":"fedora","module":"workflow.executor","workflowName":"e2e-copilot-smoke","provider":"copilot","providerSource":"workflow definition","model":"gpt-5-mini","msg":"workflow_provider_resolved"}
{"level":30,"time":1777306126393,"pid":284572,"hostname":"fedora","module":"workflow.executor","workflowName":"e2e-copilot-smoke","workflowRunId":"daf59b6774dcf92630100e98635a30c1","hasIssueContext":false,"issueContextLength":0,"msg":"workflow_starting"}
🚀 **Starting workflow**: `e2e-copilot-smoke`

> Smoke test for the GitHub Copilot community provider.
{"level":30,"time":1777306126416,"pid":284572,"hostname":"fedora","module":"workflow.dag-executor","workflowName":"e2e-copilot-smoke","nodeCount":2,"layerCount":2,"hasIssueContext":false,"issueContextLength":0,"msg":"dag_workflow_starting"}
{"level":30,"time":1777306126419,"pid":284572,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"simple","provider":"copilot","msg":"dag_node_started"}
[simple] Started
{"level":30,"time":1777306126440,"pid":284572,"hostname":"fedora","module":"copilot-binary","binaryPath":"/stuff/WorkSpace/Archon/node_modules/.bin/copilot","source":"path","msg":"copilot.binary_resolved"}
{"level":40,"time":1777306133883,"pid":284572,"hostname":"fedora","module":"provider.claude","rateLimitInfo":{"status":"allowed","resetsAt":1777314000,"rateLimitType":"five_hour","overageStatus":"allowed","overageResetsAt":1777593600,"isUsingOverage":false},"msg":"claude.rate_limit_event"}
{"level":30,"time":1777306134434,"pid":284572,"hostname":"fedora","module":"service.title-generator","conversationDbId":"c2a6d79b3ff57bd2a2a2a8d1be283c94","title":"E2E Copilot Smoke Test Run","msg":"title.generate_completed"}
COP

IL

OT

_OK
{"level":30,"time":1777306137192,"pid":284572,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"simple","durationMs":10773,"msg":"dag_node_completed"}
[simple] Completed (10.8s)
{"level":30,"time":1777306137199,"pid":284572,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"assert","type":"bash","msg":"dag_node_started"}
[assert] Started
{"level":30,"time":1777306137210,"pid":284572,"hostname":"fedora","module":"workflow.dag-executor","nodeId":"assert","durationMs":11,"msg":"dag_node_completed"}
[assert] Completed (11ms)
{"level":30,"time":1777306137215,"pid":284572,"hostname":"fedora","module":"workflow.dag-executor","nodeCount":2,"anyCompleted":true,"anyFailed":false,"msg":"dag_workflow_finished"}
PASS: simple=COPILOT_OK

Workflow completed successfully.

Side note while I have you: the docs page had four auth options listed flat. Tightened in ef16bac to make clear Copilot has one auth model (GitHub OAuth, subscription-billed); the four "options" are delivery paths for the same OAuth token. Worth a quick eyeball.