fix(ci): make shard manifest refresh reliable (BLO-24241)

[kkroo]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Its CI uses duration-aware general-server shards so expensive suites are distributed across ARC runners
• The shard duration manifest can drift as suites are added, and the former 90% gate turned a small amount of drift into a required-policy failure that skipped the rest of CI
• The unattended refresh path also had two reliability hazards: it could kill Vitest on an unconsumed stdout buffer, and it could destroy durable manifest guidance or create a PR with a credential that cannot trigger its checks
• This pull request separates the hard safety floor from visible freshness diagnostics, measures and merges all shard durations on a schedule, and makes refresh fail closed with explicit App-token credentials
• The benefit is that shard balance stays observable and recoverable without cascading unrelated PRs or silently producing a review PR that can never run CI

Linked Issues or Issue Description

Paperclip issue: BLO-24241

GitHub Issues are disabled in this repository, so the issue is described here: the general-server duration manifest was allowed to drift as suites landed; the old required 90% coverage assertion converted ordinary drift into a policy failure that skipped unrelated CI lanes; and the unattended refresh path could fail silently through output buffering, metadata loss, or a GITHUB_TOKEN-authored PR whose checks never start.

Related prior work: #1173 is the earlier merged manifest refresh. #1278 is the stale predecessor for this BLO-24241 repair and is behind current master; #1487 is the fresh replacement rebased onto the current base. The predecessor is intentionally left unchanged while this replacement goes through review.

What Changed

• Extract shared general-server suite enumeration so the runner and freshness diagnostics use one source of truth.
• Replace the brittle required 90% cliff with a 75% hard floor plus a strict, visible, non-blocking 100% freshness check naming missing suites and the repair command.
• Add four-way scheduled shard measurement and a tested manifest merge that preserves $notes, prunes deleted suites safely, and retains durations from partial shard results.
• Stream Vitest output during measurement to avoid the unconsumed spawnSync buffer/ENOBUFS failure mode.
• Require COMMITPERCLIP_KEY for unattended refresh PR creation, use the generated App token for both push and GitHub CLI operations, and use a non-App commit author.
• Preserve complete dry-run JSON output with a synchronous write and add workflow-contract coverage for the token fallback and partial-shard behavior.

Verification

• node --test scripts/__tests__/run-vitest-stable-shard.test.mjs scripts/check-shard-manifest-freshness.test.mjs scripts/__tests__/measure-general-server-shard-durations.test.mjs scripts/__tests__/merge-shard-duration-manifest.test.mjs scripts/__tests__/refresh-shard-manifest-workflow.test.mjs (39 passed)
• pnpm typecheck (full workspace, 37 projects passed)
• ACTIONLINT_BIN=/private/tmp/paperclip-tools/bin/actionlint node scripts/check-workflows-parse.mjs (29 workflows passed)
• node scripts/check-github-runner-labels.mjs (passed)
• node scripts/check-commit-author-attribution.mjs --base origin/master --head HEAD (passed)
• git diff --check (passed)

Risks

The weekly refresh intentionally opens a PR rather than mutating master; a failed measurement shard keeps its prior durations, while a total empty measurement fails loudly. The 75% floor still blocks a genuinely abandoned or malformed manifest, but ordinary small drift remains visible without skipping unrelated required jobs. The refresh job fails closed when the App credential is unavailable, so repository secret/configuration repair is required before unattended refresh can create a PR.

Model Used

OpenAI Codex, GPT-5. Exact runtime model identifier is the Codex GPT-5 coding-agent model available in this session; extended tool use and code execution enabled.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes: # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have run tests locally and they pass
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots
• I have updated relevant documentation to reflect my changes
• I have considered and documented any risks above
• All Paperclip CI gates are green
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-24241

[allyblockcast]

🔗 Paperclip issue: BLO-24241

[kkroo]

@ally

Please review the exact current head 76130de075eed170b2d442c5a7a84b1b940fbc92.

Focus on:

• COMMITPERCLIP_KEY provenance and the absence of any github.token fallback
• manifest coverage and the 75% hard floor versus the visible 100% freshness check
• refresh workflow author identity and App-token push/PR behavior
• partial-shard and empty-measurement handling
• dry-run JSON output integrity

Please report consolidated Critical and Important findings against this exact SHA.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 76130de

Critical Issues (0)

Important Issues (1)

• [gstack/review] .github/workflows/refresh-shard-manifest.yml:201-218 — the no-CI alarm uses the generated installation token for gh api repos/.../actions/runs, but the workflow's permissions: actions: read only applies to the default GITHUB_TOKEN; an installation token is authorized only by the App installation's own permissions. If commitperclip is installed without Actions read (a common least-privilege setup, and not asserted by this workflow), this step exits on the API call before posting either the action_required or zero-run alert, so the new failure signal is lost.
  • Use a credential that is guaranteed to have Actions read for the alarm (for example the workflow token, if repository policy permits it), or explicitly validate/document the App installation's Actions permission and handle a 403 as a distinct loud failure. Add a workflow-contract test covering the credential and permission contract.

Suggestions (0)

Strengths

• The refresh workflow fails closed when COMMITPERCLIP_KEY is absent and does not include a github.token fallback.
• The 75% hard floor is separated from the visible, non-blocking 100% freshness check.
• Partial shard artifacts preserve prior durations, empty total measurements fail loudly, and durable $notes guidance is preserved.
• Dry-run JSON is written synchronously, and suite enumeration is shared between the runner and freshness diagnostics.

Recommended Action

1. Fix Important issues before merge.
2. Address Suggestions opportunistically.

[kkroo]

@ally

Please re-review the exact current head d4c7c9e89 after addressing your Important finding on the previous head 76130de0.

The refresh PR still uses the commitperclip App token exclusively for the branch push and PR create/update. The no-CI alarm now uses ${{ github.token }} instead, matching the job-level actions: read and pull-requests: write permissions; the workflow-contract test asserts the split and prevents the App token from being used for the alarm.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: d4c7c9e

Prior Findings Dispositioned (1)

• prior:76130de critical 1 — fixed — .github/workflows/refresh-shard-manifest.yml:246-252 — the no-CI alarm now authenticates with ${{ github.token }}, whose Actions and pull-request permissions are declared on the refresh job; the contract test also asserts the alarm does not use the App installation token.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The no-CI alarm now uses the workflow token for the API read and diagnostic comment while keeping the commitperclip App token isolated to push and PR creation.
• The workflow-contract test explicitly protects the split credential and permission contract.
• Refresh remains fail-closed without COMMITPERCLIP_KEY, and the manifest merge preserves partial shard results and durable notes.

Recommended Action

1. Merge when the remaining repository checks are green.

[kkroo]

@ally

Please re-review the exact current head 328c129dee25072e52bbf8d822b47e2b78a87e22 after the clean rebase onto f952483a474f9069b8d62cf9243c69bf4938497a. The code is unchanged relative to the previously reviewed PR commits; only the base advanced. Please submit the formal consolidated review against this full SHA.

Correction: the prior comment contained a mistyped SHA string. Ally formal review is recorded against the exact SHA above.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 328c129

Prior Findings Dispositioned (1)

• prior:76130de important 1 — fixed — .github/workflows/refresh-shard-manifest.yml:246-252 — the no-CI alarm now uses ${{ github.token }} for the Actions read and diagnostic comment, while the App installation token remains isolated to push and PR creation; the workflow contract test asserts this split.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The refresh workflow fails closed when COMMITPERCLIP_KEY is absent, preventing an unattended PR authored with the workflow token.
• Partial shard artifacts preserve prior durations, while an entirely empty measurement fails loudly.
• Manifest merging preserves durable $notes guidance and safely prunes only suites absent from the current checkout.
• Vitest measurement and dry-run output avoid the prior output-buffer and truncated-pipe failure modes.

Recommended Action

1. Merge when the remaining repository checks are green.

Stale review re-anchored by the force-push; superseded by exact-head review 5005650995 on 328c129.

[kkroo]

@ally

Please re-review the exact current head cb52180b11051468eb576ef928481188315ddb82 after the clean rebase onto current master (ff3c822636fdd5b84534fd3dcdc4296da6569c43).

The rebase resolved the generated shard manifest as the union of current on-disk suites: 443/443 entries, preserving master’s plugin-status-metrics measurement and the PR’s durable $notes plus 44 backfills. Focus on the existing credential split, workflow behavior, and manifest merge correctness. Please submit the formal consolidated review against this exact SHA with Critical and Important findings.

[kkroo]

@ally

Please perform the formal consolidated review for the exact current PR head:
cb52180b11051468eb576ef928481188315ddb82

This is a fresh request after the prior reviewer wake failed during Paperclip setup. Review the current PR, report Critical and Important findings, and submit the formal GitHub review against that exact full SHA.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: cb52180

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The refresh workflow separates the commitperclip App token used for push and PR creation from the workflow token used for Actions-run inspection and diagnostic comments.
• Partial measurement shards preserve existing durations, while a completely empty measurement fails closed instead of rewriting the manifest with unusable data.
• Suite enumeration is shared between the runner, freshness diagnostic, and merge pruning, reducing drift between what CI runs and what the manifest describes.
• The manifest merge preserves durable $notes guidance, and the measurement path avoids the prior unconsumed stdout buffer failure.

Recommended Action

1. Merge when the remaining repository checks are green.