Repository navigation
fix(repository-credentials): name an App key that goes bad after startup - #3269
Conversation
The prepared Git owner's maintenance loop reported every failure as PREPARED_GITHUB_MAINTENANCE_FAILED. A rotated Secret with a key that cannot sign now logs worker.error with PREPARED_GITHUB_APP_KEY_INVALID and the same fixed reason as the startup refusal, never key bytes or the path. TokenService.ownFamilies passes the pass's error to failed(). Docs: the production startup flow says the worker reads the App key at startup and exits before readiness; the runbook describes the runtime log. Trimmed equal words from the startup flow to stay under the cap.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
Internal review nits: keep the test key out of the socket directory helper, note that the 60 s test timeout bounds the wait, tell owners never to log the error passed to failed(), and give the 5 s retry interval in the runbook.
clawsweeper
Bot
added
clawsweeper:intake:bug
|
Codex review: needs maintainer review before merge. What this changesThis PR reports a fixed App-key error code and reason when repository credential maintenance encounters an unusable key after startup. Example: After worker startup, the mounted RSA private key is replaced with an RSA public key.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review Keep this PR open: it addresses a current diagnostic gap with a focused change, and no actionable correctness or security findings remain. Priority: P2 Before mergeNone. FindingsNone. Tests
Agent review detailsHow this fits togetherThe controller worker's prepared Git owner reloads its mounted App key during maintenance; TokenService schedules passes and forwards failures to the owner, whose fixed diagnostics enter worker logging. flowchart TB
A[Worker maintenance] --> B[TokenService pass]
B --> C[Prepared Git owner]
C --> D[Read mounted App key]
D --> E[Typed key refusal]
E --> F[Fixed code and reason]
F --> G[Worker error log]
Technical reviewBest possible solution: Retain error classification in the existing prepared Git owner and preserve generic reporting for unrelated maintenance failures. Do we have a high-confidence way to reproduce the issue? Current-main source establishes that a runtime key-reading error reaches a callback that discards its classification; the author reports a base-failing, head-passing regression, which this read-only review did not execute. Is this the best way to solve the issue? Forwarding the exception to the existing owner is the smallest complete repair and leaves scheduling, key restrictions, issuance, and authorization unchanged. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against da921f0dd4b0. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
Likely related people:
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 23, 2026, 12:07 AM ET / 04:07 UTC. |
Problem
#3258 made the worker refuse a prepared-git App key that cannot sign before it reports ready, with
PREPARED_GITHUB_APP_KEY_INVALIDand a fixedreason. If the key goes bad while the worker runs (for example a rotated Secret with an unusable key), maintenance still logged onlyPREPARED_GITHUB_MAINTENANCE_FAILED, so the operator could not tell a bad key from any other failed pass. The production startup flow also did not mention the startup refusal (a P3 review note on #3258).Change
TokenService.ownFamiliespasses the failed pass's error tofailed(error).PreparedGitHubAppKeyErrorfrom maintenance asPREPARED_GITHUB_APP_KEY_INVALIDwith its fixedreason, which the worker logs asworker.error. Other failures still reportPREPARED_GITHUB_MAINTENANCE_FAILED. No key bytes or path are logged.docs/flows/production-startup.mdsays the worker reads the App key at startup and exits before readiness when it cannot sign. To stay under the word cap, it drops a debugging bullet that repeated theocc installation getID check from step 5 and tightens two sentences.Verification
tests/integration/postgres-prepared-github-git-use.test.mjs: starts an owner with a valid key, replaces the key with a public key, runs the real maintenance loop and asserts the report isPREPARED_GITHUB_APP_KEY_INVALID/app_key_unparsablewith no issuance. It fails on main (PREPARED_GITHUB_MAINTENANCE_FAILEDwith no reason) and passes 10/10 at 2 CPUs. The whole file passes (18/18), as doespostgres-prepared-token-service.test.ts.tsc --build, eslint and prettier are clean on touched files.production-startup.mdis at 2487 words (was 2488).Risks
Low. The only behavior change is the code and
reasonlogged for one maintenance failure class. Alerts that matchPREPARED_GITHUB_MAINTENANCE_FAILEDwill no longer fire for a bad App key. They should matchPREPARED_GITHUB_APP_KEY_INVALID, which startup already logs.