fix(workflows,commands): preserve repo identity for issue URLs; unblock .archon fixes - #2417
Conversation
…ck .archon fixes **#2412 — a cross-repo issue URL read the wrong repo.** `extract-issue-number` was instructed to emit "ONLY the bare number", and `fetch-issue` ran `gh issue view "$ISSUE_NUM"`, which resolves against the CURRENT checkout. So a run started with https://github.com/other/repo/issues/456 silently investigated this repo's #456. Silently wrong, not an error. It now emits structured `{issue_number, repo}` — `repo` empty unless the request actually named one, never guessed — and `fetch-issue` passes `--repo` when present. Fixed in the bundled `archon-fix-github-issue` as well as the experimental copy; the bundled one ships this bug to every user. The rewrite also removes the pre-existing double-quoted-substitution warning on that node. **The .archon rule was absolute and made a whole directory unfixable.** #2358 added "modifications under `.archon/` are never yours to commit" to stop runs sweeping the operator's copied-in workflow edits into target-repo PRs. Correct purpose, wrong scope: an issue whose fix IS a workflow, command or script had no path forward. A run on #2412 hit exactly that and blocked, reporting "contradictory instructions" and asking for confirmation — the right call, and the reason this fix is hand-written rather than generated. The rule now distinguishes by intent rather than path: a file your plan names is your work; every other dirty `.archon/` file is not.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR updates issue-fixing guidance and both GitHub issue workflows. Planned ChangesIssue-fixing workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant IssueRequest
participant IssueExtractor
participant FetchIssue
participant GitHubCLI
IssueRequest->>IssueExtractor: issue reference
IssueExtractor->>FetchIssue: issue_number and optional repo
FetchIssue->>FetchIssue: validate issue_number
FetchIssue->>GitHubCLI: gh issue view with optional --repo
GitHubCLI-->>FetchIssue: issue details
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/commands/defaults/archon-fix-issue.md:
- Around line 23-33: Update the `.archon/` file-handling guidance to require
recording each planned file’s baseline diff before editing, preserving those
pre-existing hunks, and staging only implementation changes. Before committing,
inspect `git diff --cached` in addition to the staged file list; when changes
overlap, retain the baseline content and commit only the agent’s patch.
In @.archon/workflows/defaults/archon-fix-github-issue.yaml:
- Around line 35-43: Define issue-reference precedence in the issue-resolution
instructions so each reference remains an atomic issue_number/repo pair; never
combine the number from one reference with the repository from another. Update
.archon/workflows/defaults/archon-fix-github-issue.yaml lines 35-43 and
.archon/workflows/experimental/archon-fix-github-issue-experimental.yaml lines
129-137 consistently, requiring explicit selection or stopping for clarification
when multiple complete references are present.
🪄 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 Plus
Run ID: 6ab1aabf-af35-41ac-896f-274867992e3e
⛔ Files ignored due to path filters (1)
packages/workflows/src/defaults/bundled-defaults.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (3)
.archon/commands/defaults/archon-fix-issue.md.archon/workflows/defaults/archon-fix-github-issue.yaml.archon/workflows/experimental/archon-fix-github-issue-experimental.yaml
…efs atomic Two review findings on this PR, both correct holes in the fix itself. **A named .archon file is not a blank cheque for that file.** The rule said a file your plan names is your work, and the guard checks staged PATHS — neither sees inside a file that already carries copied-in edits from before the run. Staging it whole would commit those too. Now: record `git diff -- <file>` as a baseline before editing, stage only your own hunks with `git add -p`, and stop rather than commit if they cannot be separated. **An issue reference is an atomic (number, repo) pair.** The rules listed how to find a number and how to find a repo as separate steps, so a request carrying both a bare `#123` and a cross-repo URL could take the number from one and the repo from the other — fetching an issue nobody asked for. Each recognised form now yields one complete pair, and several DIFFERENT pairs is an explicit failure naming all of them rather than a silent pick. Both workflows updated; both now validate with zero warnings.
|
Both findings were correct — holes in the fix itself, not in the issue. Pushed 1. A named
|
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/commands/defaults/archon-fix-issue.md:
- Around line 44-51: Update the baseline workflow in the planned-file staging
instructions to capture each file’s complete pre-existing changes, including
staged changes, using a unique baseline file per planned file rather than the
shared /tmp/archon-baseline.diff. Before committing, compare each file’s cached
diff against its corresponding baseline, stage only the agent’s hunks, and stop
when pre-existing staged changes cannot be separated.
🪄 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 Plus
Run ID: 84321229-7069-45cf-b6e6-91c796392013
⛔ Files ignored due to path filters (1)
packages/workflows/src/defaults/bundled-defaults.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (3)
.archon/commands/defaults/archon-fix-issue.md.archon/workflows/defaults/archon-fix-github-issue.yaml.archon/workflows/experimental/archon-fix-github-issue-experimental.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- .archon/workflows/experimental/archon-fix-github-issue-experimental.yaml
- .archon/workflows/defaults/archon-fix-github-issue.yaml
| ```bash | ||
| git diff -- <the-planned-file> > /tmp/archon-baseline.diff # empty if clean | ||
| ``` | ||
|
|
||
| After editing, stage **only your own hunks** — `git add -p <file>` — and reject any | ||
| hunk that also appears in the baseline. If the two are entangled such that you cannot | ||
| separate them, stop and say so rather than committing someone else's work under your | ||
| change. That is the same call the 2026-08-03 run made, and it was the right one. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Capture a complete, file-specific baseline before staging.
git diff -- <the-planned-file> compares the worktree with the index. It misses pre-existing changes already staged, which git add -p will not show or remove. Those changes can still enter the commit.
The shared /tmp/archon-baseline.diff path also overwrites the baseline when multiple planned files are processed. Store one baseline per file, include staged changes with git diff HEAD, and verify the final cached diff against each baseline before committing. Stop if pre-existing staged hunks cannot be separated.
🤖 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/commands/defaults/archon-fix-issue.md around lines 44 - 51, Update
the baseline workflow in the planned-file staging instructions to capture each
file’s complete pre-existing changes, including staged changes, using a unique
baseline file per planned file rather than the shared /tmp/archon-baseline.diff.
Before committing, compare each file’s cached diff against its corresponding
baseline, stage only the agent’s hunks, and stop when pre-existing staged
changes cannot be separated.
`git diff -- <file>` compares the worktree against the INDEX, so pre-existing changes that were already staged never appeared in the baseline — and `git add -p` neither shows nor removes staged hunks, so they would have ridden into the commit invisibly. Exactly the leak the baseline was added to prevent. Now `git diff HEAD -- <file>`, which captures staged and unstaged alike, plus an explicit `git restore --staged <file>` before `git add -p` so the only thing that can be staged is what was deliberately picked.
|
Correct, and a real leak in the fix I added. Pushed.
Two changes: git diff HEAD -- <the-planned-file> > /tmp/archon-baseline.diff # staged AND unstagedand, before staging anything of my own: git restore --staged <the-planned-file> # no-op if nothing was staged
git add -p <the-planned-file> # only deliberately picked hunksClearing the index first makes the guarantee structural rather than dependent on the agent noticing something in a diff — the only thing that can be staged afterwards is what was explicitly chosen.
|
…ut a number Two review findings, both real; a third declined. **owner/repo#N was losing its repository — a regression I introduced.** #2417 handled that form (`{issue_number:"709", repo:"owner/repo"}` -> `--repo`); this command dropped it, and my own examples table said so: `owner/repo#88` yielded `repo_url: ""`. So `gh issue view 88` resolved against the current checkout — exactly the bug #2412 exists to fix, reintroduced for the shorthand while fixed for the URL. Adds a separate verbatim `repo` field. Still nothing constructed: a URL goes in repo_url, a shorthand goes in repo, neither is derived from the other. **The numeric gate ran before the URL branch**, so a best-effort miss on issue_number aborted a run whose repo_url alone was a complete, valid argument to `gh issue view`. Moved into the else, where it gates only the number path. DECLINED: adding `additionalProperties: false` to the output_format schemas. `packages/providers/src/shared/structured-output.ts:154` already recursively injects it on every object schema, respecting any pre-existing value. No bundled workflow declares it because the engine owns it; hand-writing it would duplicate engine behaviour in every author's YAML.
1 participant
Closes #2412. Hand-written rather than generated, for a reason worth recording.
Why a run could not do this
An Archon run on #2412 reached
implementand produced nothing. Not a failure to try — it stopped and said:That rule came from #2358, the first PR of this session. It exists to stop runs sweeping the operator's copied-in
.archon/edits into a target-repo PR — a real problem it solved. But it was absolute, so any issue whose fix is a workflow, command or script had no path forward.The implementer behaved exactly right: detected the contradiction, named both sides, refused to guess. Worth noting the engine then recorded
node_completedfor a node that had explicitly reported itself blocked (#2327), and onlyassert-implementedcaught it.1. Repo identity (#2412)
extract-issue-numberwas told to emit "ONLY the bare number".fetch-issuethen ran:gh issue view "$ISSUE_NUM" --json …which resolves against the current checkout. A run started with
https://github.com/other/repo/issues/456silently investigated this repo's #456 — plausible output, wrong issue.Now structured:
repois empty unless the request actually named one — an empty value means "current checkout", the common case, and the prompt says never to guess.fetch-issuepasses--repoonly when present.Fixed in the bundled
archon-fix-github-issuetoo, not just the experimental copy — the bundled one ships this bug to every user. The rewrite also clears the pre-existing double-quoted-substitution warning on that node (#1884).2. The
.archon/ruleNow distinguishes by intent, not path: a file your plan names is your work; every other dirty
.archon/file is the operator's and stays untouched. The original guard against sweeping up unrelated edits is intact — the staged-file check remains — it just no longer makes a directory unfixable.Validation
Both workflows now validate with zero warnings, down from one each.
Not verified
That a cross-repo URL end-to-end now reads the right issue — it needs a run started with another repository's URL, which I have not done. The unit-level change is straightforward but the claim is untested.
Summary by CodeRabbit
New Features
Bug Fixes