feat(labels): auto-provision canonical label set on watched repos #101

Merged
code-lead merged 2 commits from boss/97 into main 2026-04-19 19:39:20 +00:00
Collaborator

Summary

Auto-provisions the canonical routing label set on every repo the service watches — fixes the silent-drop failure mode where add_issue_labels on a missing label no-ops server-side, stranding e.g. the designer → design-reviewer handoff 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.tsreconcileLabels(repo, token, specs) that POST /api/v1/repos/{repo}/labels for 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-level repos: [] in config/agents.json using 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 via bun run src/labels.ts. Token falls back to FORGEJO_TOKEN env var or agents.boss.token_file from agents.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; forgejoUrl option 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)

  • Milestone creation.
  • Color / description updates on existing labels.
  • Multi-org support — repos: is still a single list of owner/repo.
  • Option B from the issue (react to repository.created webhook) — not worth the wiring yet; startup + manual recipe cover the real flows.

Test plan

  • bun test src/labels.test.ts — 7 / 7 pass
  • bun test — full suite 267 / 267 pass
  • bun x tsc --noEmit — clean
  • bun x biome check src/ — clean
  • bun x biome format src/ — clean
  • Operator: edit config/agents.json repos: [...] for a new repo, restart service, verify [labels] created … log lines and that the labels appear in Forgejo's label UI.
  • Operator: 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

## Summary Auto-provisions the canonical routing label set on every repo the service watches — fixes the silent-drop failure mode where `add_issue_labels` on a missing label no-ops server-side, stranding e.g. the `designer → design-reviewer` handoff 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)` that `POST /api/v1/repos/{repo}/labels` for 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-level `repos: []` in `config/agents.json` using 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 via `bun run src/labels.ts`. Token falls back to `FORGEJO_TOKEN` env var or `agents.boss.token_file` from `agents.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; `forgejoUrl` option 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) - Milestone creation. - Color / description updates on existing labels. - Multi-org support — `repos:` is still a single list of `owner/repo`. - Option B from the issue (react to `repository.created` webhook) — not worth the wiring yet; startup + manual recipe cover the real flows. ## Test plan - [x] `bun test src/labels.test.ts` — 7 / 7 pass - [x] `bun test` — full suite 267 / 267 pass - [x] `bun x tsc --noEmit` — clean - [x] `bun x biome check src/` — clean - [x] `bun x biome format src/` — clean - [ ] Operator: edit `config/agents.json` `repos: [...]` for a new repo, restart service, verify `[labels] created …` log lines and that the labels appear in Forgejo's label UI. - [ ] Operator: `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
feat(labels): auto-provision canonical label set on watched repos
All checks were successful
qa / qa (pull_request) Successful in 2m36s
qa / dockerfile (pull_request) Successful in 11s
6fb0d4df70
Repos the webhook points at now get their routing labels
(`area:design`, `area:design-review`, `type:*`, …) auto-created at
service startup and via `just labels-bootstrap <repo>`. Fixes the
silent-drop failure mode seen in #97 where `add_issue_labels` on a
missing label no-ops server-side, stranding the design-review handoff.

Reconcile is idempotent and safety-first: existing labels are never
touched regardless of their color / description (operators may have
customised); 404 / 403 from Forgejo log a warning and move on.

Closes #97
code-lead force-pushed boss/97 from 6fb0d4df70
All checks were successful
qa / qa (pull_request) Successful in 2m36s
qa / dockerfile (pull_request) Successful in 11s
to c80b707536
All checks were successful
qa / qa (pull_request) Successful in 2m33s
qa / dockerfile (pull_request) Successful in 7s
2026-04-19 19:19:18 +00:00
Compare
reviewer requested changes 2026-04-19 19:23:00 +00:00
Dismissed
reviewer left a comment

Review: 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 waiting state 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 log

File: src/main.ts, in the label bootstrap block (after the registerWorker loop)

Problem: loadLabelSpecs() is called inside the outer try-catch that guards the entire startup config block. If config/labels.json is missing or contains invalid JSON, readFileSync / JSON.parse will throw. That exception propagates to the outer catch, which logs:

[startup] config not loaded — Forgejo webhooks disabled: ...

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.json is isolated from the rest of startup:

try {
  const specs = loadLabelSpecs();
  void (async () => {
    for (const repo of webhookConfig.repos) {
      await reconcileLabels(repo, firstAgent.token, specs, { forgejoUrl: webhookConfig.forgejoUrl });
    }
  })();
} catch (err) {
  console.warn("[labels] failed to load label specs — skipping bootstrap:", err);
}

🔎 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 end

Problem: The comment says:

// Create would fail (no POST handler for forge.example.com) —
// but the list passes and the POST is then attempted against the
// same base URL, returning the 500 → failure.

But the stub handler returns 200 [] for all URLs starting with https://forge.example.com/ — including the POST. The create call fails because the response body ([]) has no id field, so createLabel returns null, which reconcileLabels records as a failure. No 500 is ever returned. The assertion expect(result.failed).toEqual(["area:a"]) is correct, but the comment misrepresents why it fails.

Fix: Update the comment to accurately describe the failure path:

// The list endpoint returns 200 [] (area:a is missing), so a POST is attempted.
// The POST also hits the forge.example.com handler and returns 200 [] —
// no `id` in the response body, so createLabel returns null → failure.
expect(result.failed).toEqual(["area:a"]);
## Review: 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 `waiting` state 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 log **File**: `src/main.ts`, in the label bootstrap block (after the `registerWorker` loop) **Problem**: `loadLabelSpecs()` is called inside the outer `try-catch` that guards the entire startup config block. If `config/labels.json` is missing or contains invalid JSON, `readFileSync` / `JSON.parse` will throw. That exception propagates to the outer catch, which logs: ``` [startup] config not loaded — Forgejo webhooks disabled: ... ``` 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.json` is isolated from the rest of startup: ```typescript try { const specs = loadLabelSpecs(); void (async () => { for (const repo of webhookConfig.repos) { await reconcileLabels(repo, firstAgent.token, specs, { forgejoUrl: webhookConfig.forgejoUrl }); } })(); } catch (err) { console.warn("[labels] failed to load label specs — skipping bootstrap:", err); } ``` --- ### 🔎 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 end **Problem**: The comment says: ``` // Create would fail (no POST handler for forge.example.com) — // but the list passes and the POST is then attempted against the // same base URL, returning the 500 → failure. ``` But the stub handler returns `200 []` for **all** URLs starting with `https://forge.example.com/` — including the POST. The create call fails because the response body (`[]`) has no `id` field, so `createLabel` returns `null`, which `reconcileLabels` records as a failure. No 500 is ever returned. The assertion `expect(result.failed).toEqual(["area:a"])` is correct, but the comment misrepresents *why* it fails. **Fix**: Update the comment to accurately describe the failure path: ```typescript // The list endpoint returns 200 [] (area:a is missing), so a POST is attempted. // The POST also hits the forge.example.com handler and returns 200 [] — // no `id` in the response body, so createLabel returns null → failure. expect(result.failed).toEqual(["area:a"]); ```
fix(labels): isolate bootstrap errors + clarify forgejoUrl test comment
All checks were successful
qa / qa (pull_request) Successful in 2m35s
qa / dockerfile (pull_request) Successful in 10s
695737b25f
Addresses review on PR #101.

- `src/main.ts`: wrap `loadLabelSpecs()` + the background reconcile in
  their own try/catch. Previously a bad `config/labels.json` would
  propagate to the outer startup catch and log "config not loaded —
  Forgejo webhooks disabled", misleading operators into thinking the
  whole service was broken when only the label bootstrap had failed
  (workers are already registered by that point).
- `src/labels.test.ts`: fix the comment in the `forgejoUrl` test. The
  failure path is "POST returns 200 []; no `id` in body → createLabel
  returns null", not "POST returns 500".
Author
Collaborator

Addressed both review items in 695737b:

  • Issue 1src/main.ts: wrapped loadLabelSpecs() + 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).
  • Issue 2src/labels.test.ts: rewrote the comment to match reality (POST returns 200 []; no id in body → createLabel returns null → failure). No 500 in the stub.

Verification: bun test 273/273 pass, bun x tsc --noEmit clean, bun x biome check src/ clean. Pushing this commit should also unstick the CI run.

Addressed both review items in `695737b`: - **Issue 1** — `src/main.ts`: wrapped `loadLabelSpecs()` + 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). - **Issue 2** — `src/labels.test.ts`: rewrote the comment to match reality (POST returns 200 `[]`; no `id` in body → `createLabel` returns `null` → failure). No 500 in the stub. Verification: `bun test` 273/273 pass, `bun x tsc --noEmit` clean, `bun x biome check src/` clean. Pushing this commit should also unstick the CI run.
Collaborator

Code Review — REQUEST_CHANGES (CI pending)

Outcome: REQUEST_CHANGES — CI run #1636 was still running at 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.tsreconcileLabels() 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.tsrepos field defensively parsed (filters to strings containing /, defaults to [] when the key is absent). WebhookConfig interface updated cleanly.

src/main.ts — Background IIFE (void (async () => { … })()) fires reconcile for each repo without blocking server startup. The try/catch around loadLabelSpecs() is correctly isolated from the outer startup catch so a bad labels.json doesn't disable webhook routing for all agents. Token selection (find((a) => a.token)) correctly skips agents whose token file couldn't be read.

justfile labels-bootstrap recipe — Token fallback chain (TOKEN arg → FORGEJO_TOKEN env → agents.boss.token_file → first agent) works correctly. TOKEN='' defaults to empty string so the fallback logic fires on a bare just labels-bootstrap charles/foo call.

src/labels.test.ts — 7 tests: partial existing set, full no-op, 404, 403, per-label create failure (continues loop), multi-page pagination, and forgejoUrl option routing. All clearly named and self-contained with an injected fetchImpl to 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.

## Code Review — REQUEST_CHANGES (CI pending) **Outcome: REQUEST_CHANGES** — CI run #1636 was still `running` at 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`** — `repos` field defensively parsed (filters to strings containing `/`, defaults to `[]` when the key is absent). `WebhookConfig` interface updated cleanly. **`src/main.ts`** — Background IIFE (`void (async () => { … })()`) fires reconcile for each repo without blocking server startup. The `try/catch` around `loadLabelSpecs()` is correctly isolated from the outer startup catch so a bad `labels.json` doesn't disable webhook routing for all agents. Token selection (`find((a) => a.token)`) correctly skips agents whose token file couldn't be read. **`justfile` `labels-bootstrap` recipe** — Token fallback chain (TOKEN arg → `FORGEJO_TOKEN` env → `agents.boss.token_file` → first agent) works correctly. `TOKEN=''` defaults to empty string so the fallback logic fires on a bare `just labels-bootstrap charles/foo` call. **`src/labels.test.ts`** — 7 tests: partial existing set, full no-op, 404, 403, per-label create failure (continues loop), multi-page pagination, and `forgejoUrl` option routing. All clearly named and self-contained with an injected `fetchImpl` to 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.**
reviewer approved these changes 2026-04-19 19:38:09 +00:00
reviewer left a comment

Review — PR #101: feat(labels): auto-provision canonical label set on watched repos

CI: green (695737b — job #1636, 2m46s, success)

Acceptance criteria — all met

Criterion Status
config/labels.json with {name, color, description, scope} 13 labels, correct format
reconcileLabels POSTs missing, leaves existing alone, idempotent pass
main.ts startup reconciles every repo in webhookConfig.repos background void-IIFE, won't block HTTP server
just labels-bootstrap ad-hoc recipe 3-level token fallback (arg → FORGEJO_TOKEN → agents.boss.token_file)
Only creates missing — never deletes / renames pass
Log format matches spec pass
404 / 403 from list endpoint → warn + move on, no crash pass
labels.test.ts partial set creates missing, skips existing pass
labels.test.ts 404 / 403 tolerance pass
CLAUDE.md bootstrap section pass

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.

## Review — PR #101: feat(labels): auto-provision canonical label set on watched repos **CI**: green (695737b — job #1636, 2m46s, success) ### Acceptance criteria — all met | Criterion | Status | |---|---| | config/labels.json with {name, color, description, scope} | 13 labels, correct format | | reconcileLabels POSTs missing, leaves existing alone, idempotent | pass | | main.ts startup reconciles every repo in webhookConfig.repos | background void-IIFE, won't block HTTP server | | just labels-bootstrap <repo> ad-hoc recipe | 3-level token fallback (arg → FORGEJO_TOKEN → agents.boss.token_file) | | Only creates missing — never deletes / renames | pass | | Log format matches spec | pass | | 404 / 403 from list endpoint → warn + move on, no crash | pass | | labels.test.ts partial set creates missing, skips existing | pass | | labels.test.ts 404 / 403 tolerance | pass | | CLAUDE.md bootstrap section | pass | ### 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.
code-lead deleted branch boss/97 2026-04-19 19:39:21 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
charles/claude-hooks!101
No description provided.