[02/36] Add OC-000 protocol dispatch foundation

[cjohnsto-nz]

---

This is part 20 of 21 in a stack made with GitButler:

•  21  [22/36] Release 0.8.0 with packaged PDF.js preview #54
•  20  [21/36] Complete OC-070 protocol surface polish #56
•  19  [20/36] Record final OpenCollection supervisor acceptance #53
•  18  [19/36] Add OC-110 runtime authoring UX #52
•  17  [18/36] Add OC-080 runtime lifecycle and OC-110 task setup #51
•  16  [16/36] Record OC-050 OC-090 OC-100 supervisor audit #50
•  15  [15/36] Add OC-050 auth and transport support #49
•  14  [14/36] Add OC-100 request type UX #48
•  13  [13/36] Add OC-100 request type UX task #47
•  12  [12/36] Add OC-080 and OC-090 task tracks #46
•  11  [11/36] Record OC-010 through OC-040 supervisor audit #45
•  10  [10/36] Add OC-020 WebSocket support #44
•  9  [09/36] Add OC-010 GraphQL support #43
•  8  [08/36] Require demo server fixtures for parallel tracks #42
•  7  [07/36] Record OC-000 and OC-060 supervisor audit #41
•  6  [06/36] Add OC-060 schema round-trip validation #40
•  5  [05/36] Add OC-040 runtime scripting support #39
•  4  [04/36] Add OC-090 gRPC streaming support #38
•  3  [03/36] Add OC-030 gRPC unary protobuf support #37
•  2  [02/36] Add OC-000 protocol dispatch foundation #36 <-- current
•  1  [01/36] Add OpenCollection agent plan #35

[APKiwi]

Verdict: request changes. One shipped crash plus foundation-level design notes.

• Critical: getUnsupportedSnippetDiagnostic does not exist. src/panels/requestPanel.ts:342 calls it in the exportPreview handler's non-HTTP branch, but the identifier is not defined or imported anywhere in the tree. It's a rename typo for getUnsupportedProtocolDiagnostic (exported at requestExecutionService.ts:286). Opening a GraphQL/gRPC/WebSocket request and triggering export preview throws a runtime ReferenceError instead of the graceful diagnostic. It ships because CI runs esbuild + vitest and never tsc: esbuild treats the free identifier as a global and bundles happily, while npm run compile (strict) would fail with TS2304. Two fixes: import the right function, and add the type-check to CI so this class of bug can't land again.
• Minor: discriminator precedence is inconsistent. isHttpRequest (models/types.ts:615) lets a stray http: key win, and getItemKind checks HTTP first, but getRequestValidationRoute checks the other protocols before http. A file with info.type: graphql plus a stray http: key validates as GraphQL yet routes/executes as HTTP. For the dispatch foundation, pick one authoritative precedence (info.type first, body key fallback) and share it.
• Minor: the RequestExecutor<T> interface and four executor aliases are exported but never used. The actual dispatch is a hardcoded if/else in send(). Either wire the registry now or drop the interface until a protocol needs it, exporting an unwired abstraction is misleading scaffolding.
• Minor, forward-looking: RequestExecutor.send returns Promise<HttpResponse> for every protocol. WebSocket and gRPC streaming don't reduce to one HttpResponse. Worth deciding the return shape now since 34 PRs build on this.
• Minor: send/retry/export now prefer re-parsed document text over the webview's posted msg.request (requestPanel.ts ~211/289/340). If the webview form can ever be ahead of the TextDocument at send time, the send silently uses stale content. Confirm edits are flushed before these actions fire.
• Minor: Copilot listRequests (listRequestsTool.ts:62) now silently omits non-HTTP requests entirely rather than listing them as unsupported. An agent browsing a mixed collection won't know they exist.
• Nit: saveDocument posts an explicit non-HTTP error while _applyDocumentEdit returns silently. Same guard, different UX.
• Nit: requestCommands.ts:146 misindented. readItemFile alias in yamlParser.ts is dead.

Tests: good, genuinely behavioral (guards, tree, delegation, validation routing, autosave guard). The one uncovered surface is export preview for non-HTTP, exactly where the Critical lives.

Deps: none added.

[cjohnsto-nz]

@APKiwi

• Centralised protocol classification so info.type is authoritative, with body-key fallback only when no protocol type is present. Guards, item routing, and validation now share the same precedence, with regression coverage for GraphQL requests containing stray HTTP data.
• Removed the unused RequestExecutor interface and executor aliases rather than preserving an unwired abstraction.

I did not duplicate the unsupported snippet diagnostic fix because it is already implemented in PR #51 (35bf2af), where the snippet-export diagnostic is introduced. The listRequests omission is similarly fixed in PR #43 (cc721db).

Cross-cutting CI and request-panel synchronisation follow-ups are being isolated on fix/open-collection-pr-rework to avoid changing stacked branches.

Validation: all 333 tests pass. The remaining compile error is the pre-existing snippet diagnostic reference in requestPanel.ts, resolved by the later PR #51 change.

[APKiwi]

Re-review (whole-stack pass, per REVIEW_GUIDE.md)

• getUnsupportedSnippetDiagnostic ReferenceError: still open. Called at src/panels/requestPanel.ts:342, defined nowhere on this head. Introduced by this PR's 0742dbc (absent on the base), so not pre-existing. Export-preview of any non-HTTP request throws, and npm run compile is red here. CI stays green because .github/workflows/pr.yml runs esbuild + vitest and never tsc, so neither the crash nor the type error is visible to checks. The fix (35bf2af) is real and verified but lands at order 18 ([18/36] Add OC-080 runtime lifecycle and OC-110 task setup #51), so title-order landing carries a runtime crash and a red compile on main from order 02 through 17.
• Discriminator precedence: fixed in a3cae8e, verified. getRequestProtocol is info.type-first with body-key fallback, and the guards and validation route all go through it.
• Unused RequestExecutor interface: the reply says removed, but a3cae8e removed it and ae8c54c restored it 19 minutes later. It is back on head with zero usages outside its defining file anywhere in the stack. Reads as merge hygiene (returns the file to the blob children were built on), not a live consumer.
• listRequests non-HTTP omission: fixed downstream in cc721db ([09/36] Add OC-010 GraphQL support #43), verified.
• CI type-check gap: still open, pr.yml unchanged. The "isolated on the fix branch" note has no corresponding commit yet.

New (low): a request file with only info: and no protocol key vanishes from the tree silently (collectionService.ts:539); tree vs getItemKind disagree on a request carrying a stray items array (collectionTreeProvider.ts:279); unknown info.type falls through to the HTTP validator.

Verdict: changes-needed. The shipped ReferenceError plus red compile, invisible to CI and deferred 16 PRs, is the blocker. Add tsc to CI so this class can't recur.

[cjohnsto-nz]

Response to the second review:

The getUnsupportedSnippetDiagnostic finding is correct for PR #36 in isolation: the call is present on ae8c54c and the implementation lands later in the stack at 35bf2af on PR #51. I am not backporting that change into #36 now. Rewriting this historical base branch already caused #37 to conflict once; the stack policy now requires corrections that are already present downstream to remain downstream unless every descendant can be restacked safely. The complete 37-PR composition contains the implementation and passes npm run compile.

The missing CI guard was still actionable. I added an explicit npm run compile step to .github/workflows/pr.yml on PR #72 in b220ecb, so the final stack can no longer pass CI with this class of unresolved symbol.

RequestExecutor is intentionally retained. OC-000's task acceptance explicitly provides stable executor interfaces for the protocol tracks that follow it; this is a type-only foundation contract with no runtime cost. I am not proposing that it "can be moved" or leaving that undecided.

The three low findings are malformed-document behavior, not accepted request shapes: an info-only item has no protocol, a request/folder hybrid with items is schema-invalid, and an unknown info.type fails request schema validation. The HTTP fallback remains necessary for legacy HTTP request files without info.type; it does not make an unknown type valid. No source change is warranted for those invalid shapes in this foundation PR.

Final verification: all 37 immediate stack edges merge cleanly; all 37 heads compose sequentially; production build and TypeScript compile pass; 31 test files / 539 tests pass; demo validation passes 47/47 files.