diff --git a/CONTEXT.md b/CONTEXT.md index 5055bb85a..1b414ce80 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -138,3 +138,44 @@ All workflow file names use hyphens; `` attributes inside those ### "Follow the X workflow" prose fragments are non-standard — use "Execute end-to-end." After stripping prose @-refs, some command `` blocks retained bolded "**Follow the X workflow**" fragments. ADR-0002 standard is `Execute end-to-end.` for single-workflow commands. Routing commands with flag dispatch use `execute the X workflow end-to-end.` in routing bullets (no bold, no redundant path). + +--- + +## Recurring CodeRabbit review patterns (2026-05-05, PRs #3152/#3154/#3155) + +### Changeset metadata drift (`pr:` points at issue instead of PR) +- In `.changeset/*.md`, reviewers repeatedly flag `pr:` values that accidentally reference issue ids. +- **Rule**: `pr:` must equal the GitHub PR number carrying the change. +- **Pre-flight check**: before push, verify each new changeset file against current branch PR number. + +### Test diagnostics quality for command-output parsing +- Even when behavior is correct, CR requests clearer failure surfaces before `.map()` on parsed output. +- **Rule**: after `JSON.parse`, assert output object shape (e.g., `Array.isArray(output.phases)`) with raw-output-prefix diagnostics. +- This prevents opaque `TypeError` failures and shortens triage loops when CLI output shape changes. + +### Merge gate discipline: CodeRabbit pass is necessary but not sufficient +- CI/checks can be green while unresolved review threads still block clean merge policy. +- **Rule**: always gate on all three together: required checks green, CodeRabbit pass, unresolved thread count = 0. +- Keep using GraphQL `reviewThreads` as authoritative unresolved state, not summary comments/check badge alone. + +--- + +## SDK Runtime Bridge review synthesis (PR #3158, 2026-05-05) + +### What we fixed +- Deepened one **SDK Runtime Bridge Module** seam (`sdk/src/query-runtime-bridge.ts`) for dispatch routing and observability. +- Replaced orphan event typing with a canonical union (`RuntimeBridgeEvent`). +- Made bridge observability non-intrusive: `onDispatchEvent` now runs behind a safe emitter so callback failures cannot alter dispatch outcomes. +- Corrected strict-mode event semantics: strict native-adapter rejection now reports `dispatchMode: 'native'` (no fake subprocess attempt). +- Preserved execution policy defaults by passing `allowFallbackToSubprocess` through as `undefined` when unset (no forced override in `GSDTools`). +- Fixed transport decision ordering: fallback-disabled guard now throws before emitting subprocess decision events. +- Added explicit invariant in `subprocessReason` for impossible states (fail loud on contract drift). +- Updated user-facing docs (`README.md`, `docs/CLI-TOOLS.md`, `docs/ARCHITECTURE.md`) and ADR narrative consistency. + +### What we should not do again +- Do not let observability callbacks sit on the critical path without isolation. +- Do not emit structured events that claim a transport mode that never happened. +- Do not force option defaults at call sites when policy Modules already define defaults. +- Do not keep duplicate/inert exported types; expose one canonical union Interface. +- Do not emit decision events before guard checks that may reject the path. +- Do not leave architectural docs with ambiguous seam ownership between CLI and SDK paths. diff --git a/README.md b/README.md index 4670edbb5..e08f19ba9 100644 --- a/README.md +++ b/README.md @@ -215,6 +215,7 @@ For the full configuration reference — all settings, git branching strategies, | [Commands](docs/COMMANDS.md) | Every command with flags and examples | | [Configuration](docs/CONFIGURATION.md) | Full config schema, model profiles, git branching | | [Architecture](docs/ARCHITECTURE.md) | How the multi-agent orchestration works | +| [CLI Tools](docs/CLI-TOOLS.md) | `gsd-sdk query` and programmatic SDK dispatch seams | | [Features](docs/FEATURES.md) | Complete feature index | | [Changelog](CHANGELOG.md) | What changed in each release | diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 4bb55b400..6a28c605f 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -55,7 +55,7 @@ GSD is a **meta-prompting framework** that sits between the user and AI coding a ┌──────▼──────────────▼─────────────────▼──────────────┐ │ CLI TOOLS LAYER │ │ gsd-sdk query (sdk/src/query) + gsd-tools.cjs │ -│ SDK Runtime Bridge Module routes native vs fallback │ +│ Programmatic SDK bridge: GSDTools/query-runtime-bridge.ts │ └──────────────────────┬───────────────────────────────┘ │ ┌──────────────────────▼───────────────────────────────┐ diff --git a/docs/CLI-TOOLS.md b/docs/CLI-TOOLS.md index c21a84a76..06bbbe428 100644 --- a/docs/CLI-TOOLS.md +++ b/docs/CLI-TOOLS.md @@ -62,7 +62,7 @@ Use this when authoring workflows, not when you only need the command list below | `node gsd-tools.cjs roadmap analyze` | `gsd-sdk query roadmap analyze` | -**SDK state reads:** `gsd-sdk query state json` / `state.json` and `gsd-sdk query state load` / `state.load` currently share one native handler (rebuilt STATE.md frontmatter — CJS `cmdStateJson`). The legacy CJS `state load` payload (`config`, `state_raw`, existence flags) is still **CLI-only** via `node …/gsd-tools.cjs state load` until a separate registry handler exists. Full routing and golden rules: [QUERY-HANDLERS.md](../sdk/src/query/QUERY-HANDLERS.md). +**SDK state reads:** `state.json` and `state.load` are both registered query handlers with parity coverage. You can invoke them through `gsd-sdk query …` and through the SDK Runtime Bridge (`GSDTools` → `sdk/src/query-runtime-bridge.ts`), honoring `allowFallbackToSubprocess` / `strictSdk` and emitting `onDispatchEvent` observability. For direct typed dispatch, use `createRegistry()` from `sdk/src/query/index.ts`. Full routing and golden rules: [QUERY-HANDLERS.md](../sdk/src/query/QUERY-HANDLERS.md). **CLI-only (not in registry):** e.g. **graphify**, **from-gsd2** / **gsd2-import** — call `gsd-tools.cjs` until registered. diff --git a/sdk/src/gsd-tools.ts b/sdk/src/gsd-tools.ts index d8c37fcf3..be721e17f 100644 --- a/sdk/src/gsd-tools.ts +++ b/sdk/src/gsd-tools.ts @@ -78,7 +78,7 @@ export class GSDTools { execJsonFallback: (legacyCommand, legacyArgs) => this.exec(legacyCommand, legacyArgs), execRawFallback: (legacyCommand, legacyArgs) => this.execRaw(legacyCommand, legacyArgs), strictSdk: opts.strictSdk, - allowFallbackToSubprocess: opts.allowFallbackToSubprocess ?? false, + allowFallbackToSubprocess: opts.allowFallbackToSubprocess, onDispatchEvent: opts.onDispatchEvent, }); diff --git a/sdk/src/gsd-transport.ts b/sdk/src/gsd-transport.ts index eecea7238..26f436b66 100644 --- a/sdk/src/gsd-transport.ts +++ b/sdk/src/gsd-transport.ts @@ -54,7 +54,6 @@ export class GSDTransport { } } else { const reason = this.subprocessReason(request, policy); - onDecision?.({ dispatchMode: 'subprocess', reason }); if (!policy.allowFallbackToSubprocess && reason === 'native_unregistered') { throw GSDToolsError.failure( `Subprocess fallback disabled: command '${request.registryCommand}' cannot run without native dispatch`, @@ -63,6 +62,7 @@ export class GSDTransport { null, ); } + onDecision?.({ dispatchMode: 'subprocess', reason }); } return this.dispatchSubprocess(request); @@ -77,7 +77,10 @@ export class GSDTransport { if (request.workstream) return 'workstream_forced'; if (!policy.preferNative) return 'native_not_preferred'; if (!this.registry.has(request.registryCommand)) return 'native_unregistered'; - return 'native_not_preferred'; + + throw new Error( + `Unexpected subprocess reason state for command '${request.registryCommand}' with preferNative=${String(policy.preferNative)} and workstream=${String(request.workstream)}`, + ); } private shouldRethrowNativeError(error: unknown, policy: TransportPolicyLike): boolean { diff --git a/sdk/src/query-runtime-bridge.ts b/sdk/src/query-runtime-bridge.ts index e1875cf5f..5011d4421 100644 --- a/sdk/src/query-runtime-bridge.ts +++ b/sdk/src/query-runtime-bridge.ts @@ -40,14 +40,12 @@ export interface RuntimeBridgeHotpathEvent { errorKind?: 'timeout' | 'failure'; } -export interface RuntimeBridgeEvent { - type: 'query_dispatch' | 'query_hotpath_dispatch'; -} +export type RuntimeBridgeEvent = RuntimeBridgeDispatchEvent | RuntimeBridgeHotpathEvent; export interface RuntimeBridgeOptions { strictSdk?: boolean; allowFallbackToSubprocess?: boolean; - onDispatchEvent?: (event: RuntimeBridgeDispatchEvent | RuntimeBridgeHotpathEvent) => void; + onDispatchEvent?: (event: RuntimeBridgeEvent) => void; } /** @@ -71,6 +69,14 @@ export class QueryRuntimeBridge { return resolveQueryCommand(command, args, this.registry); } + private emit(event: RuntimeBridgeEvent): void { + try { + this.options?.onDispatchEvent?.(event); + } catch { + // Observability must never break dispatch behavior. + } + } + async execute(input: RuntimeBridgeExecuteInput): Promise { const startedAt = Date.now(); if (this.options?.strictSdk && !this.registry.has(input.registryCommand)) { @@ -80,12 +86,12 @@ export class QueryRuntimeBridge { input.legacyArgs, null, ); - this.options?.onDispatchEvent?.({ + this.emit({ type: 'query_dispatch', command: input.registryCommand, legacyCommand: input.legacyCommand, mode: input.mode, - dispatchMode: 'subprocess', + dispatchMode: 'native', reason: 'native_unregistered', durationMs: Date.now() - startedAt, outcome: 'error', @@ -111,7 +117,7 @@ export class QueryRuntimeBridge { }, }); - this.options?.onDispatchEvent?.({ + this.emit({ type: 'query_dispatch', command: input.registryCommand, legacyCommand: input.legacyCommand, @@ -124,7 +130,7 @@ export class QueryRuntimeBridge { return result; } catch (error) { const kind = error instanceof GSDToolsError ? error.classification.kind : 'failure'; - this.options?.onDispatchEvent?.({ + this.emit({ type: 'query_dispatch', command: input.registryCommand, legacyCommand: input.legacyCommand, @@ -155,7 +161,7 @@ export class QueryRuntimeBridge { registryArgs, mode, ); - this.options?.onDispatchEvent?.({ + this.emit({ type: 'query_hotpath_dispatch', command: registryCommand, legacyCommand, @@ -167,7 +173,7 @@ export class QueryRuntimeBridge { return result; } catch (error) { const kind = error instanceof GSDToolsError ? error.classification.kind : 'failure'; - this.options?.onDispatchEvent?.({ + this.emit({ type: 'query_hotpath_dispatch', command: registryCommand, legacyCommand,