Repository navigation
feat: implement issue #2050 — dev-lead-retry runs from main but dispatches to the stable-pinned harness: every fix-bot-comment retry has failed since #2017 merged (17 failures in 3 h) - #2083
Conversation
…tches to the stable-pinned harness: every fix-bot-comment retry has failed since #2017 merged (17 failures in 3 h)
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 30 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (3)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds documentation to AGENTS.md regarding the ordering of new client_payload fields from dispatching sweeps to prevent channel skew, and introduces a new BATS test suite (test_dev_lead_retry_channel.bats) to validate the channel-skew guard. The review feedback recommends replacing generic non-zero exit code checks (-ne 0) in the BATS tests with assertions for specific expected exit codes (such as -eq 1) to prevent tests from falsely passing on unexpected errors like syntax or command-not-found issues.
CodeAnt PR Risk: Low Risk
Assessed commit: |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Existing test changed without justification — human attention neededThe If the test really is wrong, a human should make that call, or the fix should carry a |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-09T03:13:13Z. |
|
No description provided. |
|
No description provided. |
Existing test changed without justification — human attention neededThe If the test really is wrong, a human should make that call, or the fix should carry a |
|
No description provided. |
|
This is a CodeRabbit OSS review rate-limit notice. No code findings are present in this comment — it is purely an operational notice that the free review quota has been exhausted and will reset after the specified time window. |
|
No description provided. |
|
No description provided. |
|
No description provided. |
|
No description provided. |
|
CodeRabbit rate-limit notice: this is an operational notice indicating the OSS review limit was reached. The actual code findings are from cubic-dev-ai and have been addressed in commit 5697f22. |
don-petry
added
the
dev-lead:hands-off
Claude
- tests: assert the exact failure status (exit 1) instead of any non-zero, so grep or resolve-step errors cannot pass the negative tests; skip the file locally when yq is missing, but fail in CI. - AGENTS.md: describe the #2017 skew as the pre-backstop failure and note the ordering still matters for target repos pinned to an older channel. - dev-lead-retry.yml: document that one pin covers the whole sweep (per-target-repo resolution tracked in #2086). Test-Change-Justification: review threads on #2083 (gemini-code-assist and cubic) asked for exact exit-status assertions and a yq guard in this PR's own new test file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019K6UsD7r25pAcRjJL3G6Kx
|
dev-lead is withholding action on this item. It is labeled To re-enable automated pickup: remove the |
|
No description provided. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: c7f6133e3b62048adaa627d7d06c4e5d9551b9eb
Review mode: triage-approved (single reviewer)
Summary
Pins the dev-lead retry sweep to the release it dispatches to. dev-lead-retry.yml now sparse-checks-out dev-lead.yml from the default branch, reads its agent_ref, checks it against ^dev-lead/v[0-9]+-(next|ring0|ring1|stable)$, and checks out scripts/ at that channel instead of main. The PR also adds the payload-field ordering rule to AGENTS.md and a 9-case bats suite. Risk is MEDIUM because this changes GitHub Actions workflow behaviour that only runs after merge. The change adds no new secret handling: the checkout token expression and persist-credentials: false match the existing checkout step. The resolved ref goes only into a with: input, never into a run: script, and the regex check rules out injection.
Linked issue analysis
Closes #2050.
- AC1 (main fix): done. The sweep now runs at the channel that
dev-lead.ymlpins. Nothing hard-codesv139, the job fails on an empty or malformed pin, and a bats test checks that the checkout ref comes fromagent_ref. This is exactly the plan in the issue's budget guard. - AC3: done. AGENTS.md now states the ordering rule for new
client_payloadfields, placed next to the existing channel-pin input-forwarding rule. - AC2: not done, which the issue allows. The budget guard marks it optional. AC1 removes the skew within this repo. The remaining cross-repo case (a target repo pinning an older channel) is documented in the workflow comment and AGENTS.md, and tracked in open follow-up #2086.
Findings
No blocking findings.
Verified locally
- I pulled the
channelstep's script out of the workflow and ran it against the livedev-lead.yml. It resolved todev-lead/v139-stable, exited 0, and wrote the expectedGITHUB_OUTPUTline. It also passes shellcheck. - It rejects
mainanddev-lead/v139-stable;x, both with exit 1 and no output. - It accepts quoted values and values followed by a trailing
# comment. - Comment lines in the stub that mention
agent_refstart with#, so thesedanchor skips them.
Prior bot review threads: all resolved. Gemini's exit-status thread and cubic's thread about the yq call failing every test are fixed in c7f6133. The AGENTS.md wording is fixed, and the per-repo pin concern is documented and deferred to #2086.
Test-tamper notices: earlier dev-lead passes were held back for editing tests. The final commit c7f6133 carries a Test-Change-Justification: trailer that cites those review threads. Its edits are confined to the new test file this PR adds, and they make the assertions stricter (-ne 0 became -eq 1, plus a yq guard that fails in CI).
Non-blocking notes
github.event.repository.default_branchmay be empty onscheduleevents. If it is,actions/checkoutfalls back to the triggering ref, which for a schedule is the default branch, so behaviour is unchanged.- The second, root-level checkout deletes
.channel-pin/. That is harmless because the pin has already been read by then. - Bug fixes to
scripts/dev-lead-retry.shonmainnow take effect only after the channel is promoted. The issue accepts this trade-off on purpose.
Secret scan: the MCP run_secret_scanning tool is not available in this run. The gitleaks CI check passed.
CI status
All required checks are green on c7f6133: Lint/bats, Test Dev-Lead Agent/unit, shellcheck, actionlint, CodeQL, SonarCloud (quality gate passed), gitleaks, AgentShield, and the stub/permission guards. The unit job ran all 9 new tests in test_dev_lead_retry_channel.bats (ok 458–466), none skipped. The CANCELLED and SKIPPED entries are runs on superseded commits and conditional jobs, not failures.
Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.
|
No description provided. |
|
…-parsing the parser; check every channel The text-pattern scanner kept missing cases (nine findings on #2085): comments counted as field reads, commas and brackets inside quoted values broke the key count, and a failed tag fetch silently used a stale tag. - Sent side: every function in the sweep (and the libs it sources) whose body POSTs to /dispatches is executed with `gh` stubbed to capture the request; the top-level client_payload keys come from the real JSON via jq. Both the --input JSON form and the -f/-F client_payload[key]= form are captured. A dispatcher that sends nothing for the probe call is a setup error (exit 2), never a pass. - Read side: the intent parser, and the libs it sources, at each channel tag are parsed by bash first (wrapped in a function and printed with declare -f), which drops comments and keeps strings verbatim, before client_payload.<key> reads are collected. - Channels: since #2083 the sweep runs at this repo's pin, so the skew left is a target repo on an older channel (#2086). The check now compares against every dev-lead/v<N>-{next,ring0,ring1,stable} tag. The pinned tag must resolve; a missing other channel is skipped with a warning. In CI an unfetchable tag fails closed instead of falling back to a stale local copy. - Merged main (AGENTS.md conflict with #2083's rewrite of the same rule: kept main's text and added the check's reference). Tests: 13 cases, including one per review finding; 11 of them fail on the previous scanner. Test-Change-Justification: tests/test_retry_payload_fields.bats is new in this PR (#2085) and is rewritten for the reworked check the maintainer asked for. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019K6UsD7r25pAcRjJL3G6Kx
2 participants
Problem
dev-lead-retry runs from main but dispatches to the stable-pinned harness: every fix-bot-comment retry has failed since #2017 merged (17 failures in 3 h)
From the issue:
.github/workflows/dev-lead-retry.ymlchecks out and runsscripts/dev-lead-retry.shfrommain. The runs it dispatches execute the harness at the pinned channel,dev-lead/v139-stable. When a change to the sweep and the matching change to the harness merge together, the sweep starts sending a payload the pinned harness cannot read, and every such run fails until the channel catches up.Risk
Medium — changes GitHub Actions workflow behavior, which is exercised only post-merge; verify via the affected workflow runs.
Test plan
Tests added/updated:
tests/dev-lead/unit/test_dev_lead_retry_channel.bats. Verification:bash scripts/dev-lead-lint.sh(shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.Rollback
Revert this PR. No non-revertible side effects (no tags, migrations, or external state).
Monitoring
Watch the affected workflow run(s) in the Actions tab and this PR's Lint check for regressions.
Closes #2050