Skip to content

Commit 33132a3

Browse files
chrisleekrclaude
andauthored
feat(ship): scoped commands (US5) + remove SHIP_USE_TRIGGER_SURFACES_V2 flag (#77)
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 928811b commit 33132a3

46 files changed

Lines changed: 5150 additions & 305 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

docs/BOT-WORKFLOWS.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,12 +152,12 @@ Composite workflows like `ship` insert a child row per step. When the child comp
152152

153153
### ship (PR shepherding lifecycle, `ship_intents`)
154154

155-
A separate, newer lifecycle layered on top of the composite handler**flag-gated** behind `SHIP_USE_TRIGGER_SURFACES_V2`, `SHIP_USE_PROBE_VERDICT`, and `SHIP_USE_CONTINUATION_LOOP`. Default off; the composite path above is unchanged when these flags are unset. See [`docs/SHIP.md`](SHIP.md) for the operator-facing summary.
155+
A separate, newer lifecycle layered on top of the composite handler. The probe-verdict ladder and continuation-loop architecture remain **flag-gated** behind `SHIP_USE_PROBE_VERDICT` and `SHIP_USE_CONTINUATION_LOOP` (default off; the composite path above is unchanged when these flags are unset). The three trigger surfaces — literal, natural-language, and label — are permanent v1 and require no flag. See [`docs/SHIP.md`](SHIP.md) for the operator-facing summary.
156156

157157
- **State**: rows in `ship_intents` (status: `active` | `paused` | `merged_externally` | `ready_awaiting_human_merge` | `deadline_exceeded` | `human_took_over` | `aborted_by_user` | `pr_closed`). Wake events queued in Valkey `ship:tickle`. Cancellation flag at `ship:cancel:{intent_id}`.
158158
- **Three trigger surfaces (FR-027)** — all functionally equivalent, normalised to a single `CanonicalCommand`:
159-
1. **Literal**: `bot:ship` (or `bot:ship --deadline 2h`) PR comment. Deterministic regex parser. Available without the v2 flag.
160-
2. **Natural language**: `@chrisleekr-bot ship this please`. Mention-prefix-gated NL classifier (FR-025a) — zero LLM cost on comments without the mention. Bedrock single-turn classification.
159+
1. **Literal**: `bot:ship` (or `bot:ship --deadline 2h`) PR comment. Deterministic regex parser. Permanent surface.
160+
2. **Natural language**: `@chrisleekr-bot ship this please`. Mention-prefix-gated NL classifier (FR-025a) — zero LLM cost on comments without the mention. Bedrock single-turn classification. Permanent surface.
161161
3. **Label**: apply `bot:ship` (or `bot:ship/deadline=2h`). Bot self-removes the label after acting (FR-026a). Re-application is the supported re-trigger mechanism.
162162
- **Lifecycle commands** (same three surfaces): `bot:stop` / `bot:resume` / `bot:abort-ship`.
163163
- **Reactor (T023-T027)**: `pull_request.{synchronize,closed}`, `pull_request_review.submitted`, `pull_request_review_comment.{created,edited,deleted}`, `check_run.completed`, `check_suite.completed` early-wake any active intent on the affected PR via Valkey `ZADD ship:tickle 0 <intent_id>`. Reactor on `synchronize` from a non-bot pusher transitions to terminal `human_took_over` + `manual-push-detected` (FR-010). Reactor on `pull_request.closed` transitions to `merged_externally` or `pr_closed`.

docs/CONFIGURATION.md

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -113,20 +113,19 @@ See [Triage](TRIAGE.md) for the binary `heavy` signal, circuit breaker, and the
113113

114114
The new shepherding lifecycle (intents tracked in `ship_intents`) is gated by feature flags so it can be rolled out incrementally per `research.md` R8.
115115

116-
| Variable | Default | Notes |
117-
| --------------------------------- | --------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
118-
| `MAX_WALL_CLOCK_PER_SHIP_RUN` | `4h` | Hard ceiling on a single intent's wall-clock budget. Accepts integer ms or duration suffix (`4h` / `30m` / `90s`). Per-invocation `--deadline` is clamped to this ceiling. |
119-
| `MAX_SHIP_ITERATIONS` | `50` | Iteration cap (FR-012). Checked at the start of each iteration; firing transitions the intent to terminal `human_took_over` + `BlockerCategory='iteration-cap'`. |
120-
| `CRON_TICKLE_INTERVAL_MS` | `30000` | How often the cron tickle scans `ship:tickle` for due intents. Lower = faster wake but more Valkey traffic. |
121-
| `MERGEABLE_NULL_BACKOFF_MS_LIST` | `500,1500,4500` | Comma-separated bounded backoff schedule used by `runProbe` when `mergeable=null`. Per FR-021, exhaustion yields `mergeable_pending` and the session yields rather than spinning. |
122-
| `REVIEW_BARRIER_SAFETY_MARGIN_MS` | `1200000` | (20 min) FR-023: minimum elapsed time since last bot push before the bot may declare `ready` without a non-bot review on the current head SHA. |
123-
| `FIX_ATTEMPTS_PER_SIGNATURE_CAP` | `3` | FR-013: maximum attempts per failure signature within a single intent. Cap firing terminates the intent with `BlockerCategory='flake-cap'`. |
124-
| `SHIP_FORBIDDEN_TARGET_BRANCHES` | empty | Comma-separated branch names (e.g., `main,production,release`) that the bot refuses to shepherd PRs against. Per FR-015 refusal case 4. |
125-
| `SHIP_USE_PROBE_VERDICT` | `false` | Rollout flag — when `true`, terminal-readiness uses the new GraphQL probe verdict ladder. Default off keeps the legacy in-process loop in charge. |
126-
| `SHIP_USE_CONTINUATION_LOOP` | `false` | Rollout flag — when `true`, the iteration loop exits after each phase and re-enters via the cron tickle (Valkey `ship:tickle`). Restart-safe and slot-friendly. |
127-
| `SHIP_USE_TRIGGER_SURFACES_V2` | `false` | Rollout flag — when `true`, the natural-language and label trigger surfaces (FR-025/025a/026/026a/027) are active alongside the legacy literal-comment surface. Default off preserves today's behaviour with zero LLM cost. |
128-
129-
After a one-week soak with all three flags set to `true` and clean operation observed, a follow-up PR removes the flags and the legacy code paths (research.md R8 cutover plan, T071).
116+
| Variable | Default | Notes |
117+
| --------------------------------- | --------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
118+
| `MAX_WALL_CLOCK_PER_SHIP_RUN` | `4h` | Hard ceiling on a single intent's wall-clock budget. Accepts integer ms or duration suffix (`4h` / `30m` / `90s`). Per-invocation `--deadline` is clamped to this ceiling. |
119+
| `MAX_SHIP_ITERATIONS` | `50` | Iteration cap (FR-012). Checked at the start of each iteration; firing transitions the intent to terminal `human_took_over` + `BlockerCategory='iteration-cap'`. |
120+
| `CRON_TICKLE_INTERVAL_MS` | `30000` | How often the cron tickle scans `ship:tickle` for due intents. Lower = faster wake but more Valkey traffic. |
121+
| `MERGEABLE_NULL_BACKOFF_MS_LIST` | `500,1500,4500` | Comma-separated bounded backoff schedule used by `runProbe` when `mergeable=null`. Per FR-021, exhaustion yields `mergeable_pending` and the session yields rather than spinning. |
122+
| `REVIEW_BARRIER_SAFETY_MARGIN_MS` | `1200000` | (20 min) FR-023: minimum elapsed time since last bot push before the bot may declare `ready` without a non-bot review on the current head SHA. |
123+
| `FIX_ATTEMPTS_PER_SIGNATURE_CAP` | `3` | FR-013: maximum attempts per failure signature within a single intent. Cap firing terminates the intent with `BlockerCategory='flake-cap'`. |
124+
| `SHIP_FORBIDDEN_TARGET_BRANCHES` | empty | Comma-separated branch names (e.g., `main,production,release`) that the bot refuses to shepherd PRs against. Per FR-015 refusal case 4. |
125+
| `SHIP_USE_PROBE_VERDICT` | `false` | Rollout flag — when `true`, terminal-readiness uses the new GraphQL probe verdict ladder. Default off keeps the legacy in-process loop in charge. |
126+
| `SHIP_USE_CONTINUATION_LOOP` | `false` | Rollout flag — when `true`, the iteration loop exits after each phase and re-enters via the cron tickle (Valkey `ship:tickle`). Restart-safe and slot-friendly. |
127+
128+
The natural-language and label trigger surfaces (FR-025/025a/026/026a/027) are permanent — there is no flag gating them. After a one-week soak with the two probe/continuation flags set to `true` and clean operation observed, a follow-up PR removes those two flags and the legacy code paths (research.md R8 cutover plan, T071).
130129

131130
## Composite ship workflow
132131

docs/SETUP.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ After you set permissions, the **Subscribe to events** section becomes available
226226
| **Check runs** | `check_run.completed` (powers the `bot:ship` reactor — early-wakes active intents on CI completion) | `src/webhook/events/check-run.ts` |
227227
| **Check suites** | `check_suite.completed` | `src/webhook/events/check-suite.ts` |
228228

229-
The PR shepherding reactor uses the new `synchronize`, `closed`, `edited`, `deleted`, `check_run`, and `check_suite` subscriptions to early-wake active sessions. `SHIP_USE_TRIGGER_SURFACES_V2=true` gates the **trigger-surface** wiring (the new literal/NL/label trigger router) — **not** the reactor wake subscriptions, which must be registered regardless. Existing `bot:ship` (composite) operation does not require the new wake subscriptions.
229+
The PR shepherding reactor uses the new `synchronize`, `closed`, `edited`, `deleted`, `check_run`, and `check_suite` subscriptions to early-wake active sessions. The literal/NL/label trigger surfaces are all permanent v1 features and require no flag gating. Existing `bot:ship` (composite) operation does not require the new wake subscriptions.
230230

231231
The bot also recognises four GitHub labels on PRs (FR-026): `bot:ship`, `bot:stop`, `bot:resume`, `bot:abort-ship`, plus the suffix-overridden variants `bot:ship/deadline=2h` etc. The bot self-removes the label after acting (FR-026a) — re-application is the supported re-trigger mechanism.
232232

docs/SHIP.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ There are three functionally equivalent surfaces (FR-027). All three produce the
1212
| Natural | `@chrisleekr-bot ship this please` | Mention-prefix-gated NL classifier (FR-025a). Zero LLM cost on comments without the mention. |
1313
| Label | Apply the `bot:ship` label _(or `bot:ship/deadline=2h`)_ | Bot self-removes the label after acting (FR-026a). Re-application is the supported re-trigger mechanism. |
1414

15-
The natural-language and label surfaces are gated on `SHIP_USE_TRIGGER_SURFACES_V2=true`. The literal `bot:ship` surface works regardless.
15+
All three surfaces — literal, natural-language, and label are permanent v1 features. The natural-language path costs nothing on comments without the `TRIGGER_PHRASE` mention (FR-025a gate runs before the LLM call).
1616

1717
The four recognised verbs (each available across all three surfaces): `ship`, `stop`, `resume`, `abort-ship`. See `contracts/bot-commands.md` for full syntax.
1818

@@ -36,6 +36,6 @@ Abort sets a Valkey cancellation flag, waits ≤2 s for the next cooperative che
3636

3737
For a recoverable pause, use `bot:stop` (and later `bot:resume`) instead. A stopped session preserves its continuation row; the deadline keeps counting down while paused.
3838

39-
## Rollout flag
39+
## Rollout flags
4040

41-
`SHIP_USE_TRIGGER_SURFACES_V2=true` activates the NL + label surfaces. After one week of clean soak, a follow-up PR removes the flag (research.md R8). Set this in the orchestrator's environment, not the daemon's — the trigger surfaces are dispatched from the webhook server.
41+
The remaining v1 rollout flags are `SHIP_USE_PROBE_VERDICT` (probe-verdict ladder vs. legacy in-process review/resolve loop) and `SHIP_USE_CONTINUATION_LOOP` (cron-tickle re-entry vs. in-process loop). Both default off; flip on after the corresponding soak per research.md R8.

eslint.config.mjs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,42 @@ export default tseslint.config(
130130
"no-debugger": "error",
131131
},
132132
},
133+
{
134+
// T087 — `bot:triage` (FR-034) is suggest-only in v1. The handler
135+
// file MUST NOT import or invoke any GitHub mutation that could
136+
// change issue state. Linting backs the intent at edit time;
137+
// `test/workflows/ship/scoped/triage.test.ts` backs it at runtime.
138+
files: ["src/workflows/ship/scoped/triage.ts"],
139+
rules: {
140+
"no-restricted-syntax": [
141+
"error",
142+
{
143+
// REST: forbid the named octokit.rest.issues.* mutation methods.
144+
selector:
145+
"CallExpression[callee.property.name=/^(addLabels|removeLabel|removeAllLabels|setLabels|update|lock|unlock|addAssignees|removeAssignees|pin|unpin)$/]",
146+
message:
147+
"bot:triage v1 is suggest-only — issue mutation methods are forbidden in this file (FR-034 / T087).",
148+
},
149+
{
150+
// GraphQL via plain string: forbid mutation names in the
151+
// string-literal first arg of `octokit.graphql(...)`.
152+
selector:
153+
"CallExpression[callee.property.name='graphql'] Literal[value=/\\b(addLabelsToLabelable|removeLabelsFromLabelable|closeIssue|lockLockable|unlockLockable|addAssigneesToAssignable|removeAssigneesFromAssignable|pinIssue|unpinIssue)\\b/]",
154+
message:
155+
"bot:triage v1 is suggest-only — forbidden issue GraphQL mutations are not allowed in this file (FR-034 / T087).",
156+
},
157+
{
158+
// GraphQL via template literal: same set of forbidden mutation
159+
// names appearing in any tagged-template part of an
160+
// `octokit.graphql(...)` call.
161+
selector:
162+
"CallExpression[callee.property.name='graphql'] TemplateElement[value.raw=/\\b(addLabelsToLabelable|removeLabelsFromLabelable|closeIssue|lockLockable|unlockLockable|addAssigneesToAssignable|removeAssigneesFromAssignable|pinIssue|unpinIssue)\\b/]",
163+
message:
164+
"bot:triage v1 is suggest-only — forbidden issue GraphQL mutations are not allowed in this file (FR-034 / T087).",
165+
},
166+
],
167+
},
168+
},
133169
{
134170
// Relaxed rules for test files
135171
files: ["**/*.test.ts", "**/test/**/*.ts"],

0 commit comments

Comments
 (0)