🔒 Make canonical core authoritative for execution - #432
Draft
taras wants to merge 10 commits into
Draft
Conversation
PR #432: 🔒 Make canonical core authoritative for execution14 files, +1873 / -198 Scope🔴 PR has 2071 lines changed. Split into focused PRs. 🟡 2071 lines changed. PRs under 400 receive more thorough review. 🟡 PR mixes config and source changes. 🟡 package.json changed without dependency justification. StructuralOxlint structural signals:
Slop
Static AnalysisOxlint: 27 diagnostics across 3 files (9 rules) no-unused-vars (12): packages/core/src/agent/components.ts, packages/testing/src/components.ts, packages/core/src/execute.ts CorrectnessNo extraneous code patterns detected. |
A stable Api name shares the middleware context; a createApi() instance owns its own default handler. Canonical core invokes the public chain through a private same-name instance whose default is closed over this invocation, so every public handler composes exactly as before while the terminal belongs to one execution and no other. The exported Execution.execute default now always refuses, so calling it with a captured live request settles nothing. Consumption moved behind the issued invocation: it accepts only a request created for that exact invocation, at the current generation, unconsumed, validating everything before writing anything — so a rejected foreign delegation consumes neither, and both requests stay usable. Null, primitives, reconstructed values and hostile proxies are refused with a fresh cause-free error rather than a native one. Each invocation owns a child scope, so contextual behavior an installation establishes reaches document teardown, stays out of a concurrent invocation, and is gone from the next execution. Also in this revision: the retained history every admission is held to is frozen before anyone is offered it; completion precedence is first-failure across prompts, teardown and test policies; and the conformance table, terminology and ACP contract are reconciled with what the code does.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // is a compile-time claim: without this an admission could splice, reorder | ||
| // or empty the history in place, and every later admission, root-history | ||
| // validation and the replay itself would consume what it left behind. | ||
| // The events themselves are the retained graph's own, already sealed. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // The events themselves are the retained graph's own, already sealed. |
| // operation. | ||
| const replayed = yield* readCompletedRun(executeOptions.stream); | ||
| // The request's candidate stream, inspected and not acted on: hydration | ||
| // restores what the journal already recorded and decides nothing. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // restores what the journal already recorded and decides nothing. |
| return yield* bridgeRootProvider(rootProvider, executeOptions, failures, next); | ||
| // Additive: prompt failures and a provider teardown failure turn a | ||
| // successful document into a failure. A document that already failed | ||
| // keeps its own failure — the completion policy adds, it does not replace. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // keeps its own failure — the completion policy adds, it does not replace. |
| // is a compile-time claim: without this an admission could splice, reorder | ||
| // or empty the history in place, and every later admission, root-history | ||
| // validation and the replay itself would consume what it left behind. | ||
| // The events themselves are the retained graph's own, already sealed. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // The events themselves are the retained graph's own, already sealed. |
| return retained; | ||
| // The same objects the admissions were held to. `readAll` is declared | ||
| // mutable by the protocol, so this is a fresh array over the identical | ||
| // sealed events rather than a second reading of the backend. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // sealed events rather than a second reading of the backend. |
| throw new ExecutionProtocolError("delegated a request that a later withOptions() superseded"); | ||
| } | ||
| // Snapshotted before the invocation is marked consumed, so a failure to | ||
| // detach leaves nothing consumed either. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // detach leaves nothing consumed either. |
| // is a compile-time claim: without this an admission could splice, reorder | ||
| // or empty the history in place, and every later admission, root-history | ||
| // validation and the replay itself would consume what it left behind. | ||
| // The events themselves are the retained graph's own, already sealed. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // The events themselves are the retained graph's own, already sealed. |
| *[Symbol.iterator]() { | ||
| // Registered in the *consumer's* scope, so halting a consumer that is | ||
| // still waiting takes the invocation down with it. After settlement this | ||
| // does nothing, which is what lets a completed handle be read again. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // does nothing, which is what lets a completed handle be read again. |
| }, | ||
| }); | ||
|
|
||
| // Whatever a handler returns is not an execution, so it is not read. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // Whatever a handler returns is not an execution, so it is not read. |
| // `#invocation in request` recognizes a value this class constructed | ||
| // without reading anything off it and without a registry to consult. It is | ||
| // also total: `in` on a primitive, on null, or on a proxy whose traps throw | ||
| // is guarded here, so nothing native or planted escapes. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // is guarded here, so nothing native or planted escapes. |
| throw new ExecutionProtocolError("delegated a request that a later withOptions() superseded"); | ||
| } | ||
| // Snapshotted before the invocation is marked consumed, so a failure to | ||
| // detach leaves nothing consumed either. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // detach leaves nothing consumed either. |
| try { | ||
| return #invocation in value; | ||
| } catch { | ||
| // A revoked proxy, or one whose `has` trap refuses. Not one of ours. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // A revoked proxy, or one whose `has` trap refuses. Not one of ours. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Execution.executemiddleware used to be handed the finishedDocumentExecutionand have its return value taken as the result. That made every handler
authoritative by construction: one could answer without delegating and its
invented completion was the execution, and one could wrap the returned handle
and rewrite what the document had already settled.
That is also why #426 had no correct place to put workflow-run identity. Two
review rounds moved it — into
ReplayGuard, then into anExecution-installedstream wrapper — and both were defeated by registration order, because both
surfaces are composable policy.
This PR makes canonical core authoritative for execution and gives a trusted
host an explicit way to attach requirements. It is a prerequisite: it contains
no workflow-run recognition, no workflow filesystem, and no CLI lifecycle.
Stack
Public API
@executablemd/core— the policy surface:@executablemd/core/host— the infrastructure boundary:No
admitJournal(), norequiredJournalAdmissions(), noJournalAdmissionsContext. An admission travels as a function the host holds and passes, which is
why a separately loaded package composes here without agreeing on a name.
How it works
Middleware return values are never read.
execute(options)is the same callwith no installations, so ordinary execution is unchanged.
Migrated consumers
packages/testinguseTesting()decorateCompletion(inner, …)addCompletionFailure()packages/testing<Testing>executeOptions.stream, thendecorateCompletionrequest.options.stream; completion becomesaddCompletionFailure()packages/coreAgentbridgeRootProvider()manufactured aDocumentExecutionwith its own channelExecution.document; prompt + teardown failures becomeaddCompletionFailure()decorateCompletion()is deleted from both packages — core's completion facilityreplaces it.
bridgeRootProvider()is deleted: nothing manufactures aDocumentExecutionany more.Test-installed handlers needed no migration: they either use
document(whichis unchanged) or refuse before delegating.
Verification
c7aa595c98ff6d8e5bf08113b8efe0dd73dfd9e3, gates read by exit code:Focused: Tier EP (28 unique steps),
root-provider+agent-components(18 steps),use-testing,document-target-execution,secret-detection.Mutations
Each applied, the suite re-run, the source restored from
HEADand confirmedbyte-identical before the next.
install()Two honest notes:
document changes nothing, because the loop's
!settled.okguard alreadyreturns the existing failure. The two guards are redundant with each other by
design; each alone is covered by the other, and neither can be removed without
R7b or the call-site shape catching it.
ExecutionProtocolError, which a silently-ignored look-alike also produces via"returned without delegating". It now asserts the refusal names the look-alike.
Behavior changes
Agent path combined a document failure with provider teardown into an
AggregateError; under additive precedence an execution that already failedkeeps its own failure. Prompt-failure aggregation against a successful
document is unchanged and RP3/AC13 still pass on their existing assertions.
added, not thrown — same precedence the bridge had, now without a manufactured
handle.
Intentionally not here
specs/workflow-spec.mdis untouched; 📁 Give a workflow run's document its own filesystem (#366 PR 1) #426 owns the concrete installation andhost handoff after it rebases.
JournalProvenancestays at its two existing trusted wrapping sites — thisadds no wrapping site, because requirements are contributed into core's
existing admission read rather than wrapped around it.
relaxed.
Scope confirmation