fix(workflows): apply the missing-specification guard to the bundled default - #2500
Conversation
…default #2499 fixed bridge-artifacts in the experimental copy only. The bundled archon-fix-github-issue.yaml — the one users actually get — carried the identical warn-and-continue branch, so the same refusal-reads-as-success path was still shipped. Caught in review of #2499. Also clear .pr-number/.pr-url at the top of capture-pr-number in the experimental workflow. pr-exists treats the file's presence as authoritative, so the write must be the product of the validation that just ran, not of an earlier attempt. The bundled workflow's review tail does not need the pr-exists gate: its synthesize uses trigger_rule: one_success, so a run that dies before create-pr leaves every reviewer skipped, zero successes, and the tail skips on its own.
📝 WalkthroughWalkthroughThe workflows now fail when required investigation artifacts are missing and remove stale pull request metadata before recording current branch information. ChangesWorkflow artifact correctness
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.archon/workflows/experimental/archon-fix-github-issue-experimental.yaml:
- Around line 526-530: Move the .pr-number and .pr-url removal into a dedicated
always-run node that executes before create-pr, including when upstream
validation fails or is skipped. Update the pr-exists condition to require
evidence that the current attempt successfully reset or captured the PR
artifacts, rather than relying on file presence alone; ensure cleanup failure
prevents stale artifacts from being treated as current.
🪄 Autofix
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 Plus
Run ID: 986b5fd4-f11e-483b-8ed5-8eb23f1f716a
⛔ Files ignored due to path filters (1)
packages/workflows/src/defaults/bundled-defaults.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (2)
.archon/workflows/defaults/archon-fix-github-issue.yaml.archon/workflows/experimental/archon-fix-github-issue-experimental.yaml
| # Clear first so .pr-number reflects THIS validation and nothing earlier. | ||
| # pr-exists treats the file's presence as authoritative, so a stale one | ||
| # left by a prior attempt would send the review tail at a PR that is no | ||
| # longer there. | ||
| rm -f "$ARTIFACTS_DIR/.pr-number" "$ARTIFACTS_DIR/.pr-url" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Run stale-PR cleanup on every failure path.
Because capture-pr-number depends on create-pr at Line 540 without an all_done trigger, a failed or skipped create-pr skips this cleanup. In .archon/workflows/experimental/archon-fix-github-issue-experimental.yaml, Lines 555-563 then treat an old .pr-number file as current, so synthesize can run against a stale PR. An unchecked rm -f also does not prove that both files were removed.
Move cleanup to an always-run node before create-pr, and make pr-exists require a successful current-attempt reset or capture instead of file presence alone. This preserves the supplied workflow contract that .pr-number represents the current attempt.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.archon/workflows/experimental/archon-fix-github-issue-experimental.yaml
around lines 526 - 530, Move the .pr-number and .pr-url removal into a dedicated
always-run node that executes before create-pr, including when upstream
validation fails or is skipped. Update the pr-exists condition to require
evidence that the current attempt successfully reset or captured the PR
artifacts, rather than relying on file presence alone; ensure cleanup failure
prevents stale artifacts from being treated as current.
Summary
bridge-artifactswarn-and-continue branch in the experimental copy only. The bundledarchon-fix-github-issue.yaml— the workflow users actually get — carried the identicalecho "WARNING: … implement may fail"+ exit 0, so the refusal-reads-as-success path was still shipped.investigate/planproduce no artifact, the bundled workflow continues intoimplementwith nothing to implement, and unlike the experimental copy it has noassert-implementedbackstop — it proceeds tovalidateandcreate-pr.capture-pr-numbernow clears.pr-number/.pr-urlbefore validating sopr-existsreads the current attempt's result.pr-existsgate — itssynthesizeusestrigger_rule: one_success, so a run that dies beforecreate-prleaves every reviewer skipped, zero successes, and the tail skips on its own. Only the experimental copy (which usesall_done) needed that gate. Adding anassert-implementedbackstop to the bundled workflow is left to Epic: bundled defaults overhaul — curated 8-workflow SDLC set, 36→13 commands, code/agent separation #2123.UX Journey
Before
After
Architecture Diagram
Before
After
Connection inventory:
.pr-numbernow reflects only the current validationLabel Snapshot
risk: lowsize: XSworkflowsworkflows:defaultsChange Metadata
bugworkflowsLinked Issue
Validation Evidence (required)
.archon/workflows/defaults/archon-fix-github-issue.yaml:163-165— byte-identical warn branch.type-check/test— only the regenerated bundle is TypeScript, andcheck:bundledverifies it in CI.Security Impact (required)
No— narrows what a run will do.NoNoNo(therm -ftargets two files the same node writes, inside$ARTIFACTS_DIR)Compatibility / Migration
Yesfor any run producing a specification; a run producing none now fails atbridge-artifactsrather than continuing.NoNoHuman Verification (required)
one_successvsall_done) before deciding thepr-existsgate was not needed there; confirmed the bundled copy has noassert-implemented.trigger_rule: one_successonbridge-artifactsis unchanged, so the plan-path (non-bug issues,investigateskipped) still bridgesplan.md→investigation.md.68b832f4); the changed lines are identical.Side Effects / Blast Radius (required)
archon-fix-github-issue(bundled, ships to all users) andcapture-pr-numberin the experimental copy.$ARTIFACTS_DIRnow hard-fails instead of continuing. Intended.bun run cli validate workflowscovers all 66.Rollback Plan (required)
git revert <sha>thenbun run generate:bundled.bridge-artifactswith a populated$ARTIFACTS_DIR.Risks and Mitigations
rm -fruns before thegh pr viewcheck, so a transientghfailure now leaves no.pr-numberwhere a stale one previously existed.capture-pr-numberalready exits 1 in that case, so the file's absence and the node's failure agree instead of contradicting.Summary by CodeRabbit