Repository navigation
Add shared tool execution and durable bounded work - #2
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c52e5c314
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const key = cacheIdentity(provider); | ||
| const previous = cache.get(key); | ||
| const refresh = previous === undefined || options.freshness === "turn" || invalidatedConnections.has(provider); | ||
| if (!refresh && previous !== undefined) { |
There was a problem hiding this comment.
When a ConnectedProvider omits the optional contextScope but its buildSnapshot() follows a changed authenticated principal or workspace, this lookup always uses the same {} scope key and returns the previous session-cached snapshot without calling the provider. Reusing that connection can therefore expose one principal's context to another; require a stable declared scope or derive and validate the cache key from the returned snapshot before reusing it.
Useful? React with 👍 / 👎.
| const state=this.requireState(); | ||
| const stepNames=new Set(Object.values(this.options.steps).map(step=>step.toolName)); | ||
| const last=this.executions?.recover().filter(execution=>execution.settled && stepNames.has((execution.settled.receipt as {toolName:string}).toolName)).at(-1); | ||
| const lastStepExecution=last ? Buffer.byteLength(JSON.stringify(last.settled)) <= 16*1024 ? {executionId:last.executionId,...last.settled} : {executionId:last.executionId,receipt:last.settled!.receipt,artifactRef:state.outputArtifacts[last.executionId],resultAvailable:true} : undefined; |
There was a problem hiding this comment.
When a step settlement exceeds the 16 KiB status cutoff but the execution remains below the separate 30 KiB artifact cutoff, no artifact is created, yet this branch removes the result and reports an undefined artifactRef with resultAvailable: true. After the immediate step response is gone, of_work_status cannot provide or reference that result; align the cutoffs or retain the inline settlement whenever no artifact exists.
Useful? React with 👍 / 👎.
1 participant
Tool calls previously followed different validation, execution, and result-recording paths across hosts. This change routes direct calls, MCP, chat, agents, and workflows through a shared policy, scheduler, and receipt boundary, and adds an opt-in local controller for bounded work with durable recovery.
The controller uses persisted execution and completion grants, authoritative lease checks, separate execution/work journals, immutable output artifacts, and verification bound to current inputs and mutation epochs. Queued work and verification recheck ownership before dispatch. Completion fingerprints every proof subject after awaited readbacks and immediately after the final authority lookup. Uncertain effects stop queued work; explicit independent readback can reconcile an interrupted correction without repeating its effect or rewriting the original unknown receipt.
Included
Hosted Siftable durable recovery remains disabled before any claim: SDK 1.5.1 does not establish the supported public parser or receipt/key guarantees required for automatic recovery. The versioned contract matrix records these gaps. Native host actions outside the OpenFunction executor remain outside its journals.
Validation
The updated commit was checked locally on Node 26.5.1:
npm test: 269 existing tests pass.npm run test:focused: 220 tests pass, including 65 controller tests, eight work-output tests, five real MCP protocol tests, 19 executor tests, and 10 context tests.All six CI jobs pass on this updated commit on Node 22/24: full tests, compiler parity, fresh tarball consumers, compiled grant CLI, native OpenClaw/Pi gates, and real pinned Hermes grant/recovery proofs. Hermes pin:
7cc3fb8cb55898747dd1dfa56fda3e16ebba028e. CI results.Host proofs use isolated filesystem jobs and deterministic model endpoints. No paid model calls or live hosted queue mutations were needed.