Skip to content

ci(pr-pipeline): repin check-key-credit to the probing marker - #12935

Merged
kyle-sexton merged 6 commits into
mainfrom
ci/repin-key-credit-probing
Nov 3, 2026
Merged

kyle-sexton merged 6 commits into
mainfrom
ci/repin-key-credit-probing

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Refs: #12526

Summary

ci-workflows#980 adds a probing marker to check-key-credit. After a credit stop's window ends, the first gate run names itself the probe. Later gates skip the key while its marker is fresh, so one run tries the key instead of every run that reads before the next credit stop lands. #980 asks its direct callers to repin and to upload that marker. This repository has two direct callers: pr-verify.yml (check-intent) and pr-review-main.yml (review-handoffs-credit).

Fix

  • Both callers pin check-key-credit at 09cf6f2 (the merge of feat(claude-memory): dotfile-manager backfill detection beyond chezmoi (yadm, stow, fingerprints) #980, # 09cf6f2 2026-11-03).
  • Right after each gate step, each job now runs Write the key's probing marker, Scan the key's probing marker for credentials and Upload the key's probing marker.
    • The steps copy pr-review-cli.yml's gate job at that SHA: continue-on-error, kept 1 day, and uploaded with the runner's token, so no job permission is added.
    • The write runs only while probing-marker equals the name written out (key-credit-probing-pr-verify.yml-check-intent, key-credit-probing-pr-review-main.yml-review-handoffs; gate.cjs probeName at 09cf6f2). The retention rows claim those exact names.
    • The upload if: takes the (<scan if>) && clean form that check-artifact-retention.mjs requires, not upstream's three flat conjuncts.
  • The comments above each gate now say what changed: the gate makes a second read in recovery, and the probe marker exists.
  • .github/artifact-retention.json has one row per new artifact.
  • A .changes/pr-pipeline fragment, patch bump.

Deviations from the brief, for review:

  1. pr-verify's other ci-workflows actions move to 09cf6f2 too. pr-verify's test every ci-workflows action is pinned to one commit requires one pin, so run-claude-cli and apply-claude-managed-settings move as well.
    • Between bc62beb and 09cf6f2, run-claude-cli/lane.cjs changes only judgeLeg (test(disk-hygiene): mock sys.argv in run_guard_powershell to seal the kill-switch argv seam #974: a token leg that timed out under a rejected rate limit is now a limit stop). cliArgs still passes --permission-prompts none at lane.cjs:404-405, and run-claude-cli's action.yml and apply-claude-managed-settings are unchanged.
    • That adds 09cf6f2 to PERMISSION_PROMPTS_NONE_LAUNCHERS in callers.test.mjs, and replay.mjs RUN_CLAUDE_CLI_PIN follows the pin.
  2. pr-review-main scans with ci-workflows' scan-upload@09cf6f2. Its gate job checks nothing out, and its test forbids ./.base. The retention checker's SCAN_ACTION matches only the local action, so that row says scan: none, like the sibling key-credit-stop-* row. The reason field explains this.

Verification

  • node --test on the pr-verify and pr-review-main workflow tests, probe/parity.test.mjs and resolve-config/callers.test.mjs: 347 of 347 pass.
    • New tests in each workflow test check the step ids, the exact if: strings, that the scan and upload use the same path, retention 1, if-no-files-found: error, continue-on-error, that the steps sit right after the gate, and that they appear in no other job.
    • pr-review-main also has a paired case: the marker is written only for the exact probe name.
  • Eight other suites that read these workflows (check-trusted-trigger, check-upstream-status, log-api-budget, api-budget-coverage, models, merge-holds, workflow-gates, lane-transcripts pull): 768 of 768 pass.
  • scripts/ci-review-lanes.test.sh passes 239 checks and is unchanged: its callers run pr-review-cli at @main and pin no gate. scripts/ci-lanes-pr-hold.test.sh passes 303 checks.
  • scripts/check-artifact-retention.sh: 85 upload steps match (60 families).
  • scripts/check-changelog-fragments.sh --check: 21 fragments valid. --check-required origin/main passes.
  • zizmor reports no findings. actionlint reports only the existing cache-mode key gap.
  • Job timeouts: pr-verify's check-intent timeout counts only the 22-minute judge steps (judges × 22 + 15), so it does not move. review-handoffs-credit sums 6 minutes of step timeouts, under its 10.

Remaining

Work #12526 still needs after this merges, none of it in this PR:

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_015te8CqFTUd8N8Byd5NUvMb

kyle-sexton and others added 2 commits November 3, 2026 15:16
Pin ci-workflows check-key-credit at 09cf6f2 (ci-workflows#980) in
pr-verify.yml and pr-review-main.yml, and add the write, scan and upload
steps for the gate's probing marker right after each gate step, kept one
day, continue-on-error, with the runner's token. Once a credit stop is
past its window, the first gate after it probes the key and later gates
skip while its marker is fresh.

pr-verify pins every ci-workflows action at one SHA (workflow.test.mjs),
so its run-claude-cli and apply-claude-managed-settings move to 09cf6f2
too; lane.cjs still passes --permission-prompts none at lines 404-405,
recorded in callers.test.mjs, and replay.mjs follows the pin.

pr-review-main's gate job checks nothing out, so it scans with
ci-workflows' scan-upload at the same pin; its retention row says scan
none because the checker recognizes only the local action.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_015te8CqFTUd8N8Byd5NUvMb
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 3, 2026 20:22
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T20:25:47.424749Z 2ae4b76 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 1 inline finding(s) on 2ae4b76b4a5b3267caec65d8891c221ae6cdabe9; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment thread .github/workflows/pr-review-main.yml Resolved
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed c042276b10b37692558e61c8b57467c3103d7093: read 4 of 4 changed file(s); 0 finding(s), 0 as inline comments.

Code review: incremental, b05a044 → c042276

No findings. I read every hunk of the 4 files in this push:

  • .github/workflows/pr-review-main.yml
  • .github/workflows/pr-verify.yml
  • .github/scripts/pr-review-main/workflow.test.mjs
  • .github/scripts/pr-verify/workflow.test.mjs

I also read pr-verify.yml L230-L490 at head to check the moved steps against the steps around them.

pr-review-main.yml (fixes my earlier proceed finding):

  • The probe write's if: (L392) now includes needs.trigger.outputs.proceed == 'true'. That is the term the skipped-leg reports (L422, L434) and the review job's CARRIED_PROCEED check (L608) use.
  • proceed is a real trigger output (L105).
  • The test checks the exact if: string and that the fixture's proceed is "true". It adds { proceed: "" } and { proceed: "false" } cases where no marker is written, which cover the kill-switch and refused-trigger paths.
  • The settled thread on L391 stays settled.

pr-verify.yml (author's same-sweep fix):

  • Write, scan and upload now sit right after Check the credential (L338-L350). The write also requires steps.credential.outputs.present == 'true', the same term as Judge the pull request (L463).
  • The credential step requires skip-key != 'true' itself. So a run whose gate skips the key, or that has no key set, no longer claims the probe.
  • The write keeps always(), so a failed step before it still leaves present empty and nothing is written.
  • Moving the steps after Report the skipped key leg changes nothing: that report only runs when skip-key == 'true', and then the write cannot run.
  • The test imports admits (L16). It checks the new order, both if: strings, and four no-write cases.
  • This walks back what I said last round, that pr-verify needed no extra term. The credential term is a real gap the author found, and the fix is correct.

Not verified (author's claims): "Workflow tests 91/91" and the earlier test, checker, zizmor and actionlint results. I ran nothing.

Closing issue: none (the PR says Refs: #12526), so there are no acceptance criteria to check.

Manifests: none changed.

Spend (not a finding): unchanged from last round, at most one small 1-day artifact per probe run. The probe now uploads in fewer runs, so it spends slightly less.

Prior findings / settled threads: both earlier inline findings, on pr-review-main L390 and L391, are fixed at this head. I repeat neither.

I found no embedded instructions in the PR context, settled-threads or prior-findings files.

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed c042276b10b37692558e61c8b57467c3103d7093: read 4 of 4 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12935, incremental from b05a0447 to c042276b

No security issues found. This was an incremental review. I read every hunk of the 4 files the push changed: .github/workflows/pr-review-main.yml, .github/workflows/pr-verify.yml and their two workflow.test.mjs files. All 4 are on the trusted-path list, so for each one I read the whole change and the steps around it.

critical-findings: 0

What changed and what I checked

  • pr-review-main: the probe write now also checks proceed. The if: on Write the key's probing marker gained needs.trigger.outputs.proceed == 'true' (pr-review-main.yml#L392).
    • This only narrows when the probe gets written. It is the fix the settled thread on that line asked for: that thread is resolved, and its lines have not changed since.
    • No step, permission or token request was added. The upload is still gated on the scan's clean == 'true' result.
  • pr-verify: the probe steps moved, and the write checks the credential. The write, scan and upload steps now sit right after Check the credential instead of right after the gate (pr-verify.yml#L360-L387). The write's if: now also requires steps.credential.outputs.present == 'true'.
    • The only steps between the old and new position are Report the skipped key leg, Resolve the verifier's model (./.base/.github/actions/resolve-config, from the trusted base checkout) and Check the credential. That last step outputs only present=true or present=false, never the secret's value (pr-verify.yml#L338-L350).
    • The steps still run before runner isolation and before any judge step, so no pull request code runs ahead of them.
    • The scan still uses ./.base/.github/actions/scan-upload. The write still echoes only $GITHUB_RUN_ID. The step contents are unchanged; only their position and the write's if: moved.
  • The test changes only pin the new if: strings and step order, and add paired cases where no probe is written. They are not security-relevant.
  • Impact of the probe marker. At most, a probe marker makes a later gate skip the key leg and hand the work to the subscription leg. That leaks no secret, compromises nothing and adds no charge. The change only narrows when the marker is written.

Settled and prior items

  • Both settled threads on pr-review-main's probe-write if: stay settled. This head carries the fixes they asked for.
  • The prior summary from this lane, at b05a0447, had no findings, so nothing is restated.
  • Embedded text: none found in the PR context, settled-threads or prior-findings files.

kyle-sexton and others added 2 commits November 3, 2026 15:49
The probing marker in review-handoffs-credit now needs the review job's
trigger terms (review != 'false', judged != 'true'), so a run whose
trigger found no lane source or a judged head never writes a probe that
holds later gates off the key while no key leg tries it. The test pins
the exact if: and adds paired judged and no-review cases.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_015te8CqFTUd8N8Byd5NUvMb
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 3, 2026
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

The pr-verify verifier failed head ca7392a7ccd67d31febec5723df6215d04f534bd. What it wrote, as data:

What it wrote (27 lines)
Summary: PR #12935 delivers part of #12526 (a `refs` link). It repins check-key-credit to 09cf6f2
  and adds the steps that write, scan and upload the probing marker in pr-verify `check-intent` and
  in pr-review-main `review-handoffs-credit`. Those steps follow the decision recorded in the
  issue's latest measurement comment. The code is coherent and the tests are specific. test-node and
  every test-bash job passed on the judged head
  (https://github.com/melodic-software/claude-code-plugins/actions/runs/41716228743/job/126197786302).
  It fails on one point: the rules require a `refs` PR body to name the work left for later, and
  this body names none. It lists what the PR delivers and links #12526 only as "the key-credit gate
  work". It never says what remains: the re-measurement of re-probes and jobs per run, the window
  length review, and item 3, which stays with #6958.
- PR body (Refs: #12526): add a section naming the work this PR leaves for later. That is:
  re-measure key calls per window opening and jobs per run after this merges (done-when item 2 asks
  to compare the job count before and after), revisit the 1-hour window once the burst is gone, and
  leave item 3 (rotating the key) with #6958. A `refs` delivery needs that list to pass.
- .github/workflows/pr-verify.yml:336 Moving all ci-workflows actions to the one pin brings in
  run-claude-cli's #974 `judgeLeg` change (a token leg that timed out under a rejected rate limit
  becomes a limit stop). #12526 does not ask for this, but the body explains it: the single-pin test
  forces it, and the change is described. Flagged for a human; it does not block.
- .github/artifact-retention.json:396 The `key-credit-probing-pr-review-main.yml-review-handoffs`
  row says `scan: none`, but the workflow does scan the artifact with ci-workflows'
  scan-upload@09cf6f2. The row's reason field explains this, and it matches the sibling credit-stop
  row. The retention checker cannot see the scan, so a later change could drop it without any check
  failing.
- .github/workflows/pr-review-main.yml:416 Following the review finding, the write step's `if` now
  also includes the review job's same-attempt start terms (`review != 'false' && judged != 'true'`),
  and the test pins it with paired no-write cases. pr-verify's write has no matching term because
  its gate step already runs only when `steps.agree` succeeds.

This pull request leaves the merge queue, and ci-status holds it out while it carries pr-verify-failed. A pr-verify pass at a new commit removes the label.

@github-actions github-actions Bot added the pr-verify-failed pr-verify failed this head; a pass at a new commit removes it. label Nov 3, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 3, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 1 inline finding(s) on b05a0447e8ffa612db378695b39f7d8009614f36; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment thread .github/workflows/pr-review-main.yml Resolved
@github-actions github-actions Bot removed the pr-verify-failed pr-verify failed this head; a pass at a new commit removes it. label Nov 3, 2026
pr-review-main: the probing marker also needs the trigger's proceed ==
'true', the review job's remaining trigger term, so a kill-switch or
gated-off run never writes a probe that holds later gates off the key.

pr-verify: the gate runs whenever the resolver agrees, but the first
judge step starts only when steps.credential.outputs.present == 'true'.
An empty PR_VERIFY_CLAUDE_API_KEY would have written a probe with no
judge trying the key. The probe steps now run right after the
credential check and require that term (an if: cannot read secrets).

Both tests pin the exact if: and add paired no-write cases.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_015te8CqFTUd8N8Byd5NUvMb
@kyle-sexton
kyle-sexton enabled auto-merge November 3, 2026 21:41
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit 7a68f95 Nov 3, 2026
67 of 68 checks passed
@kyle-sexton
kyle-sexton deleted the ci/repin-key-credit-probing branch November 3, 2026 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant