Skip to content

feat(runtime): establish durable form interactions - #4379

Open
me2seeks wants to merge 6 commits into
mainfrom
feat/4364-runtime-form-interactions
Open

feat(runtime): establish durable form interactions#4379
me2seeks wants to merge 6 commits into
mainfrom
feat/4364-runtime-form-interactions

Conversation

@me2seeks

@me2seeks me2seeks commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Establishes the provider-neutral Runtime Interaction foundation for structured form requests without introducing an MCP-specific authority.

  • Adds a bounded primitive form contract covering strings, numbers, integers, booleans, single-select, and multi-select fields. Requests are exact-shape decoded, defaults and accepted values are validated against the stored schema, and admission rejects forms that cannot produce any bounded accepted answer.
  • Parks the exact Tool call in ToolRuntime and brokers hosted requests through the existing Runtime Host Interaction owner. InteractionStore remains the sole pending/outcome authority: invalid answers leave the request pending, equivalent concurrent answers converge on one canonical result, and only that result resumes the captured continuation.
  • Carries form request and answer-ack facts through the Runtime Event Log and pending Session projection. Turn stop, Turn terminal, and Host restart use the existing durable closure path. Runtime Host compatibility epoch 81 rejects older peers before they can observe the new closed protocol shape.

Refs #4364 (rollout PR 1).

Rollout

This PR intentionally has no MCP request handler or user-facing renderer. Follow-up PRs will add Desktop/TUI form surfaces and then adapt MCP elicitation/create onto this authority; neither follow-up may introduce a second pending-request or winner state.

Verification

  • npm run build
  • 181 focused Core, Runtime, Runtime Event projection, protocol, and authentic SQLite Runtime Host tests passed
  • npx biome check on all 28 changed files
  • git show --check HEAD
  • Screenshots: N/A — this PR changes no Desktop or TUI surface. The surface PR will include real before/after screenshots.

Review focus

  • The Host validates answers against the canonical stored form before committing a winner; the surface does not become an answer authority.
  • Hosted admission completes before publication, and durable closure settles the exact captured continuation before release.
  • The primitive schema remains provider-neutral; MCP wire state and retry behavior are deliberately absent from this layer.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex analyzed the existing Interaction authority, implemented the form contract and lifecycle, added tests, and performed separate correctness/lifecycle and architecture/ownership review passes. The commit contains the required Generated-by trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/XL Over 1000 readable lines label Aug 31, 2026
@me2seeks
me2seeks force-pushed the feat/4364-runtime-form-interactions branch from be4056c to ed745fb Compare August 31, 2026 17:47
@me2seeks
me2seeks force-pushed the feat/4364-runtime-form-interactions branch 2 times, most recently from 032d77f to d8bae9f Compare August 31, 2026 19:00

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #4379d8bae9f — blind review sealed (stack base)

Summary: Durable form interaction baseline (Runtime/Core/Storage/Runtime Host). Exact head d8bae9fffaa501b5fa0de383371ece11421bc3a2 frozen, 0 checks/0 reviews/0 threads on stacked base; current-main da84f88de merge has only the explicitly ignored protocol epoch 83→84 vs 83→85 conflict (per @me2seeks). Source is approvable with comments; no simplify finding. Full build passed; focused SQLite/auth tests 208/208 green.

Findings (reproducible, decision-changing only):

  • P2 — unsafe projection tool form textform message/requester/field/option text is only byte-bounded, not reused Interaction safe projection. Probe with U+202E + sk-live-… persists and projects verbatim in the form path, while question path escapes bidi and redacts — untrusted provider can spoof source or leak secret. Fix: reuse existing safe projection/redaction on form rendering.
  • P2 — answer/outcome cap mismatch — both 8 KiB caps, but outcome adds 4 required strings + min=max=2025 wrapping. A form admitting exactly 8,172-byte answer is legal, wrapped as canonical outcome 8,195 bytes is rejected by store codec → pending Tool can never accept, only decline/cancel. Fix: reserve outcome overhead from answer cap or make admission account for wrapping.
  • P3 — date-time validationdate-time uses regex + Date.parse; 2023-02-30T00:00:00Z is accepted and normalized to Mar 02 by canonical Host validator. Fix: strict calendar validation.

Gating: exact-head hosted checks green (2/2), no approval/review threads. Stack inheriting risk noted for #4384/#4392/#4397.

Automated review notice: This comment was posted by an automated review agent operated by AstroHan. It is not an independent human review and does not replace one.


简体中文

本条结论来自 @捣蛋鬼 在 exact head d8bae9f 的独立盲审,已按 @me2seeks 指示排除 compatibility epoch 冲突的计分。我作为编排仅核对 head 未漂移与 exact-head CI 状态,未替代独立审查。

@me2seeks
me2seeks force-pushed the feat/4364-runtime-form-interactions branch from d8bae9f to 1fba905 Compare September 1, 2026 08:26
@me2seeks

me2seeks commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all three findings in 1fba905: form-facing text now goes through the shared safe projection/redaction boundary (including projected-label collision checks), accepted answers are bounded against the actual canonical outcome envelope, and date/date-time validation rejects normalized invalid calendar values. Added focused regressions for each case.

I also rebased the stack onto current main (920d714).

@me2seeks
me2seeks requested a review from Astro-Han September 1, 2026 14:49
@me2seeks
me2seeks force-pushed the feat/4364-runtime-form-interactions branch from 1fba905 to 4609622 Compare September 1, 2026 15:04
@me2seeks

me2seeks commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (afbcabd). Main's catalog protocol change already occupies epoch 87, so the form interaction contract now advances to epoch 88; both compatibility notes remain in the ledger. Re-ran the 108 interaction/protocol checks on the rebased head with an isolated writable test root.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed a12633d2. I spent most of the time trying to break the claim that this reuses the existing Interaction authority instead of growing a parallel one, and on the Host side I couldn't: #answerQuestion#answerStoredInteraction, #requireLiveQuestion#requireLiveStored, #commitAnswer widening to LiveStoredEntry, #requireLiveClientCapability folding into the shared path, InteractionStore picking up 12 lines inside the existing branch. That's real generalization. Concurrency convergence and continuation exactness also hold — I traced and measured both.

The findings are all one thing, so I'll describe it once.

The root: "which interaction kinds exist" is stored 18 times, as allow-lists with silent fallthrough

18 production sites dispatch on user_question_request. Six learned about form; twelve didn't. What separates them isn't care — it's whether the compiler could speak.

The one site that had to be updated, session-event-runtime-mapper.ts, is the only one of the six ending in:

const _exhaustive: never = event;

Adding FormRequestEvent to SessionEvent made that a compile error. Everything that was missed looks like the opposite — a silent default: (statusFromEvent, reduceInteractionQueues, both stream-graph-*) or a hand-maintained positive list (runtime-kernel.ts:1591 and :3335). Add a kind and each is quietly wrong, and nothing says so.

Desktop and CLI surfaces (7 sites) are reasonably deferred to #4384/#4392. These five are this PR's own layer:

site consequence for form
runtime-kernel.ts:1591 assertInteractionPublication the publication linearization point never runs; tracked.published stays false, so close() blocks on Promise.all(publicationBarriers) (interaction-authority.ts:447)
runtime-kernel.ts:3335 interactionResumeAllowed never runs — which makes the form_answer_ack branch this PR added to settlementMatchesAck dead code
session-projection-helpers.ts:118 statusFromEvent session reads running the whole time it is blocked on the user
stream-graph-{projection,read-model}.ts a supervisor can't see a child blocked on a form
ui/interaction-queue.ts:68 no form case, but reconcileInteractions takes whatever listActiveInteractions returns — which session-projector.ts now includes forms in

The first row is the one that bothers me: the mechanism this PR exists to establish is switched off for the kind it introduces, and runtime-kernel.ts isn't in the diff at all. The last row plus chat-composer-region.tsx is concrete: a form becomes activeInteraction, hidden={… || Boolean(activeInteraction)} hides the composer, none of the three narrowing checks match, so nothing renders in its place.

The same shape recurses inside functions — details inline: projectInteractionFormRequest projects a list of fields rather than every displayable string (field.name, field.default, option.value keep raw ESC bytes); assertFormHasAcceptedAnswer witnesses required fields at their lower bound, so it proves "some answer fits" where admission needs "every legal answer fits"; and tool-runtime.ts adds 212 lines with 2 new kind discriminants where interaction-projection.ts adds 36 with 9 — copying doesn't need branches.

The repair that matches the cause

Not twelve new form branches. That's the same manual sweep again, and the sixth kind pays it a third time — a simplification pass alongside this review put the cost of adding one at ~45 edit sites. Instead, let the compiler do this sweep and every future one, using the idiom already in a file this PR touches:

  1. Replace the silent default: in statusFromEvent, reduceInteractionQueues and the two stream-graph-* projections with const _exhaustive: never = event. Sites that genuinely care about a few event types should narrow their parameter instead — that narrowing is the work the default has been hiding.
  2. Derive assertInteractionPublication and interactionResumeAllowed from one table of which kinds are hosted interactions, rather than two lists that have to agree by hand.
  3. Project every displayable string by construction, and witness the upper bound.

What disappears is the obligation: the sixth kind becomes a table entry plus whatever the compiler then points at. That's also the honest answer to what this change made redundant — right now it adds 1365 production lines and removes none, and the best evidence the pattern has passed its useful point is that this PR itself missed twelve sites.

Grading

None of this is reachable today — requestUserForm has no production caller (only two test files, against askUserQuestion's two production tools and a response path implemented down to desktop IPC). Every finding is a seam gap that goes live when a producer lands, which is why they're P2 and not higher, and why I'd fix them here rather than in a child PR that would otherwise have to switch on the root's own invariant. Specifically on the publication one: with a producer, immediate stop mode still reaches the right final state because ai-sdk-backend.stop() aborts the scope and finalize() seals publications — inverted ordering resting on an unrelated subsystem, not breakage. The hang needs after_step, which no client sends, so I'm not grading on it.

Two P3s: the local respondToUserForm path can't be reached in either configuration (backend-types.ts has no form twin, and the only production new SessionManager( always passes interactionAuthority, which makes the embedded responders throw), so ~95 production lines and the 200-line tool-runtime-form-interaction.test.ts cover a path that can't ship. And the nine INTERACTION_FORM_* constants have no consumer outside interaction.ts, four duplicating existing constants at the same value and meaning (FIELD_LABELOPTION_LABEL, FIELD_DESCRIPTIONOPTION_DESCRIPTION, FORM_VALUEANSWER, FORM_REQUESTER_NAMEINTERACTION_TOOL_NAME).

Coordination note: #4184 also sets RUNTIME_HOST_COMPATIBILITY_EPOCH = 88. And AgentGraphSupervisorAttentionReason is a closed union, so adding form_request to the supervisor signals later costs another bump — doing it now spends this one once instead of twice.

Evidence boundary: read against afbcabdc74 (#4433), the true base — a stale local main makes session-transcript-pager.ts appear in the diff and it is not part of this PR. Decode, projection, witness and convergence results are from running this branch's built @maka/core; the publication-barrier behaviour was reproduced by driving the real RuntimeInteractionRunBinding with the kernel's predicate copied verbatim, not through a live kernel and Host — there's no producer to drive one with. The 18-site count is a grep for user_question_request, so a site dispatching by some other spelling wouldn't appear. All four touched suites pass on this head (135 tests, 0 failures). I didn't review #4384/#4392/#4397.


AI-assisted review: drafted with Maka; I verified the exhaustiveness asymmetry, the 18-site count, the publication predicate, the projection gaps and the witness measurements against the branch source myself.

Comment thread packages/core/src/events.ts
Comment thread packages/core/src/interaction.ts
Comment thread packages/core/src/interaction.ts
};
}

private async requestUserForm(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 — the third copy of one mechanism, and the discriminant count shows it.

Against main:

file added kind discriminants
interaction-projection.ts +36 20 → 29
interaction-coordinator.ts +122 40 → 46
interaction-authority.ts +112 10 → 15
tool-runtime.ts +212 54 → 56

Generalizing adds branches to absorb a kind; copying doesn't need any. requestUserForm (95 lines) and askUserQuestion (99) are the same control flow line for line — throwIfAborted, interactionRun(), park, the onAbort closure, if (hostedRun) void parked.catch, createXSettlement, admitXRequest, racePromiseWithAbort, ack, finally — differing only in payload construction and type names. settleUserFormAnswer, closeUserForm, finishDeferredFormTurnClosure and createFormSettlement are each their question twin too, so there are now three parallel registries and three deferred-closure flags whose shapes differ only in payload type.

P3 because it isn't a defect — the Host side really did generalize, and this is the one place that didn't. But it's the concrete reason twelve missed sites were possible: when each kind carries its own copy, nothing forces a new kind through a single seam where the gaps would show.

Related: requestUserForm, respondToUserForm, pendingUserFormCount and the new decodeInteractionFormResponse export have no production callers, and the non-hosted half can't be reached in either configuration. Extracting one kind-generic parking mechanism, or dropping the unreachable local path (~95 production lines plus the 200-line test that only exercises it), each leaves less here than there is now. If this layer is meant to land deliberately ahead of its producer, saying so in the description would help — a reader can't currently tell an omission from a plan.

Define a bounded provider-neutral primitive form contract and carry its request and acknowledgement facts through the Runtime Event Log. Broker pending forms through the existing InteractionStore authority so schema-invalid answers remain pending, concurrent equivalent answers converge on one canonical outcome, and Turn closure or Host restart closes the exact continuation.

Part of #4364.

Generated-by: OpenAI Codex
Expose one closed decoder for renderer-to-runtime form responses so surface adapters do not copy protocol validation. Queue the same canonical continuity refresh for form requests that user questions already receive.

Refs #4364.

Generated-by: OpenAI Codex
@me2seeks
me2seeks force-pushed the feat/4364-runtime-form-interactions branch from a12633d to 066718d Compare September 1, 2026 16:38
@me2seeks

me2seeks commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the form-interaction P2 follow-up in 066718d.

Form requests and answer acknowledgements now share the hosted-interaction predicates used by Kernel publication and resume. I also added their session/stream-graph projections, projected string defaults through the existing review-text boundary, and made admission reserve the full legal answer envelope—including optional fields—so a valid submission cannot fail later at persistence.

The rebase retains mains catalog epoch 88 and moves the form contract to epoch 89. Focused Core and Runtime suites pass 57/57.

@me2seeks

me2seeks commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Followed up on the remaining Desktop queue gap in 562b832. The parent PR now keeps the composer queue explicitly limited to request kinds this surface can render and settle; a form remains authoritative in Runtime Host, but can no longer hide the composer with no prompt in its place. Rehydration applies the same boundary.

The actual form renderer and responder stay in sibling #4384, which widens this surface boundary together with the prompt. That avoids making #4379 claim a Desktop capability it does not yet provide. UI queue, Desktop typecheck, and the focused Core/Runtime suites pass (64 tests total across those suites).

@Astro-Han

Copy link
Copy Markdown
Contributor

Read 562b8325. The direction is right on all of it, and two of the three structural points landed the way I'd hoped:

  • isHostedInteractionRequestEvent / isHostedInteractionSettlementAckEvent replacing the two hand-maintained lists is exactly it — the Kernel now asks instead of enumerating, and the next kind can't be half-registered.
  • assertEveryFormAnswerFitsCanonicalOutcome is the stronger property, and the comment states the asymmetry correctly. formFieldMaximumEnvelope using '😀'.repeat(min(maxLength, ⌊2048/4⌋)) is a sound over-approximation of the code-point/byte mismatch — worst case is 4 bytes per code point, so the envelope can only be too generous, never too tight.
  • Projecting default while leaving name and option value as protocol identities, with the comment saying which is which, is better than what I asked for — the distinction is now readable from the code instead of from the projector's field list.

562b83258 is better than my suggestion. I proposed adding a form branch; narrowing ComposerInteraction back to the three kinds the surface can render and filtering in reconcileInteractions puts the compile-time guarantee back, which is the thing I said the earlier commit had removed.

The build is red on this head, and it's the closed union I mentioned. 066718d8d adds form_request to the stream-graph facets and attention reasons, but those are protocol-owned:

src/server/agent-graph-coordinator.ts(523,5): TS2322
  '… | "form_request" | …' is not assignable to 'Mutable<AgentGraphRecordFacet>[]'
src/server/agent-graph-coordinator.ts(524,5): TS2322
  … not assignable to 'Mutable<AgentGraphSupervisorSignal>[]'

AgentGraphRecordFacet is at packages/runtime-host/src/protocol/agent-graph.ts:104 and AgentGraphSupervisorSignal at :123. Adding the member there should be the whole fix — and since this branch already moved to epoch 89, it's covered.

One epoch note while you're there: main is on 88 as of #4460, and #4184 is also sitting on 89, so whichever of you merges second will need to move again.

The one piece I'd still mention, not as a blocker: statusFromEvent and the two stream-graph-* projections got case 'form_request': rather than the exhaustiveness change, so their default: still absorbs an unknown kind silently. That closes this gap but not the next one. Entirely reasonable to leave for whoever adds the sixth kind — I only want it written down somewhere, since the twelve missed sites were exactly this.

Happy to re-approve once the build is green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Over 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants