Repository navigation
feat: implement issue #1866 — standards-sync rewrites the per-repo channel pin it is told to preserve — PR #1819 would move dev-lead from stable to next in the repo that hosts it - #1889
Conversation
…annel pin it is told to preserve — PR #1819 would move dev-lead from stable to next in the repo that hosts it
|
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 reachedNext included review available in 22 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (11)
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 introduces a same-file channel-pin and comment consistency guard script (channel-pin-consistency.sh) along with supporting test fixtures and a BATS test suite to prevent configuration drift in workflow stubs. The feedback recommends securing temporary file creation in the test suite using mktemp instead of predictable paths with $$, avoiding standalone conditional expressions in the library script to prevent unintended termination under set -e, and tightening test assertions by checking for specific non-zero exit codes rather than generic non-zero statuses.
CodeAnt Nitpicks1 code suggestion1. A missing or unreadable file is treated as no pin, so invalid paths and read failures pass the consistency guard as not applicable.Error handling · |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-09-22T18:34:08Z. |
|
This PR's diff does not touch |
Dev-Lead — review-changes (applied)Changes committed and pushed. Requested items addressed:
|
|
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-09-22T18:40:39Z. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 308a4a132c129b03abf6fd4cf6344bb8fd4ea259
Cascade: triage → deep (triage: haiku 4.5 [sonnet 5] → deep: opus 4.8 [sonnet 5] + duck: o4-mini → audit: fable 5)
Summary
Adds a pure receiving-side channel-pin/comment consistency guard (scripts/lib/channel-pin-consistency.sh) with fixtures and a 19-case BATS suite (all pass locally), plus a one-line comment fix to pr-review-mention.yml aligning its header (v2-stable -> v2-next) to the already-existing uses: pin — exactly the #1819 stale-comment shape the guard exists to catch; LIVE test #19 confirms the file is now consistent. Core control/data flow is correct: no-pin and cross-agent/contrast cases are handled and return N/A, and conflict detection covers comment and agent_ref desync. No security-critical path is touched; safety checks report no hard-stops (CI_WEAKENING/PROMPT_INJECTION false). Downstream impact: none.
Findings
- minor: cpc_stub_conflicts/cpc_assert_consistent treat a missing or unreadable file as 'no pin -> N/A' (returns 0). Harmless in current wiring since the only caller (the LIVE bats test) globs existing .github/workflows/*.yml with nullglob, so a nonexistent path is unreachable; but if this guard is later wired to a manifest/user-supplied path, a typo'd path would silently pass instead of erroring. Confirmed by invoking cpc_assert_consistent on a nonexistent path (exit 0, 'no reusable channel pin — N/A').
- minor: Advisory (gemini, security-high) flags the predictable temp path /tmp/cpc_live.$$ in the LIVE bats test (symlink-attack risk of $$ in /tmp). Real-world severity is low: this is test-only code on ephemeral CI runners, not the shipped library. Recommend mktemp with a create-failure guard as a hygiene improvement, but non-blocking.
- info: Advisory (gemini, low, 6 instances) suggests asserting the exact non-zero exit code ([ "$status" -eq 1 ]) instead of the generic [ "$status" -ne 0 ] so a test cannot pass on an unexpected failure (syntax/command-not-found). Reasonable tightening; the guard functions return exactly 1 on conflict. Style-level, non-blocking.
- info: The template-drift check shows FAILURE, but it is documented as known-red under #1448 ('must not fail every merge until that is resolved') and inspects the petry-projects/repo-template repo's committed stubs via network, which this PR does not modify. Not introduced by this PR and explicitly non-blocking. All correctness-relevant gates (Lint, shellcheck, unit-tests, actionlint, CodeQL, caller-stub-freeze, sync-scope-guard, validate-fixtures, stub-structure) are green; the touched bats suite passes 19/19 locally.
Reviewed by the PR-review cascade (triage: haiku 4.5 [sonnet 5] → deep: opus 4.8 [sonnet 5] + duck: o4-mini → audit: fable 5). Reply if you need a human review.
2 participants
Problem
standards-sync rewrites the per-repo channel pin it is told to preserve — PR #1819 would move dev-lead from stable to next in the repo that hosts it
From the issue: The standards-sync that opens
chore: sync N org-standard workflow stub(s)PRs rewrites the per-repo channel pin — the one field the org standard explicitly designates as repo-owned — and in PR #1819 it moved three stubs fromstabletonext, includingdev-leadin the repository that hostsdev-lead-reusable.yml.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/fixtures/channel-pin/stub-agent-ref-desync.yml,tests/fixtures/channel-pin/stub-aligned.ymltests/fixtures/channel-pin/stub-both-differ.yml,tests/fixtures/channel-pin/stub-comment-mismatch.ymltests/fixtures/channel-pin/stub-contrast.yml,tests/fixtures/channel-pin/stub-no-pin.ymltests/fixtures/channel-pin/stub-other-agent.yml,tests/test_channel_pin_consistency.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 #1866