Add workflow discovery diagnostics#1975
Conversation
Workflow re-entry could silently fall back to worktree discovery when a stored codebase lookup failed, making project workflows disappear from resume/approve/reject paths. Changes: - Log the effective workflow discovery root and bundled/global/project counts for every non-JSON, non-quiet workflow run - Fail fast when codebase-backed resume/approve/reject re-entry cannot load the stored codebase - Add tests for discovery diagnostics, fail-fast lookup handling, and detached child cwd preservation Fixes #1959
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds discovery diagnostic logging to workflow runs showing bundled/global/project source counts, centralizes codebase lookup error handling across resume/approve/reject commands with fail-loudly behavior, and expands test coverage for discovery state and codebase lookup failures. ChangesWorkflow Discovery Diagnostics and Error Handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 Comprehensive PR ReviewPR: #1975 SummaryPR #1975 is narrowly scoped and generally aligns with the requested workflow discovery diagnostics and fail-fast codebase lookup behavior. The available review artifacts found no critical or high-severity issues, and the source/test changes are mostly behavior-focused. The main concern is a medium-severity approve/reject recovery path where codebase lookup can fail after a decision has already been persisted, leaving the user with incomplete recovery instructions. Verdict:
🔴 Critical IssuesNone. 🟠 High IssuesNone. 🟡 Medium Issues (Needs Decision)Approve/Reject Codebase Lookup Errors Hide That The Decision Was Already Recorded📍
Options: Fix now | Create issue | Skip View detailsRecommended fix: move try {
const discoveryCwd = result.codebaseId
? await resolveDiscoveryCwdForCodebase(runId, result.codebaseId, 'approve')
: undefined;
await workflowRunCommand(result.workingPath, result.workflowName, result.userMessage ?? '', {
resume: true,
codebaseId: result.codebaseId ?? undefined,
conversationId: platformConversationId,
discoveryCwd,
});
} catch (error) {
const err = error as Error;
throw new Error(
`Approved but failed to resume workflow '${result.workflowName}': ${err.message}\n` +
`The approval was recorded. Run 'bun run cli workflow resume ${runId}' to retry.`
);
}Apply the same shape to the reject
Recommendation: Fix now. Discovery Diagnostic Suppression Is Only Indirectly Covered📍 The new discovery diagnostic is gated by Options: Fix now | Create issue | Skip View detailsRecommended test: it.each([
['json', { json: true, noWorktree: true }],
['quiet', { quiet: true, noWorktree: true }],
])('does not print discovery diagnostic in %s mode', async (_mode, options) => {
const { discoverWorkflowsWithConfig } = await import('@archon/workflows/workflow-discovery');
(discoverWorkflowsWithConfig as ReturnType<typeof mock>).mockResolvedValueOnce({
workflows: [makeTestWorkflowWithSource({ name: 'assist' }, 'project')],
errors: [],
});
try {
await workflowRunCommand('/repo/root', 'assist', 'hello', options);
} catch {
// downstream failure is acceptable; this test only verifies diagnostic suppression
}
expect(consoleSpy).not.toHaveBeenCalledWith(expect.stringContaining('Discovery: root='));
});
Recommendation: Fix now if touching tests in this PR. 🟢 Low IssuesView 1 low-priority suggestion
✅ What's Good
📋 Suggested Follow-up Issues
Next Steps
Reviewed by Archon comprehensive-pr-review workflow |
Fixed:\n- Wrap approve/reject codebase discovery failures in recorded-decision recovery errors\n- Add explicit JSON/quiet discovery diagnostic suppression tests\n- Complete approve/reject codebase lookup failure coverage\n\nTests added:\n- workflow CLI suppression and codebase lookup recovery cases\n\nSkipped:\n- none
Fix Report: PR #1975Date: 2026-06-12T06:40:58Z SummaryAll three review findings were addressed. Approve/reject auto-resume now reports recorded decisions when codebase discovery lookup fails after mutation, and the CLI tests now directly cover diagnostic suppression plus the missing approve/reject lookup failure variants. Nothing was skipped or blocked. Fixes Applied
Tests Added
Docs Updated(none) Skipped Findings(none) Blocked (Could Not Fix)(none) Suggested Follow-up Issues(none) Validation
|
The simplify step refactored three console web files this PR never touched (when-grammar.ts, ModelPickerField.tsx, model-options.ts). Behavior-preserving or not, they are out of scope for a CLI discovery fix — restore them to the merge-base state.
1 participant
Summary
Describe this PR in 2-5 bullets:
archon workflow runcould be hard to diagnose when workflow discovery found only bundled workflows, and resume/approve/reject re-entry could silently fall back to worktree discovery when a stored codebase lookup failed.workflow rundisagree withworkflow listand hide the actionable root cause.UX Journey
Before
After
Architecture Diagram
Before
After
Connection inventory (list every module-to-module edge, mark changes):
workflowRunCommanddiscoverWorkflowsWithConfigworkflowRunCommandDiscovery: root=... workflows=... bundled=... global=... project=...for non-JSON, non-quiet runs.workflowResumeCommandcodebaseDb.getCodebaseworkflowApproveCommandcodebaseDb.getCodebaseworkflowRejectCommandcodebaseDb.getCodebaseon_rejectre-entry.workflow runparent--cwdpreservation was verified and covered by a stronger test.Label Snapshot
risk: lowsize: Scli|testscli:workflowChange Metadata
bugcliLinked Issue
workflow runintermittently sees bundled-only workflows whileworkflow listsees all (discovery divergence) #1959 (diagnostics + fail-fast; issue stays open until the intermittent bundled-only recurrence is confirmed gone)Validation Evidence (required)
Commands and result summary:
bun install bun run type-check bun test packages/cli/src/commands/workflow.test.ts bun run lint --max-warnings 0 bun run format:check bun run validatebun run validatepassed with 5126 passed, 0 failed, 1 skipped; focused workflow CLI test file passed with 130 tests.Security Impact (required)
Yes/No): NoYes/No): NoYes/No): NoYes/No): NoYes, describe risk and mitigation: not applicable.Compatibility / Migration
Yes/No): YesYes/No): NoYes/No): NoHuman Verification (required)
What was personally validated beyond CI:
--cwd.null; codebase lookup throwing; discovery source breakdown includes bundled/global/project counts; quiet/JSON output remains protected from the new human diagnostic.Side Effects / Blast Radius (required)
Rollback Plan (required)
efc585d6or revert the two changed files,packages/cli/src/commands/workflow.tsandpackages/cli/src/commands/workflow.test.ts.Risks and Mitigations
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
--jsonand--quietmodes.