charles/claude-hooks

Fork 0

feat(deps): auto-assign unassigned-ready dependents + /unassign escape hatch #201

Merged

code-lead merged 1 commit from boss/200 into main

2026-04-21 00:04:03 +00:00

code-lead commented

2026-04-20 23:57:18 +00:00

Collaborator

Summary

Policy pivot on top of #196: when a blocker closes and the dependent issue is unassigned but ready, auto-assign to the suggested agent type and post a one-line audit comment instead of asking the operator to click. At single-operator scale, a manual tap per unblock defeats the point of automation; the heuristic is good enough to try directly and /unassign makes the occasional misroute cheap to fix.

propagateDependencyClosure unassigned branch → updateIssueAssignees + audit createIssueComment. Forgejo's follow-on issues.assigned webhook kicks off the real dispatch — we don't call onAssignedReady from the propagator in this path.
New /unassign trust-gated slash command clears assignees via PATCH /issues/{N} and posts a short ack. No SQLite persistence — the next close-cycle re-evaluates from state. /hold still exists for the "suppress all automation" semantics.
Dropped READY_COMMENT_MARKER / renderReadyComment / hasReadyComment / ClearedBlocker: the assignee-state check is the natural dedup for duplicate webhook deliveries.
/issues/ready endpoint semantics unchanged; it now surfaces the usually-empty "operator-unassigned or pre-boot-ready" slice.

Closes #200

Test plan

bun x turbo run typecheck — all three workspace packages clean
bun x biome check . — no lint errors
bun x turbo run test — 849 pass / 0 fail across 41 files
deps.test.ts::propagateDependencyClosure — unassigned branch now asserts PATCH { assignees: ["dev"] } + audit comment (type:chore → dev heuristic), onAssignedReady does not fire
deps.test.ts::propagateDependencyClosure — new "PATCH failure → no audit comment" case
deps.test.ts::propagateDependencyClosure — new "duplicate delivery → routes through assigned branch, no double auto-assign" case
deps.test.ts::parseUnassignCommand + applyUnassignCommand — new parser + handler coverage, including PATCH-failure path
webhook-handlers.test.ts::handleIssueComment — /unassign from trusted user fires the PATCH + ack; untrusted user is a no-op
Live canary on #170 M18-9 Sunset: closing its last remaining blocker (#167 M18-6) auto-dispatches boss within seconds

## Summary Policy pivot on top of #196: when a blocker closes and the dependent issue is unassigned but ready, auto-assign to the suggested agent type and post a one-line audit comment instead of asking the operator to click. At single-operator scale, a manual tap per unblock defeats the point of automation; the heuristic is good enough to try directly and `/unassign` makes the occasional misroute cheap to fix. - `propagateDependencyClosure` unassigned branch → `updateIssueAssignees` + audit `createIssueComment`. Forgejo's follow-on `issues.assigned` webhook kicks off the real dispatch — we don't call `onAssignedReady` from the propagator in this path. - New `/unassign` trust-gated slash command clears assignees via `PATCH /issues/{N}` and posts a short ack. No SQLite persistence — the next close-cycle re-evaluates from state. `/hold` still exists for the "suppress all automation" semantics. - Dropped `READY_COMMENT_MARKER` / `renderReadyComment` / `hasReadyComment` / `ClearedBlocker`: the assignee-state check is the natural dedup for duplicate webhook deliveries. - `/issues/ready` endpoint semantics unchanged; it now surfaces the usually-empty "operator-unassigned or pre-boot-ready" slice. Closes #200 ## Test plan - [x] `bun x turbo run typecheck` — all three workspace packages clean - [x] `bun x biome check .` — no lint errors - [x] `bun x turbo run test` — 849 pass / 0 fail across 41 files - [x] `deps.test.ts::propagateDependencyClosure` — unassigned branch now asserts PATCH `{ assignees: ["dev"] }` + audit comment (type:chore → dev heuristic), `onAssignedReady` does not fire - [x] `deps.test.ts::propagateDependencyClosure` — new "PATCH failure → no audit comment" case - [x] `deps.test.ts::propagateDependencyClosure` — new "duplicate delivery → routes through assigned branch, no double auto-assign" case - [x] `deps.test.ts::parseUnassignCommand` + `applyUnassignCommand` — new parser + handler coverage, including PATCH-failure path - [x] `webhook-handlers.test.ts::handleIssueComment` — `/unassign` from trusted user fires the PATCH + ack; untrusted user is a no-op - [ ] Live canary on **#170 M18-9 Sunset**: closing its last remaining blocker (#167 M18-6) auto-dispatches boss within seconds

code-lead added 1 commit

2026-04-20 23:57:18 +00:00

feat(deps): auto-assign unassigned-ready dependents + /unassign escape hatch (#200 )

qa / qa (pull_request) Successful in 3m43s

Details

qa / dockerfile (pull_request) Successful in 8s

Details

878e35d1ba

Policy pivot on top of #196: when a blocker closes and the dependent
issue is unassigned but ready, auto-assign to the suggested agent type
and post a one-line audit comment instead of asking the operator to
click. `/unassign` slash command is the cheap reroute for misroutes;
`/hold` still exists for "suppress all automation".

- `propagateDependencyClosure` unassigned branch → `updateIssueAssignees`
  + audit `createIssueComment`; Forgejo's `issues.assigned` webhook
  drives the actual dispatch.
- New `/unassign` trust-gated slash command clears assignees + posts a
  short ack, no SQLite persistence (next close-cycle re-evaluates).
- Dropped `READY_COMMENT_MARKER` / `renderReadyComment` / `hasReadyComment`
  / `ClearedBlocker` — the assignee-state check is the natural dedup.
- `/issues/ready` endpoint semantics unchanged; it now surfaces the
  usually-empty "operator-unassigned or pre-boot-ready" slice.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

code-lead self-assigned this

2026-04-20 23:57:26 +00:00

code-lead requested review from reviewer

2026-04-21 00:01:13 +00:00

reviewer approved these changes

2026-04-21 00:03:33 +00:00

reviewer left a comment

Review

CI: ✅ green (run #1820, sha 878e35d, 3m53s). Safe to approve.

All acceptance criteria from #200 met. Reviewed: deps.ts, deps.test.ts, webhook-handlers.ts, webhook-handlers.test.ts, forgejo-api.ts.

Core logic — `propagateDependencyClosure` unassigned branch

The pivot is clean and correct:

updateIssueAssignees fires first; on failure (!assigned) the function logs + continues — no audit comment is posted for a state change that didn't happen. Good defensive ordering.
createIssueComment fires after the PATCH succeeds; comment failure is warn-logged but doesn't revert the assignment — correct, the authoritative state already changed.
onAssignedReady is not called from this branch — the subsequent issues.assigned webhook owns dispatch. This is the right design: avoids a double-dispatch and keeps the propagator's surface area narrow.

Duplicate-delivery dedup is handled naturally: a second issues.closed for the same blocker sees the dependent is now assigned (first pass already ran), routes to the assignees.length > 0 branch, and handleIssueAssigned's existing 30s type-level dedup drops the redundant dispatch.

`/unassign` slash command (`webhook-handlers.ts`)

Pattern is correctly mirrored from /hold:

Trust check (isTrustedUser) before any mutation — ✅
probeToken() for the PATCH — consistent with how handleIssueClosed's propagation gets its token — ✅
Returns null regardless of applyUnassignCommand's boolean return — correct; the boolean matters for the propagator's comment-sequencing, not for the webhook response

`applyUnassignCommand` (`deps.ts`)

Return semantics are correct: returns true when the PATCH succeeded (assignees cleared), even if the ack comment subsequently fails — because the authoritative state is the Forgejo issue, not the comment. Callers can use the boolean to decide whether to trust that the clear landed.

Test coverage

deps.test.ts additions are thorough:

Unassigned-ready → auto-assign PATCH + audit comment (verifies onAssignedReady does NOT fire)
PATCH failure → no audit comment, no dispatch
Duplicate delivery → routes to assigned branch, no second PATCH, no second comment
parseUnassignCommand: first-token-prefix, case-insensitive, mid-line negative
applyUnassignCommand: success path, PATCH-failure path (ack skipped)

webhook-handlers.test.ts: trusted user gets PATCH + ack; untrusted user is a no-op (no PATCH). Correct trust-gate coverage.

Minor observations (no action needed)

UNASSIGN_ACK message "Re-add a blocker + close it to re-trigger auto-assign" is technically accurate but slightly awkward — it means re-open + re-close an existing one, or add a new one. Acceptable for operator tooling at this scale.
AUTO_ASSIGN_COMMENT_MARKER exported as a stable string for future dashboard/scraper use — good forward-thinking.
forgejo-api.ts change is a pure docstring update to listIssueComments adding /unassign to the consumer list — ✅ no logic change.
The live canary (unchecked in the test plan) is expected — it can only be verified post-merge in production.

LGTM. Clean policy pivot with correct failure sequencing and solid test coverage.

## Review CI: ✅ green (run #1820, sha `878e35d`, 3m53s). Safe to approve. **All acceptance criteria from #200 met.** Reviewed: `deps.ts`, `deps.test.ts`, `webhook-handlers.ts`, `webhook-handlers.test.ts`, `forgejo-api.ts`. --- ### Core logic — `propagateDependencyClosure` unassigned branch The pivot is clean and correct: 1. `updateIssueAssignees` fires first; on failure (`!assigned`) the function logs + `continue`s — **no audit comment is posted for a state change that didn't happen**. Good defensive ordering. 2. `createIssueComment` fires after the PATCH succeeds; comment failure is warn-logged but doesn't revert the assignment — correct, the authoritative state already changed. 3. `onAssignedReady` is **not** called from this branch — the subsequent `issues.assigned` webhook owns dispatch. This is the right design: avoids a double-dispatch and keeps the propagator's surface area narrow. Duplicate-delivery dedup is handled naturally: a second `issues.closed` for the same blocker sees the dependent is now assigned (first pass already ran), routes to the `assignees.length > 0` branch, and `handleIssueAssigned`'s existing 30s type-level dedup drops the redundant dispatch. --- ### `/unassign` slash command (`webhook-handlers.ts`) Pattern is correctly mirrored from `/hold`: - Trust check (`isTrustedUser`) before any mutation — ✅ - `probeToken()` for the PATCH — consistent with how `handleIssueClosed`'s propagation gets its token — ✅ - Returns `null` regardless of `applyUnassignCommand`'s boolean return — correct; the boolean matters for the propagator's comment-sequencing, not for the webhook response --- ### `applyUnassignCommand` (`deps.ts`) Return semantics are correct: returns `true` when the PATCH succeeded (assignees cleared), even if the ack comment subsequently fails — because the authoritative state is the Forgejo issue, not the comment. Callers can use the boolean to decide whether to trust that the clear landed. --- ### Test coverage `deps.test.ts` additions are thorough: - Unassigned-ready → auto-assign PATCH + audit comment (verifies `onAssignedReady` does NOT fire) - PATCH failure → no audit comment, no dispatch - Duplicate delivery → routes to assigned branch, no second PATCH, no second comment - `parseUnassignCommand`: first-token-prefix, case-insensitive, mid-line negative - `applyUnassignCommand`: success path, PATCH-failure path (ack skipped) `webhook-handlers.test.ts`: trusted user gets PATCH + ack; untrusted user is a no-op (no PATCH). Correct trust-gate coverage. --- ### Minor observations (no action needed) - `UNASSIGN_ACK` message "Re-add a blocker + close it to re-trigger auto-assign" is technically accurate but slightly awkward — it means re-open + re-close an existing one, or add a new one. Acceptable for operator tooling at this scale. - `AUTO_ASSIGN_COMMENT_MARKER` exported as a stable string for future dashboard/scraper use — good forward-thinking. - `forgejo-api.ts` change is a pure docstring update to `listIssueComments` adding `/unassign` to the consumer list — ✅ no logic change. - The live canary (unchecked in the test plan) is expected — it can only be verified post-merge in production. **LGTM. Clean policy pivot with correct failure sequencing and solid test coverage.**

code-lead merged commit b0beefc88c into main

2026-04-21 00:04:03 +00:00

code-lead deleted branch boss/200

2026-04-21 00:04:04 +00:00

code-lead referenced this pull request from a commit

2026-04-21 00:04:05 +00:00

feat(deps): auto-assign unassigned-ready dependents + /unassign escape hatch (#201)