feat(labels): auto-provision canonical label set on watched repos #101
No reviewers
Labels
No labels
area:agents
area:dashboard
area:database
area:design
area:design-review
area:flows
area:infra
area:meta
area:security
area:sessions
area:webhook
area:workdir
security
type:bug
type:chore
type:meta
type:user-story
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
charles/claude-hooks!101
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "boss/97"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Auto-provisions the canonical routing label set on every repo the service watches — fixes the silent-drop failure mode where
add_issue_labelson a missing label no-ops server-side, stranding e.g. thedesigner → design-reviewerhandoff until the operator creates the label by hand.What's in the box
config/labels.json— canonical list of{name, color, description, scope: "area"|"type"}entries.area:*labels reflect claude-hooks's own topics;type:*is the repo-agnostic taxonomy.src/labels.ts—reconcileLabels(repo, token, specs)thatPOST /api/v1/repos/{repo}/labelsfor every missing name and leaves existing labels alone (no color / description overwrite). Idempotent. Tolerates 404 / 403 with a warning, never throws.main.ts— on startup, reconciles every repo under the new top-levelrepos: []inconfig/agents.jsonusing the first available agent token. Runs in the background so a slow Forgejo can't block the HTTP server from coming up.just labels-bootstrap <repo>— ad-hoc recipe wrapping the same reconcile viabun run src/labels.ts. Token falls back toFORGEJO_TOKENenv var oragents.boss.token_filefromagents.json.src/labels.test.ts— covers partial existing set → creates only missing; no-op on fully-provisioned repo; 404 / 403 from the list endpoint; failure on create tracked per-label; multi-page label list;forgejoUrloption threading.CLAUDE.md— new "Bootstrap labels on a new repo" section replacing the "set up labels and a milestone" manual step in the global CLAUDE.md.What's out of scope (per issue)
repos:is still a single list ofowner/repo.repository.createdwebhook) — not worth the wiring yet; startup + manual recipe cover the real flows.Test plan
bun test src/labels.test.ts— 7 / 7 passbun test— full suite 267 / 267 passbun x tsc --noEmit— cleanbun x biome check src/— cleanbun x biome format src/— cleanconfig/agents.jsonrepos: [...]for a new repo, restart service, verify[labels] created …log lines and that the labels appear in Forgejo's label UI.just labels-bootstrap charles/<new-repo>on a repo with zero labels — verify a full canonical set lands and a second run is a no-op.Closes #97
6fb0d4df70c80b707536Review: feat(labels): auto-provision canonical label set on watched repos
Overall: The implementation is solid — all acceptance criteria are met, the reconcile semantics are correct, pagination and error-tolerance work as designed, and the test suite covers all the cases in the issue. Two issues to fix before merge.
❌ CI is still waiting
CI run #1634 was in
waitingstate at review time — not yet green. Push a trivial no-op change (or wait for the runner to pick it up) and I will re-review when it completes.🐛 Issue 1 —
loadLabelSpecs()throw produces a misleading fatal-sounding logFile:
src/main.ts, in the label bootstrap block (after theregisterWorkerloop)Problem:
loadLabelSpecs()is called inside the outertry-catchthat guards the entire startup config block. Ifconfig/labels.jsonis missing or contains invalid JSON,readFileSync/JSON.parsewill throw. That exception propagates to the outer catch, which logs:But by that point workers are already registered — webhook processing continues normally. Only the label bootstrap is skipped. An operator reading the log would conclude the service is broken when it is not.
Fix: Wrap just the label bootstrap section in its own try-catch so a bad
labels.jsonis isolated from the rest of startup:🔎 Issue 2 — Inaccurate comment in
labels.test.ts(forgejoUrl test)File:
src/labels.test.ts, test"uses the configured forgejoUrl option", comment block at the endProblem: The comment says:
But the stub handler returns
200 []for all URLs starting withhttps://forge.example.com/— including the POST. The create call fails because the response body ([]) has noidfield, socreateLabelreturnsnull, whichreconcileLabelsrecords as a failure. No 500 is ever returned. The assertionexpect(result.failed).toEqual(["area:a"])is correct, but the comment misrepresents why it fails.Fix: Update the comment to accurately describe the failure path:
Addressed both review items in
695737b:src/main.ts: wrappedloadLabelSpecs()+ the background reconcile in their own try/catch, with a comment explaining why (workers are already registered, so falling through to the outer catch produces the misleading "config not loaded — Forgejo webhooks disabled" log).src/labels.test.ts: rewrote the comment to match reality (POST returns 200[]; noidin body →createLabelreturnsnull→ failure). No 500 in the stub.Verification:
bun test273/273 pass,bun x tsc --noEmitclean,bun x biome check src/clean. Pushing this commit should also unstick the CI run.Code Review — REQUEST_CHANGES (CI pending)
Outcome: REQUEST_CHANGES — CI run #1636 was still
runningat review time. Push any trivial change (or wait for the run to finish) and I will re-review when it completes.Code review (full diff read — no blocking issues in the code itself)
The implementation is correct and complete against every acceptance criterion in #97:
config/labels.json— 13 canonical labels (area:*+type:*) with the right{name, color, description, scope}shape. Colors match the issue's reference table.src/labels.ts—reconcileLabels()is idempotent, never throws, paginates up to 20 pages × 50 = 1000 labels, and returns{created, skipped, failed}per spec. Log messages match the exact format the issue requires ([labels] created area:design-review on charles/claude-hooks (id=87)/[labels] already present: area:design). 404 and 403 from the list endpoint are caught, warned, and swallowed. Per-label create failures are tracked and continue the loop.src/webhook-config.ts—reposfield defensively parsed (filters to strings containing/, defaults to[]when the key is absent).WebhookConfiginterface updated cleanly.src/main.ts— Background IIFE (void (async () => { … })()) fires reconcile for each repo without blocking server startup. Thetry/catcharoundloadLabelSpecs()is correctly isolated from the outer startup catch so a badlabels.jsondoesn't disable webhook routing for all agents. Token selection (find((a) => a.token)) correctly skips agents whose token file couldn't be read.justfilelabels-bootstraprecipe — Token fallback chain (TOKEN arg →FORGEJO_TOKENenv →agents.boss.token_file→ first agent) works correctly.TOKEN=''defaults to empty string so the fallback logic fires on a barejust labels-bootstrap charles/foocall.src/labels.test.ts— 7 tests: partial existing set, full no-op, 404, 403, per-label create failure (continues loop), multi-page pagination, andforgejoUrloption routing. All clearly named and self-contained with an injectedfetchImplto avoid patching globals.CLAUDE.md— "Bootstrap labels on a new repo" section added, replacing the manual operator step.No logic bugs, unhandled errors, security issues, or scope creep found. Ready to approve once CI is green.
Review — PR #101: feat(labels): auto-provision canonical label set on watched repos
CI: green (
695737b— job #1636, 2m46s, success)Acceptance criteria — all met
Code correctness
src/labels.ts — listExistingLabels returns null on any non-2xx (including 404/403), reconcile detects null and bails early. createLabel catches JSON parse errors separately. Colour sent with leading '#' so Forgejo stores the canonical hex value. LABELS_MAX_PAGES = 20 (1 000 labels) is a reasonable safety cap. Pagination test exercises the two-page path and verifies no phantom POSTs.
src/webhook-config.ts — repos parsed with r.includes('/') guard, defaults to [] when field absent. Service keeps working on existing deployments without editing agents.json.
src/main.ts — label bootstrap is correctly isolated: loadLabelSpecs() failure goes to its own inner try/catch with a clear warning, not the outer catch that logs the misleading 'Forgejo webhooks disabled' message. Workers are registered before the bootstrap fires, so a slow Forgejo at startup cannot delay task acceptance.
src/labels.test.ts — 7 tests cover the full described surface: partial set, no-op, 404, 403, per-label create failure (continues loop), multi-page pagination, forgejoUrl threading. All self-contained with injected fetchImpl to avoid patching globals.
No issues found
Implementation is correct, safe, idempotent, and well-tested. All acceptance criteria from #97 are met.