[19/36] Add OC-110 runtime authoring UX

[cjohnsto-nz]

---

This is part 4 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 <-- current
•  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
•  1  [01/36] Add OpenCollection agent plan #35

[APKiwi]

Verdict: approve with nits. Verified the important correctness properties directly: disabled assertions, actions and variables are all honored by the runtime (runtimeExecutionService.ts:306, :344, :477), gRPC loadRequest reads the right metadata source, method/params stay gated to http/graphql so they don't pollute WS/gRPC models, and all interpolation goes through the existing esc() helper so no new webview injection surface.

• Minor: incomplete rows are silently dropped on save. mergeRuntimeActions keeps a row only when selectorExpression and variableName are both set (schemaRoundTrip.ts:473-474), mergeRuntimeAssertions requires expression and operator (:457). Add a row, save before filling every field, it vanishes without warning.
• Nit: editing a description collapses a structured {content,type} description to a plain string. Expected given the single text input.
• Nit: runtimeAuthoringUx.test.ts asserts on source text (toMatch on the literal code) rather than behavior, refactor-fragile.

Tests: good model-layer coverage (per-protocol round-trips with Ajv validation, unsupported-row preservation, full CRUD and reorder). Gap: nothing drives the actual webview DOM path (moveRuntimeRow, collectRuntimeModel, checkbox to disabled), that glue is only exercised via source-string assertions.

Deps: none added.

[APKiwi]

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

The gRPC editing claim from the #40 thread (01915d7) is verified: schema-native gRPC editing lands here (isVisualEditableRequest accepts grpc, create/apply gain grpc branches, sequence-preserving).

New:

• (high) esc() (src/webview/state.ts:37-41) escapes & < > but not double quotes, and the runtime rows interpolate user YAML into double-quoted value="..." attributes (src/webview/requestPanel.ts:667,669,676,702,704,712). Any assertion expression, expected value, description, action selector, or variable name containing a " is truncated at the first quote when the row renders, and collectRuntimeModel writes the truncated string back on the next scheduled update. That silently corrupts the user's YAML. It is not theoretical: the assertion engine supports bracket expressions like res.headers["content-type"] and JSON expected values, and JSON needs double quotes, so value: '{"ok":true}' corrupts to { after open plus edit. CSP blocks script injection, so this is data corruption, not XSS. The shipped demo fixtures happen to contain no embedded quotes, so tests and demos miss it. Fix: escape quotes, or set .value via the DOM property the way the header value cells already do.
• (low) A message-less gRPC request hydrates to an empty body and any save injects grpc.message: "". Schema-valid but no-op churn.
• (low) AGENT_PROGRESS.md OC-110 row cites fe0ad14/1e6db05, neither of which exists (restack rewrote them to 01915d7/c69bcde).

Prior findings (incomplete rows dropped on save, description collapse on edit, source-text test assertions, no DOM-path tests) all still open. The last one is why the high finding above ships uncaught: the model-layer tests bypass the DOM.

Verdict: changes-needed. The quote corruption damages user-authored runtime YAML through normal editing and no current test can catch it. The model-layer round-trip safety is otherwise solid.

[cjohnsto-nz]

I addressed the valid findings and pushed the PR-owned fixes in b4b3dea / 5fe2bd8.

• esc() now encodes quotes used inside HTML attributes. test/webviewState.test.ts exercises the actual JSDOM parse path for quoted header expressions, JSON, and descriptions, so the source-text checks are no longer the only coverage for this behavior.
• Existing assertion/action rows are now preserved while a required field is temporarily incomplete, instead of being deleted by autosave. New rows created through this UI already start with schema-valid defaults (res.status / equals / 200 and $.token / runtime / token), so the normal add-row path does not create an unserialisable blank row.
• Editing a structured description now updates content while retaining its other metadata, including type.
• A message-less gRPC request no longer acquires grpc.message: "" during a no-op editor save.
• I retained the existing source/layout contract tests and added browser-DOM behavioral coverage; the contract tests still protect the supported phase/scope choices and unsupported-row diagnostics.

The stale OC-110 evidence hashes were also valid findings. I corrected all five occurrences from fe0ad14 / 1e6db05 to the actual 01915d7 / c69bcde commits in fix/open-collection-pr-rework commit 87f10e3. That ledger has extensive descendant edits, so the correction belongs on the stack-tip rework branch rather than this historical implementation branch.

Validation on PR 52: TypeScript compile passed, all 436 tests passed, and the production build passed. I also composed the updated PR 49, PR 51, PR 52, and stack-tip rework changes: compile passed, all 505 tests passed, and the production build passed.

The pre-push stack check initially detected that adding tests to test/runtimeAuthoringUx.test.ts would conflict with later work. I moved those regressions to a dedicated file and restored the existing file exactly before pushing. The final PR 51 -> PR 52, PR 52 -> PR 53, and PR 52 -> rework simulations are all STACK_CLEAN; GitHub currently reports both PR 52 and immediate child PR 53 as MERGEABLE / CLEAN.

[cjohnsto-nz]

Response to the second review: all of these findings were valid and are fixed on this PR's current head, 5fe2bd8.

The webview escape function now preserves both quote types in attribute values, with DOM-path tests for bracket expressions, quoted JSON values, and descriptions. Saving a message-less gRPC request no longer injects message: "". Temporarily incomplete runtime rows preserve the existing authored row instead of deleting it. Structured descriptions retain their schema metadata when edited. The OC-110 ledger hashes now point to the live commits, and dedicated round-trip regression tests cover all three persistence cases.

Verification on the exact final composite passes build, TypeScript compile, 31 files / 539 tests, and 47/47 demo validation. No additional change is required for PR #52.