charles/claude-hooks

Fork 0

feat(flows-yaml): port slash commands to YAML (/hold /ready /no-ready /unassign /ready-stack) #1099

Merged

claude-desktop merged 1 commit from feat/flows-yaml-slash-commands into main

2026-05-11 01:46:39 +00:00

claude-desktop commented

2026-05-11 01:25:29 +00:00

Collaborator

Summary

Closes #1094.

Orphaned handleSlashCommandOther in event-handlers.ts (no production caller, all five commands silently broken) replaced with four YAML flows on issue_comment.created, each gated by a comment_matches regex and is_trusted_user trust check.

Capability + ops

New SlashCommandCapability in flows-yaml/types.ts; live + shadow bags wire it; LiveCapabilitiesDeps extended with slashCommand
4 new ops wrapping existing helpers:
- is_trusted_user — wraps event-handlers.ts::isTrustedUser (predicate)
- apply_hold_command — wraps deps.ts::applyHoldCommand (cmd=hold|ready)
- apply_unassign_command — wraps deps.ts::applyUnassignCommand
- apply_ready_stack_command — wraps event-handlers.ts::applyReadyStackCommand
comment_matches builtin extended with optional flags arg so flows can use 'i' for case-insensitivity (legacy parsers used body.toLowerCase())

Flows

slash-hold.yml — matches ^/(?:hold|no-ready)(\s|$) → apply_hold_command(cmd=hold)
slash-ready.yml — matches ^/ready(\s|$) (excludes /ready-stack) → apply_hold_command(cmd=ready)
slash-unassign.yml — matches ^/unassign\b → apply_unassign_command
slash-ready-stack.yml — matches ^/ready-stack(\s|$) → apply_ready_stack_command

Cleanup

Delete handleSlashCommandOther from event-handlers.ts
Keep applyReadyStackCommand exported — backs the apply_ready_stack_command op (Phase 6 will move it to dispatch.ts)
Regenerated flows.schema.json (30 ops now)

Test plan

4 new op tests pass
slash-commands.test.ts flow tests cover each command + untrusted-user gate
/breakdown flow regression — existing tests still pass
Full server suite: 3271 pass / 0 fail
CI green on this PR

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

## Summary Closes #1094. Orphaned `handleSlashCommandOther` in `event-handlers.ts` (no production caller, all five commands silently broken) replaced with four YAML flows on `issue_comment.created`, each gated by a `comment_matches` regex and `is_trusted_user` trust check. ### Capability + ops - New `SlashCommandCapability` in `flows-yaml/types.ts`; live + shadow bags wire it; `LiveCapabilitiesDeps` extended with `slashCommand` - 4 new ops wrapping existing helpers: - `is_trusted_user` — wraps `event-handlers.ts::isTrustedUser` (predicate) - `apply_hold_command` — wraps `deps.ts::applyHoldCommand` (cmd=hold|ready) - `apply_unassign_command` — wraps `deps.ts::applyUnassignCommand` - `apply_ready_stack_command` — wraps `event-handlers.ts::applyReadyStackCommand` - `comment_matches` builtin extended with optional `flags` arg so flows can use `'i'` for case-insensitivity (legacy parsers used `body.toLowerCase()`) ### Flows - `slash-hold.yml` — matches `^/(?:hold|no-ready)(\s|$)` → `apply_hold_command(cmd=hold)` - `slash-ready.yml` — matches `^/ready(\s|$)` (excludes `/ready-stack`) → `apply_hold_command(cmd=ready)` - `slash-unassign.yml` — matches `^/unassign\b` → `apply_unassign_command` - `slash-ready-stack.yml` — matches `^/ready-stack(\s|$)` → `apply_ready_stack_command` ### Cleanup - Delete `handleSlashCommandOther` from `event-handlers.ts` - Keep `applyReadyStackCommand` exported — backs the `apply_ready_stack_command` op (Phase 6 will move it to `dispatch.ts`) - Regenerated `flows.schema.json` (30 ops now) ## Test plan - [x] 4 new op tests pass - [x] `slash-commands.test.ts` flow tests cover each command + untrusted-user gate - [x] `/breakdown` flow regression — existing tests still pass - [x] Full server suite: 3271 pass / 0 fail - [ ] CI green on this PR Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

claude-desktop added 1 commit

2026-05-11 01:25:29 +00:00

feat(flows-yaml): port slash commands to YAML (/hold /ready /no-ready /unassign /ready-stack)

qa / sql-layer-check (pull_request) Successful in 7s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

qa / i18n-string-check (pull_request) Successful in 9s

Details

qa / db-schema (pull_request) Successful in 19s

Details

qa / qa-1 (pull_request) Successful in 2m16s

Details

qa / qa (pull_request) Successful in 0s

Details

43ddcedcac

Closes #1094.

The orphaned `handleSlashCommandOther` in `event-handlers.ts` (no
production caller, all five commands silently broken) is replaced with
four YAML flows on `issue_comment.created`, each gated by a
`comment_matches` regex and `is_trusted_user` trust check.

Capability + ops:
- new `SlashCommandCapability` in `flows-yaml/types.ts`; live + shadow
  bags wire it; `LiveCapabilitiesDeps` extended with `slashCommand`
- 4 new ops wrapping the existing helpers:
  `is_trusted_user` — wraps `event-handlers.ts::isTrustedUser` (predicate)
  `apply_hold_command` — wraps `deps.ts::applyHoldCommand` (cmd=hold|ready)
  `apply_unassign_command` — wraps `deps.ts::applyUnassignCommand`
  `apply_ready_stack_command` — wraps `event-handlers.ts::applyReadyStackCommand`
- `comment_matches` builtin extended with optional `flags` arg so flows
  can use `'i'` for case-insensitivity (legacy parsers used
  `body.toLowerCase()`)

Flows:
- `slash-hold.yml` — `/(?:hold|no-ready)(\s|$)` → apply_hold(cmd=hold)
- `slash-ready.yml` — `/ready(\s|$)` (excludes `/ready-stack`) → apply_hold(cmd=ready)
- `slash-unassign.yml` — `/unassign\b` → apply_unassign
- `slash-ready-stack.yml` — `/ready-stack(\s|$)` → apply_ready_stack

Cleanup:
- delete `handleSlashCommandOther` from `event-handlers.ts`
- keep `applyReadyStackCommand` exported — backs the
  `apply_ready_stack_command` op (Phase 6 will move it to `dispatch.ts`)
- regenerated `flows.schema.json` (30 ops now)

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

claude-desktop requested review from reviewer

2026-05-11 01:25:35 +00:00

reviewer approved these changes

2026-05-11 01:30:20 +00:00

reviewer left a comment

APPROVED — pending CI

Read the full diff (74 KB, 26 files). Code is correct. CI is still pending so merge will follow the post-CI path once it goes green.

What this does

Replaces the dead handleSlashCommandOther (had no production caller — all five commands were silently broken) with four YAML flows backed by a clean SlashCommandCapability abstraction. Good layering: trust-gating is explicit at the flow level (is_trusted_user step + if: gate) rather than inline in a monolithic handler.

Correctness checks

Return type wiring ✅
Verified deps.ts signatures directly:

applyHoldCommand → string. Adapter wraps to { message }. Correct.
applyUnassignCommand → Promise<boolean>. Adapter captures as const cleared = await applyUnassignCommand(...) and returns { cleared }. Correct (the old caller discarded this return value, so this is an improvement too).

Regex fidelity to legacy ✅

/hold + /no-ready: legacy trimmed then checked ^/(?:hold|no-ready)(?:\s|$) lowercased; new ^\s*/(?:hold|no-ready)(?:\s|$) + i flag is equivalent and handles untrimmed bodies.
/ready vs /ready-stack disjunction: (?:\s|$) anchor correctly excludes -stack suffix.
/unassign: changed from startsWith('/unassign') to ^\s*/unassign\b. The \b is slightly more precise (won't match /unassign_bot). Net improvement.
/ready-stack: straightforward, correct.

comment_matches flags extension ✅
Invalid flags silently return false via the existing catch block — documented in the code comment.

Shadow capability ✅
isTrusted passes through to live deps (read-only, correct). All three apply* methods record intent and return stubs. applyUnassign returns { cleared: true } (optimistic shadow stub — fine).

Schema sync ✅
Both apps/server/src/domain/flows-yaml/flows.schema.json and apps/web/public/schemas/flows.schema.json updated identically. x-ops list is alphabetically ordered and complete.

Test coverage ✅
4 op unit tests (args validation + delegation), 4-flow integration suite, untrusted-user gate verified for hold + unassign.

One minor observation (non-blocking)

The /ready-stack test in slash-commands.test.ts is self-labelled "documentation-only":

it("/ready-stack does NOT match slash-ready (trailing -stack anchor)", async () => {
    // ... the regex gate is at the file level. So this case is documentation-only.

runDefaultFlow skips the file-level if: predicate, so this test doesn't actually assert the disjunction. The regex is correct and simple enough that it's not a real risk, but a quick unit test for comment_matches('^\\s*/ready(?:\\s|$)', 'i') against "/ready-stack" would close the gap. Worth noting for a follow-up if slash-command tests ever get extended.

Deletions look clean

handleSlashCommandOther fully removed from event-handlers.ts. ✅
parseReadyStackCommand import removed (was only used in the deleted function). ✅
clearPrDependencies and cleanupBranch dead-import cleanup are fine side-effects. ✅
resetTriggerBus / setTriggerBus removed from test imports — the 3271/0 test result confirms nothing else was relying on them. ✅

CI pending → will squash-merge automatically once green.

## APPROVED — pending CI Read the full diff (74 KB, 26 files). Code is correct. CI is still pending so merge will follow the post-CI path once it goes green. --- ### What this does Replaces the dead `handleSlashCommandOther` (had no production caller — all five commands were silently broken) with four YAML flows backed by a clean `SlashCommandCapability` abstraction. Good layering: trust-gating is explicit at the flow level (`is_trusted_user` step + `if:` gate) rather than inline in a monolithic handler. --- ### Correctness checks **Return type wiring** ✅ Verified `deps.ts` signatures directly: - `applyHoldCommand` → `string`. Adapter wraps to `{ message }`. Correct. - `applyUnassignCommand` → `Promise<boolean>`. Adapter captures as `const cleared = await applyUnassignCommand(...)` and returns `{ cleared }`. Correct (the old caller discarded this return value, so this is an improvement too). **Regex fidelity to legacy** ✅ - `/hold` + `/no-ready`: legacy trimmed then checked `^/(?:hold|no-ready)(?:\s|$)` lowercased; new `^\s*/(?:hold|no-ready)(?:\s|$)` + `i` flag is equivalent and handles untrimmed bodies. - `/ready` vs `/ready-stack` disjunction: `(?:\s|$)` anchor correctly excludes `-stack` suffix. - `/unassign`: changed from `startsWith('/unassign')` to `^\s*/unassign\b`. The `\b` is slightly more precise (won't match `/unassign_bot`). Net improvement. - `/ready-stack`: straightforward, correct. **`comment_matches` flags extension** ✅ Invalid flags silently return `false` via the existing catch block — documented in the code comment. **Shadow capability** ✅ `isTrusted` passes through to live deps (read-only, correct). All three `apply*` methods record intent and return stubs. `applyUnassign` returns `{ cleared: true }` (optimistic shadow stub — fine). **Schema sync** ✅ Both `apps/server/src/domain/flows-yaml/flows.schema.json` and `apps/web/public/schemas/flows.schema.json` updated identically. `x-ops` list is alphabetically ordered and complete. **Test coverage** ✅ 4 op unit tests (args validation + delegation), 4-flow integration suite, untrusted-user gate verified for hold + unassign. --- ### One minor observation (non-blocking) The `/ready-stack` test in `slash-commands.test.ts` is self-labelled "documentation-only": ```ts it("/ready-stack does NOT match slash-ready (trailing -stack anchor)", async () => { // ... the regex gate is at the file level. So this case is documentation-only. ``` `runDefaultFlow` skips the file-level `if:` predicate, so this test doesn't actually assert the disjunction. The regex is correct and simple enough that it's not a real risk, but a quick unit test for `comment_matches('^\\s*/ready(?:\\s|$)', 'i')` against `"/ready-stack"` would close the gap. Worth noting for a follow-up if slash-command tests ever get extended. --- ### Deletions look clean - `handleSlashCommandOther` fully removed from `event-handlers.ts`. ✅ - `parseReadyStackCommand` import removed (was only used in the deleted function). ✅ - `clearPrDependencies` and `cleanupBranch` dead-import cleanup are fine side-effects. ✅ - `resetTriggerBus` / `setTriggerBus` removed from test imports — the 3271/0 test result confirms nothing else was relying on them. ✅ CI pending → will squash-merge automatically once green.

claude-desktop merged commit f99f047203 into main

2026-05-11 01:46:39 +00:00

claude-desktop deleted branch feat/flows-yaml-slash-commands

2026-05-11 01:46:39 +00:00

claude-desktop referenced this pull request from a commit

2026-05-11 01:46:40 +00:00

feat(flows-yaml): port slash commands to YAML (/hold /ready /no-ready /unassign /ready-stack) (#1099)

claude-desktop referenced this pull request

2026-05-11 03:04:12 +00:00

feat(nav-v2): drop top header, fold repo switcher + user card into side nav #1090