Skip to content

fix(cli): use source checkout cwd for workflow discovery on resume/approve/reject#1743

Merged
Wirasm merged 4 commits into
devfrom
archon/thread-1043f57f
May 22, 2026
Merged

fix(cli): use source checkout cwd for workflow discovery on resume/approve/reject#1743
Wirasm merged 4 commits into
devfrom
archon/thread-1043f57f

Conversation

@Wirasm
Copy link
Copy Markdown
Collaborator

@Wirasm Wirasm commented May 21, 2026

Summary

  • Problem: workflow approve, workflow resume, and workflow reject fail with "Workflow '<name>' not found" for project-scoped workflows when worktree.enabled: false (or when the workflow YAML lives only in the user's live checkout, not in the execution path)
  • Why it matters: Approval-gate workflows are completely unusable for users who keep their workflow YAMLs only in their local repo — the interactive review flow breaks at the first human gate
  • What changed: Added discoveryCwd?: string to WorkflowRunOptions; resume/approve/reject now look up the codebase record and pass codebase.default_cwd as the discovery root so loadWorkflows() searches the original source checkout instead of the execution worktree/clone path
  • What did NOT change: working_path storage, findResumableRun signature, executor, web/chat orchestrator path, database schema — execution cwd is unchanged; only discovery cwd is separated

UX Journey

Before

User                      CLI                            DB / FS
────                      ───                            ────────
/workflow approve <id>──▶ look up run
                          workflowRunCommand(run.working_path)
                          loadWorkflows(working_path) ──▶ scan worktree FS
                                                         [workflow.yaml NOT found]
                          "Workflow 'foo' not found" ◀── error thrown
User sees error ◀─────────

After

User                      CLI                            DB / FS
────                      ───                            ────────
/workflow approve <id>──▶ look up run
                        [look up codebase by run.codebase_id]
                          workflowRunCommand(
                            run.working_path,
                            { discoveryCwd: codebase.default_cwd } [new]
                          )
                          loadWorkflows(discoveryCwd) ──▶ scan source checkout
                                                         [workflow.yaml FOUND]
                          resume execution ──────────────▶ AI continues
User sees AI response ◀──

Architecture Diagram

Before

workflowResumeCommand
  └─▶ workflowRunCommand(run.working_path)
        └─▶ loadWorkflows(cwd)          [cwd = working_path = worktree/clone]
              └─▶ discoverWorkflowsWithConfig(cwd, ...)  [misses source YAML]

workflowApproveCommand
  └─▶ workflowRunCommand(result.workingPath)
        └─▶ loadWorkflows(cwd)          [same problem]

After

workflowResumeCommand
  [+] getCodebase(run.codebase_id) ──▶ codebase.default_cwd
  └─▶ workflowRunCommand(run.working_path, { discoveryCwd: codebase.default_cwd })
        └─▶ loadWorkflows(discoveryCwd) [~] [now uses source checkout path]
              └─▶ discoverWorkflowsWithConfig(discoveryCwd, ...)  [finds YAML]

workflowApproveCommand  [~ same fix]
workflowRejectCommand   [~ same fix — sibling path with identical root cause]

Connection inventory:

From To Status Notes
workflowResumeCommand getCodebase new Fetch source cwd for discovery
workflowApproveCommand getCodebase new Same
workflowRejectCommand getCodebase new Same
workflowRunCommand loadWorkflows modified Now uses discoveryCwd ?? cwd
WorkflowRunOptions discoveryCwd field new Separates discovery from execution cwd
All other connections unchanged

Label Snapshot

  • Risk: risk: low
  • Size: size: S
  • Scope: cli
  • Module: cli:workflow

Change Metadata

  • Change type: bug
  • Primary scope: cli

Linked Issue

Validation Evidence (required)

bun run validate

All six checks passed:

Check Result
check:bundled ✅ Pass (36 commands, 20 workflows)
check:bundled-skill ✅ Pass (21 files)
bun run type-check ✅ All 10 packages clean
bun run lint ✅ 0 errors, 0 warnings
bun run format:check ✅ Prettier clean
bun run test ✅ 0 failures (2,979+ tests across all packages)

3 new regression tests added:

  • workflowResumeCommand > should discover workflows from codebase.default_cwd, not working_path
  • workflowResumeCommand > should fall back to working_path for discovery when codebase_id is missing
  • workflowApproveCommand > should discover workflows from codebase.default_cwd, not working_path

Security Impact (required)

  • New permissions/capabilities? No
  • New external network calls? No
  • Secrets/tokens handling changed? No
  • File system access scope changed? No — the discovery root was already reachable; this just points discovery at the right directory

Compatibility / Migration

  • Backward compatible? Yes — discoveryCwd is optional; existing callers unchanged. Runs without a codebase_id fall back to working_path (preserving pre-existing behavior).
  • Config/env changes? No
  • Database migration needed? No

Human Verification (required)

  • Verified scenarios: Code review of the fix logic; test assertions confirm discoverWorkflowsWithConfig is called with codebase.default_cwd and not working_path
  • Edge cases checked: missing codebase_id (fallback to working_path), getCodebase throws (warn + fallback, graceful continue), reject command sibling path
  • What was not verified: Live end-to-end run with a real worktree — the 5-step reproduction from workflow approve/resume fails for project-scoped workflows with worktree.enabled: false #1663 was not manually replayed

Side Effects / Blast Radius (required)

  • Affected subsystems/workflows: CLI workflow resume, workflow approve, workflow reject commands only
  • Potential unintended effects: None — the execution cwd (working_path) is unchanged; only the discovery lookup is redirected
  • Guardrails/monitoring for early detection: The three new unit tests cover the regression directly

Rollback Plan (required)

  • Fast rollback command/path: git revert HEAD~1 (the fix commit is isolated)
  • Feature flags or config toggles: None
  • Observable failure symptoms: "Workflow '<name>' not found" on workflow approve/resume/reject (same error as before the fix)

Risks and Mitigations

  • Risk: getCodebase DB call added to hot path of approve/resume/reject — could add latency or fail if DB is unavailable
    • Mitigation: Failure is caught, warned, and falls back to the prior behavior (working_path as discovery root) — no regression on DB hiccup, just the old bug reappears temporarily

Summary by CodeRabbit

  • Bug Fixes
    • Fixed workflow approve/resume/reject commands erroring with "Workflow not found" when the run's working path is a worktree or workspace clone.

Review Change Stack

Wirasm added 2 commits May 21, 2026 17:01
When a workflow paused at an approval gate is resumed via `workflow approve` or
`workflow resume`, the CLI re-invoked `workflowRunCommand` with `run.working_path`
as the discovery cwd. If `working_path` is a worktree or workspace clone that
does not contain the user's local (often untracked) workflow YAML, discovery
failed with "Workflow 'foo' not found" before execution could begin.

Separate the discovery path from the execution path by adding an optional
`discoveryCwd` to `WorkflowRunOptions`. Resume, approve, and reject now look up
the codebase and pass `codebase.default_cwd` as `discoveryCwd`, so the source
repo is searched even when `working_path` lives elsewhere. The execution cwd
and the existing `findResumableRun` keying are unchanged.

Changes:
- Add `WorkflowRunOptions.discoveryCwd`; use it for `loadWorkflows` in
  `workflowRunCommand`
- `workflowResumeCommand`, `workflowApproveCommand`, and `workflowRejectCommand`
  resolve `codebase.default_cwd` (with graceful fallback) and pass it through
- Tests covering discovery from `codebase.default_cwd` and fallback to
  `working_path` when no codebase is available

Fixes #1663
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 21, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 435a2d95-1799-4fc8-8aef-28ca9d453d4d

📥 Commits

Reviewing files that changed from the base of the PR and between aa71520 and 2a33a9e.

📒 Files selected for processing (7)
  • .archon/workflows/defaults/archon-refactor-safely.yaml
  • .archon/workflows/defaults/archon-workflow-builder.yaml
  • CHANGELOG.md
  • packages/cli/src/commands/workflow.test.ts
  • packages/cli/src/commands/workflow.ts
  • packages/workflows/src/dag-executor.ts
  • packages/workflows/src/defaults/bundled-defaults.generated.ts

📝 Walkthrough

Walkthrough

This PR fixes workflow discovery during resume, approve, and reject operations to use the codebase's registered source directory instead of the run's working path. It adds a discoveryCwd routing option, updates three CLI commands with codebase-aware discovery and fallback logic, expands test coverage, refactors DAG executor skip handling, and updates two workflow templates.

Changes

Workflow Discovery and Resume Flow

Layer / File(s) Summary
Discovery path routing infrastructure
packages/cli/src/commands/workflow.ts
Add optional discoveryCwd field to WorkflowRunOptions to allow callers to override workflow discovery root independently of execution working directory. Update workflowRunCommand to use discoveryCwd when provided.
Resume command discovery logic
packages/cli/src/commands/workflow.ts
workflowResumeCommand now looks up the run's codebase via codebase_id, uses its default_cwd as the discovery root, falls back to working_path if codebase lookup fails, warns on failures, and passes the resolved path to workflow run.
Approve command discovery logic
packages/cli/src/commands/workflow.ts
workflowApproveCommand computes discoveryCwd from the approval result's codebase record and passes it to the post-approval resume, with codebase source directory preference, fallback to working_path, and warning on missing codebase.
Reject command discovery and documentation
packages/cli/src/commands/workflow.ts
Update workflowRejectCommand doc comment to clarify on-reject auto-resume vs cancellation behavior. Implement codebase-aware discovery matching resume and approve: use default_cwd with working_path fallback and warning on lookup failure.
Resume/approve/reject test expansion
packages/cli/src/commands/workflow.test.ts
Expand regression coverage across all three commands to verify discovery CWD selection (codebase source vs working path), codebase lookup failures and soft fallback, warning event emission, and behavior when codebase_id is missing.
DAG executor skip node handling
packages/workflows/src/dag-executor.ts
Refactor per-node resume behavior when node is skipped: consolidate always_run reset and skip paths under single conditional, improve logNodeSkip error handling, emit node_skipped_prior_success event with prior output, and return pre-populated cached output.

Workflow Template Updates

Layer / File(s) Summary
Refactor-safely workflow artifact persistence
.archon/workflows/defaults/archon-refactor-safely.yaml
Update analyze-impact and plan-refactor nodes to write analysis and refactor plan files directly to $ARTIFACTS_DIR instead of producing captured inline output. Enable Write tool in both nodes, update node dependencies so plan-refactor depends on analyze-impact, and shift execute-refactor gate to depend on plan-refactor instead of persist-plan.
Workflow-builder template $ARGUMENTS relaxation
.archon/workflows/defaults/archon-workflow-builder.yaml
Remove the requirement that generated workflows must include $ARGUMENTS or $USER_MESSAGE in node prompts. Delete the corresponding validate-yaml validation check that scanned generated YAML for those variables.

Release Documentation

  • CHANGELOG.md: Document the fix for workflow approve/resume/reject discovery failure when run working path is a worktree/workspace clone, noting the switch to codebase default_cwd with fallback to working_path.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • coleam00/Archon#1663: Directly addresses the reported issue where workflow approve/resume/reject fails with "Workflow not found" when working path is a worktree/workspace clone, by switching discovery to use codebase.default_cwd with fallback to working_path.

Possibly related PRs

  • coleam00/Archon#1734: Modifies the same archon-refactor-safely.yaml workflow nodes and their artifact/persistence wiring, tightly related to the refactor-safely template improvements.
  • coleam00/Archon#1730: Both PRs modify the DAG executor resume logic in packages/workflows/src/dag-executor.ts around how always_run nodes are handled and associated resume/audit event emission.
  • coleam00/Archon#1530: Overlaps with the main PR's DAG executor changes to how node_skipped_prior_success events are emitted and prior outputs restored during resume.

Poem

🐰 The warren's workflows now find their way,
No matter where the coded paths may stray,
From workspace clone to source's true abode,
Discovery hops along the codebase road,
With fallback trails for when lookup fails—
Robust resume tales in execution's trails!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch archon/thread-1043f57f

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@Wirasm
Copy link
Copy Markdown
Collaborator Author

Wirasm commented May 21, 2026

Review: NEEDS FIXES (4 items)

The core CLI fix is correct — discoveryCwd threading is well-typed, the ?? cwd fallback is sound, and the catch blocks log structured context properly. However the YAML changes introduce a regression that will break archon-refactor-safely for all users, and there are two silent-fallback gaps in the error handling.


C1 — CRITICAL: archon-refactor-safely.yamldenied_tools blocks Write but the prompt now requires it

Files: .archon/workflows/defaults/archon-refactor-safely.yaml:96, :164

The PR removes the persist-impact and persist-plan bash bridge nodes and changes the analyze-impact and plan-refactor prompts from:

"Produce your complete impact analysis below (do NOT attempt to write files — your output will be captured automatically)"

…to:

"Write a thorough impact analysis to $ARTIFACTS_DIR/impact-analysis.md"

Both nodes retain denied_tools: [Write, Edit, Bash]. The SDK will block Write. The files are never created. Every downstream node (execute-refactor, verify-behavior, create-pr) that reads $ARTIFACTS_DIR/impact-analysis.md or $ARTIFACTS_DIR/refactor-plan.md will fail.

Two valid fixes:

  • Option A: Remove Write from denied_tools on both nodes (keep new prompts)
  • Option B: Revert the prompt changes + restore the bash bridge nodes (keep old prompts)

H1 — HIGH: Silent null return from getCodebase — no log, no user signal

File: packages/cli/src/commands/workflow.ts (three sites: resume, approve, reject)

const codebase = await codebaseDb.getCodebase(run.codebase_id);
if (codebase) discoveryCwd = codebase.default_cwd;
// ← no else branch

When the codebase record has been deleted, getCodebase returns null (not a throw). discoveryCwd stays undefined, the fallback to working_path activates silently, and the command fails with "Workflow not found" — the same error as before the fix, with no indication that the codebase lookup was involved.

CLAUDE.md: "Document fallback behavior with a comment when a fallback is intentional and safe; otherwise throw."

Fix at all three sites:

if (codebase) {
  discoveryCwd = codebase.default_cwd;
} else {
  getLog().warn(
    { runId, codebaseId: run.codebase_id },
    'cli.workflow_<command>_codebase_not_found',
  );
}

H2 — HIGH: discoveryCwd path not surfaced when it differs from working_path

File: packages/cli/src/commands/workflow.ts (resume/approve/reject preamble)

The console prints Path: /tmp/worktree-... (the working_path). But when discoveryCwd is set, loadWorkflows searches a different path. If codebase.default_cwd doesn't exist on disk, the error message points the user at the wrong path to debug.

Fix (one line per command, after console.log(\Path: ...`)`):

if (discoveryCwd) console.log(`Discovery path: ${discoveryCwd}`);

I1 — IMPORTANT: workflowRejectCommand has zero new tests for the fix

File: packages/cli/src/commands/workflow.test.ts

The PR adds 2 tests for resume and 1 for approve but zero for reject, despite the PR description explicitly calling it a fixed path. The fix is structurally identical in all three commands; the test gap means a future regression in the reject path would go undetected.

Minimum needed:

  • should discover workflows from codebase.default_cwd, not working_path (parallel to the approve test)
  • should warn and fall back when getCodebase throws (parallel to the resume fallback test)

S1 — SUGGESTION: Extract resolveDiscoveryCwd helper (CLAUDE.md rule-of-three threshold met)

File: packages/cli/src/commands/workflow.ts

The 13-line if (codebaseId) { try { getCodebase } catch { log.warn } } block appears verbatim in resume, approve, and reject. CLAUDE.md: "Extract shared utilities only after the same pattern appears at least three times and has stabilized." It has.

async function resolveDiscoveryCwd(
  codebaseId: string | null | undefined,
  runId: string,
  command: 'resume' | 'approve' | 'reject',
): Promise<string | undefined> {
  if (!codebaseId) return undefined;
  try {
    const codebase = await codebaseDb.getCodebase(codebaseId);
    return codebase?.default_cwd;
  } catch (error) {
    const err = error as Error;
    getLog().warn(
      { err, errorType: err.constructor.name, runId, codebaseId },
      `cli.workflow_${command}_codebase_lookup_failed`,
    );
    return undefined;
  }
}

Each call site collapses to a single line. Preserves per-command event names that tests assert on.


Minor

M1: CHANGELOG.md — user-visible bug fix warrants a ### Fixed entry under [Unreleased].

M2: Test name should fall through to auto-registration when getCodebase throws — "auto-registration" is stale terminology; the test now covers the resume-layer discoveryCwd fallback. Suggested: should warn and fall back to working_path when getCodebase throws during resume.


What's correct

  • WorkflowRunOptions.discoveryCwd?: string — correctly typed and optional
  • loadWorkflows(options.discoveryCwd ?? cwd) — correct fallback for fresh runs
  • All catch blocks include structured context (err, errorType, runId, codebaseId)
  • getCodebase throw → warn severity is correct (fallback is survivable)
  • Outer try/catch in all three commands produces actionable messages with retry commands
  • findResumableRun, executor, web/chat orchestrator, DB schema, working_path storage all correctly untouched per scope
  • No any types; bundled defaults correctly regenerated

@Wirasm
Copy link
Copy Markdown
Collaborator Author

Wirasm commented May 21, 2026

Comprehensive PR Review

PR: #1743 — fix(cli): use source checkout cwd for workflow discovery on resume/approve/reject
Reviewed by: 5 specialized agents (code-review, error-handling, test-coverage, comment-quality, docs-impact)
Date: 2026-05-21


Summary

The core fix is correct and minimal. discoveryCwd is threaded cleanly through workflowRunCommand, and all three re-entry commands (resume/approve/reject) apply an identical warn-and-continue pattern with well-structured log context. No type safety, YAGNI, or execFileAsync rule failures. All new error handlers pass the silent-failure audit.

Verdict: REQUEST_CHANGES — one blocker (missing test for the reject path), then ready to merge.

Severity Count
🟠 HIGH 1
🟡 MEDIUM 0
🟢 LOW 6

🟠 High Issue (Blocker)

Missing discoveryCwd regression test for workflowRejectCommand

📍 packages/cli/src/commands/workflow.test.tsdescribe('workflowRejectCommand')

workflowRejectCommand received the identical discoveryCwd fix as resume and approve (Deviation 1 in scope). workflowResumeCommand got 2 regression tests; workflowApproveCommand got 1. workflowRejectCommand got 0. A future refactor that drops the discoveryCwd assignment from the reject path would silently reintroduce the exact bug this PR fixes — and CI would not catch it.

Three agents flagged this independently (code-review, error-handling, test-coverage).

View recommended tests (2)
it('should discover workflows from codebase.default_cwd on reject-resume, not working_path', async () => {
  // Regression for #1663: reject with on_reject configured re-invokes
  // workflowRunCommand. Discovery must use the source repo, not the worktree.
  const workflowDb = await import('@archon/core/db/workflows');
  const codebaseDb = await import('@archon/core/db/codebases');
  const workflowDiscovery = await import('@archon/workflows/workflow-discovery');

  const runData = {
    id: 'run-reject-1663',
    workflow_name: 'my-approval-workflow',
    status: 'paused',
    user_message: 'go',
    working_path: '/tmp/worktree-without-yaml',
    codebase_id: 'cb-with-yaml',
    metadata: {
      approval: {
        type: 'approval',
        nodeId: 'gate',
        message: 'Approve?',
        onRejectPrompt: 'Fix: $REJECTION_REASON',
        onRejectMaxAttempts: 3,
      },
      rejection_count: 0,
    },
  };
  (workflowDb.getWorkflowRun as ReturnType<typeof mock>).mockResolvedValueOnce(runData);
  (workflowDb.updateWorkflowRun as ReturnType<typeof mock>).mockResolvedValueOnce(undefined);

  (codebaseDb.getCodebase as ReturnType<typeof mock>).mockResolvedValueOnce({
    id: 'cb-with-yaml',
    name: 'owner/repo',
    default_cwd: '/users/me/source-repo-with-yaml',
  });

  const discoverSpy = workflowDiscovery.discoverWorkflowsWithConfig as ReturnType<typeof mock>;
  discoverSpy.mockClear();
  discoverSpy.mockResolvedValueOnce({ workflows: [], errors: [] });

  try {
    await workflowRejectCommand('run-reject-1663', 'needs work');
  } catch {
    // downstream failure is acceptable
  }

  expect(discoverSpy).toHaveBeenCalledWith(
    '/users/me/source-repo-with-yaml',
    expect.any(Function)
  );
});

it('should fall back to working_path for discovery on reject when codebase_id is missing', async () => {
  const workflowDb = await import('@archon/core/db/workflows');
  const workflowDiscovery = await import('@archon/workflows/workflow-discovery');

  const runData = {
    id: 'run-reject-no-codebase',
    workflow_name: 'legacy',
    status: 'paused',
    user_message: 'go',
    working_path: '/tmp/old-worktree',
    codebase_id: null,
    metadata: {
      approval: {
        type: 'approval',
        nodeId: 'gate',
        message: 'Approve?',
        onRejectPrompt: 'Fix: $REJECTION_REASON',
        onRejectMaxAttempts: 3,
      },
      rejection_count: 0,
    },
  };
  (workflowDb.getWorkflowRun as ReturnType<typeof mock>).mockResolvedValueOnce(runData);
  (workflowDb.updateWorkflowRun as ReturnType<typeof mock>).mockResolvedValueOnce(undefined);

  const discoverSpy = workflowDiscovery.discoverWorkflowsWithConfig as ReturnType<typeof mock>;
  discoverSpy.mockClear();
  discoverSpy.mockResolvedValueOnce({ workflows: [], errors: [] });

  try {
    await workflowRejectCommand('run-reject-no-codebase', 'bad');
  } catch {
    // downstream failure is acceptable
  }

  expect(discoverSpy).toHaveBeenCalledWith('/tmp/old-worktree', expect.any(Function));
});

🟢 Low Issues (Non-blocking)

View 6 low-priority suggestions

| C1 | discoveryCwd JSDoc says "Resume/approve" — reject is now also a caller | workflow.ts:67-70 | Drop the caller enumeration; the "why" sentence is self-sufficient |
| C2 | codebaseId inline comment says "resume/approve" — stale since this PR | workflow.ts:65 | Update to: // Skips path-based codebase lookup when resume/approve/reject already resolved it |
| C3 | (Pre-existing) workflowRejectCommand JSDoc describes cancel-only, misses auto-resume branch | workflow.ts:1143-1145 | Out of scope; note for future cleanup |
| C4 | Identical codebase-lookup block 3× — at Rule of Three threshold | workflow.ts:1026, 1109, 1187 | Leave as-is (pattern hasn't stabilized); extract if a 4th caller emerges |
| C5 | No getCodebase throws error-path test for approve/reject (resume is already covered) | workflow.test.ts | Leave for follow-up — resume test proves the shared pattern |
| C6 | Redundant getCodebase DB query (once in caller, once in workflowRunCommand) | workflow.ts:1028-1030 | Leave as-is (YAGNI — SQLite/sub-ms, single-developer tool) |


What's Good

  • Minimal diff: The one-line change in workflowRunCommand (options.discoveryCwd ?? cwd) is exactly right — scoped to workflow discovery only, not execution path.
  • Correct fallback chain: discoveryCwd stays undefined when codebase_id is null or getCodebase returns null, falling back to working_path and preserving legacy behavior.
  • Error handling quality: All three new catch blocks use warn (correct severity for soft degradation) with complete log context including errorType — actually an improvement over the pre-existing conversation-lookup blocks above them.
  • Reject extension is justified: Applying the fix to workflowRejectCommand is correct — it is a true sibling code path with identical root cause.
  • Test event name update: The existing test was correctly updated from cli.codebase_id_lookup_failed to cli.workflow_resume_codebase_lookup_failed, preserving test intent.
  • No docs impact: All changes are internal; user-facing CLI semantics unchanged; bash-bridge nodes were never documented.
  • Type safety clean: discoveryCwd?: string properly typed; no any usage; all CLAUDE.md type rules pass.

Reviewed by Archon prp-review-agents workflow
Artifacts: ~/.archon/workspaces/coleam00/Archon/artifacts/runs/7fa6face6a2a264063f64c5e94f3fe67/review/

- C1: Remove Write from denied_tools on analyze-impact and plan-refactor nodes
  in archon-refactor-safely.yaml — prompts write to $ARTIFACTS_DIR/*.md
- H1: Add else branch with warn log when codebase record not found (null return)
  at all three discoveryCwd sites (resume/approve/reject)
- H2: Log discovery path when discoveryCwd is set so the searched path is
  visible to users debugging workflow-not-found errors
- I1: Add two regression tests for workflowRejectCommand discoveryCwd path
  (codebase found and fallback-when-null), mirroring approve/resume parity
- Fix mock pollution: remove duplicate getWorkflowRun mockResolvedValueOnce
  in "throws when on_reject configured but working_path is null" test whose
  extra queued value leaked into subsequent tests
- L3: Drop caller enumeration from discoveryCwd JSDoc; keep only the why
- L4: Update codebaseId inline comment to include reject as a caller
- L6: Fix workflowRejectCommand JSDoc to describe the auto-resume branch
- M1: Add CHANGELOG entry for the #1663 fix under [Unreleased]
- M2: Rename stale test name "fall through to auto-registration" to accurately
  describe the warn-and-fallback behavior on getCodebase failure
- Regenerate bundled-defaults.generated.ts after YAML changes
@Wirasm
Copy link
Copy Markdown
Collaborator Author

Wirasm commented May 21, 2026

⚡ Self-Fix Report (Aggressive)

Status: COMPLETE
Pushed: ✅ Changes pushed to archon/thread-1043f57f
Commit: 94c1a6ac
Philosophy: Fix everything unless clearly a new concern


Fixes Applied (9 total)

Severity Count
🔴 CRITICAL 1
🟠 HIGH 3
🟡 LOW 3
🟢 MINOR/CLEANUP 2
View all fixes
  • CRITICAL: archon-refactor-safely.yaml denied_tools blocks Write — Removed Write from denied_tools on analyze-impact and plan-refactor nodes; regenerated bundled defaults. Without this fix, the workflow would silently fail to create its artifact files on every run.
  • HIGH: Silent null return from getCodebase (workflow.ts:1030,1113,1191) — Added else warn log at all three sites so a deleted codebase record is observable, not silent.
  • HIGH: discoveryCwd not visible in output (workflow.ts) — Added if (discoveryCwd) console.log('Discovery path: ...') after each codebase lookup; the searched path is now visible when debugging "Workflow not found" errors.
  • HIGH: Missing reject regression tests (workflow.test.ts) — Added two tests: codebase-found path asserts default_cwd is used for discovery; fallback path asserts working_path is used when codebase_id is null. Mirrors resume/approve coverage.
  • LOW: discoveryCwd JSDoc names only "Resume/approve" (workflow.ts:66-71) — Dropped caller enumeration entirely; "why" sentence now durable as more callers are added.
  • LOW: codebaseId inline comment stale (workflow.ts:65) — Updated to // Skips path-based codebase lookup when resume/approve/reject already resolved it.
  • LOW: workflowRejectCommand JSDoc describes cancel-only path (workflow.ts:1143) — Updated to describe both branches (auto-resume when on_reject prompt present, otherwise cancel).
  • MINOR: CHANGELOG missing ### Fixed entry — Added under [Unreleased] referencing workflow approve/resume fails for project-scoped workflows with worktree.enabled: false #1663 and fix(cli): use source checkout cwd for workflow discovery on resume/approve/reject #1743.
  • MINOR: Stale test name "fall through to auto-registration" (workflow.test.ts:1821) — Renamed to should warn and fall back to working_path when getCodebase throws during resume.

Bonus fix: Removed a duplicate getWorkflowRun.mockResolvedValueOnce in the existing throws when on_reject configured but working_path is null test. The extra mock was never consumed and would have leaked into any tests added after the reject describe block (it was causing the new reject regression tests to fail, and would have caused flaky failures in the workflowRunCommand — progress rendering suite).


Tests Added

  • workflowRejectCommand > should discover workflows from codebase.default_cwd on reject-resume, not working_path
  • workflowRejectCommand > should fall back to working_path for discovery on reject when codebase_id is missing

Docs Updated

  • CHANGELOG.md### Fixed entry under [Unreleased]

Skipped

(none — all findings addressed)


Suggested Follow-up Issues

  1. Extract resolveDiscoveryCwd helper — At Rule of Three threshold but hasn't stabilized yet; extract when a 4th caller appears.
  2. Add getCodebase throws tests for approve/reject error paths — Resume is covered; low-risk gap worth closing in a follow-up.

Validation

✅ Type check | ✅ Lint | ✅ Tests (99 passed across all packages) | ✅ Bundled defaults up to date


Self-fix · aggressive mode · fixes pushed to archon/thread-1043f57f

@Wirasm Wirasm marked this pull request as ready for review May 22, 2026 10:37
@Wirasm Wirasm merged commit c2ba1cf into dev May 22, 2026
4 checks passed
@Wirasm Wirasm deleted the archon/thread-1043f57f branch May 22, 2026 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

workflow approve/resume fails for project-scoped workflows with worktree.enabled: false

1 participant