Skip to content

docs(aggregator): correct dry-run Checks:read credential sizing - #334

Merged
kyle-sexton merged 2 commits into
mainfrom
docs/aggregator-dry-run-header
Aug 5, 2026
Merged

docs(aggregator): correct dry-run Checks:read credential sizing#334
kyle-sexton merged 2 commits into
mainfrom
docs/aggregator-dry-run-header

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Summary

  • Correct the claude-lane-incident-aggregator header and matching comments that overclaimed dry-run reads always 403 without Checks:read.
  • Document the empirical credential-sizing story: public dry-run reads work with the ambient token; private reads typically 404 without an installation token; writes still need the App.

Closes #325

Related

Test plan

  • node --test .github/scripts/claude-lane-incident-aggregator.test.cjs (30/30 pass)
  • Confirmed no remaining no public-repository exemption / dry run 403s wording in the workflow or matching test
  • Skim PR diff: comments/docs only; permissions block and runtime behavior unchanged

Made with Cursor

Public repo check-run reads work with the ambient token; private reads 404 without an installation token rather than 403 without Checks:read.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions github-actions Bot deleted a comment from claude Bot Jul 31, 2026
@github-actions github-actions Bot deleted a comment from claude Bot Jul 31, 2026
@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@github-actions

Copy link
Copy Markdown

Warning

Automated review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

Re-run the job to retry the review. A new push does not re-trigger this lane.
An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator (auth).

@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown

Claude encountered an error after 1s —— View job


I'll analyze this and get back to you.

kyle-sexton added a commit that referenced this pull request Aug 5, 2026
…in the stale label comment (#350)

## Summary

Two findings against the claude-lane incident watchdog, both left over
from #331,
fixed in one PR because neither is worth a branch of its own.

### #348 — the write job's label comment was stale in both halves

The pinned comment claimed `claude-lane-incident` "does not exist in
this
repository's label taxonomy yet, and that taxonomy is managed by
github-iac
(Labels.cs)". It exists: github-iac declares it in
`GovernedRepositories.cs`
under this repository's `ExtraLabels`, and it is live on this repository
with
that description. `Labels.cs` owns the shared core taxonomy and is not
where a
per-repository extra label is declared, so the new comment cites
`GovernedRepositories.cs` instead. The consequence half is rewritten
too: the
next apply keeps the label rather than pruning it.

### #344 — the coverage copy named the rendered table, and was untested

`renderIssueBody` told the operator that a clean cycle counts only if it
polled
"every repository listed above" (and "below" in the header bullet). The
gate is
`coverageGap`, which reads the TRACKED index —
`MAX_TRACKED_REPOSITORIES` = 60 —
and `repositoriesSeen`, while the table is capped separately at
`MAX_RENDERED_REPOSITORIES` = 40 and a character budget. So the copy was
right
for a small incident and wrong in exactly the fleet-wide case it was
written for.
Measured on this branch: a 120-repository incident tracks 60 and renders
40 rows
at short names, 23 at the longest repository name that can exist.

Naming the tracked index alone would have replaced one overpromise with
another:
`coverageGap` also holds on `unlisted > 0`, so an incident wider than
the tracked
cap never gets a counting clean cycle however much was polled. Both
sentences now
state the whole condition — every tracked repository polled, AND the
index
accounting for everything seen — and the header bullet defers to step 4
rather
than compressing the rule into a parenthetical that would misstate it.

Step 4 also attributed a permanent hold to a gone repository alone.
`coverageGap`
gates on three, so the other two now appear in the durable artifact
rather than
only in the run warning: an incident wider than the tracked cap, and an
index
this watchdog can no longer read. The second is stated as the gate
condition
(`namesNothing`) rather than as one of its triggers — a hand-edited
state block
and one written by an older schema degrade identically, and a
`STATE_SCHEMA_VERSION` bump is how that reaches every open incident at
once, not
the only way an operator meets it.

The copy was also untested: the issue's second finding is that reverting
it left
the suite green. A rendered-body substring assertion over the fleet-wide
state now
covers it, following the convention the suite already uses.

## How the re-pin was done

The repo's pin discipline is byte-exact text comparison, not a digest.
`claude-lane-incident-write-gate.cjs` compares the workflow from the
marker
`# THE ONLY WRITE-SCOPED JOB.` to end of file against
`claude-lane-incident-write-job.pinned.yml`, and the aggregator test
additionally
asserts `workflow.endsWith(pinned)`. Every fixture that carries the
write job
carries the same bytes, so the comment could not be fixed in one file.

Rather than re-syncing the pinned tail over each fixture — which would
have made
the deliberately-broken write-job fixtures conformant and turned their
CORPUS
assertions green-on-nothing — a script substituted the same five comment
lines in
place, once per file, failing loudly on any file where the block did not
appear
exactly once. 49 files changed in one commit;
`workflow-not-a-mapping.yml` is the
one fixture carrying no write job and was correctly skipped.
`claude-lane-incident-mint-step.pinned.yml` is a separate pinned region
and is
untouched.

## Verification

- `node --test .github/scripts/*.test.cjs` — 505 pass, 0 fail (504 on
origin/main
  plus the one assertion this PR adds).
- Regression proof for #344: reverting `claude-lane-incident.cjs` alone,
with the
new test retained, turns exactly that test red (43 pass, 1 fail). A
green suite
  would not have been evidence — that is the finding.
- `npx @biomejs/biome@2.5.4 ci
--config-path=fixtures/typescript/good/biome.json
--error-on-warnings .github/scripts` — clean, matching what CI's biome
job runs.
- The pin is the workflow's byte-exact tail, checked directly
  (`workflow.slice(marker) === pinned`) as well as by the test.
- The github-iac declaration and the live label were both read at
authoring time
rather than taken from the issue text, and the "keeps rather than
prunes" claim
was traced through `Labels.Apply`, which unions `_core` with the
repository's
`ExtraLabels` for every spec with `ManagedLabels` (default `true`, which
  `ci-workflows` takes).

All 38 CI checks on the head commit are green.

Closes #344
Closes #348

## Related

- #331 — the merged PR both findings came out of, raised by its final
pre-merge
  verifier as non-blocking (verdict PASS-WITH-CONCERNS).
- melodic-software/github-iac#252 — declared `claude-lane-incident` in
the
  taxonomy, which is what made the pinned comment stale.
- #334 — also open against `claude-lane-incident-aggregator.yml`, but
only in the
workflow's header comment block and the aggregator test's poll
assertions. No
overlap with the write-job region or the fixtures this PR re-pins; the
two
  merge in either order.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Aug 5, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@claude

claude Bot commented Aug 5, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 46s —— View job


Security review complete

  • Gather context (diff, changed files)
  • Review workflow YAML changes
  • Review test file changes
  • Check against zizmor-covered concerns (skip those)
  • Post findings

No security issues found. This PR is comments-only.

Diff scope confirmed via git diff --stat origin/main...HEAD: 2 files, 10 insertions / 9 deletions, both purely explanatory-comment edits:

  • .github/workflows/claude-lane-incident-aggregator.yml — only the prose above the permissions: block changed (correcting the claim that omitting Checks:read always 403s). The actual granted scopes (checks: read, contents: read, issues: read, and the unchanged pull-requests line below it) are byte-identical to main — no permission widening, no scope added or removed.
  • .github/scripts/claude-lane-incident-aggregator.test.cjs — only the comment above the assertion changed. The assertion logic itself (poll.permissions[scope] === "read" || "write" for checks/contents/issues/pull-requests) is unchanged.

No triggers, no permissions: values, no secrets/token handling, no injection surfaces, and no runtime behavior are touched by this PR — matching the author's own test-plan note. Nothing here falls in scope for a deeper pass.
· branch docs/aggregator-dry-run-header

@kyle-sexton
kyle-sexton merged commit d25b689 into main Aug 5, 2026
38 checks passed
@kyle-sexton
kyle-sexton deleted the docs/aggregator-dry-run-header branch August 5, 2026 19:19
kyle-sexton added a commit to melodic-software/standards that referenced this pull request Aug 5, 2026
No linked issue

## Summary

Approve the reviewed `ci-workflows` v0.10.0 PR-linkage contract and its
byte-identical v0.10.1 repin, then move this repository's caller to
v0.10.1. The input, secret, and caller-permission surface is unchanged.
The reviewed runtime delta keeps every prerequisite outcome on the
caller-selected runner and replaces regex-only HTML-comment stripping
with the tested Markdown-aware parser.

This deploys the parser fix without weakening the fail-closed exact-SHA
runner policy; downstream consumers receive the approval through the
existing standards sync.

## Verification

- `actionlint .github/workflows/pr-issue-linkage.yml`
- `npm run lint:runner-policy`
- `npm run test:runner-policy` (242/242)
- `npx markdownlint-cli2 components/runner-policy/README.md`
- `git diff --check`

## Related

- melodic-software/ci-workflows#354
- melodic-software/ci-workflows#334
- [ci-workflows
v0.10.1](https://github.com/melodic-software/ci-workflows/releases/tag/v0.10.1)
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.

claude-lane-incident-aggregator: header overclaims that dry-run reads 403 without Checks:read

1 participant