4.4 KiB
Execution Controls Review
Reviewed implementation stages 1–8 against commit 57b62d3, the
implementation plan, the accepted
feature roadmap, and the applicable architecture,
testing, documentation, and agent policies.
1. Complete The Public Prepared-Notification Contract
Priority: P3. Status: complete. Owning stage: Stage 8.
Location: RunRequest.OnPrepared GoDoc.
The public GoDoc describes synchronous delivery, detached ownership, held admission, and split preparation, but omits two explicitly selected lifecycle facts:
- Notification occurs at most once per logical execution, when execution reaches that point before the first generation. Transient retries and structural repair do not repeat it.
- The callback receives the operation context, including the selected operation deadline when enabled and inherited parent cancellation. In coordination this is distinct from the parent context delivered to the next-turn callback.
These facts are already implemented by the shared execution boundary and covered by the notification, retry/repair, and coordinator contract tests. They are also expressly selected in the roadmap's accepted callback decision. However, Go declarations and GoDoc own the exact public API contract; consumers should not need a temporary roadmap or implementation inspection to discover invocation cardinality or the scope of their callback's context.
Required remediation: add these two facts to the field's GoDoc. Preserve conditional delivery and cooperative cancellation; do not promise notification for every failed request, forced callback interruption, or terminal observation after panic. This is a documentation completion, not a runtime change. Existing behavioral tests already cover these semantics; duplicate tests are unnecessary.
Completion: expanded the field's GoDoc to state invocation cardinality, non-repetition during retry/repair, and operation-context scope, including the distinction from the coordinator's next-turn callback context. Runtime behavior is unchanged. GoDoc rendering and the existing focused race-enabled notification, retry/repair, and coordinated-budget tests passed.
Stage Review Summary
| Stage | Result |
|---|---|
| 1 — API and conversion | Selected fields and sentinel are present; callback conversion captures the chosen function and detaches details only when selected. |
| 2 — Ordinary timeout | One internal scope covers ordinary execution; per-invocation cause attribution and public error projection precede terminal observation. |
| 3 — Split preparation | Handles retain duration and callback without an idle timer; claim/discard and cleanup behavior are preserved. |
| 4 — Prepared notification | Both execution paths use the actual shared snapshot, with context checks and held admission; retries and repair do not notify again. |
| 5 — Phase budgets | Tests cover preparation, capacity waiting, retry backoff, generation/repair, validation, notification time, and stable terminal outcomes. |
| 6 — Ownership and failures | Public tests cover detached messages, cache metadata, nested parameters, schema, credentials, frozen sources, retained diagnostics, and panic cleanup. |
| 7 — Coordination | Fresh turn budgets, parent-context assessment, released capacity, sibling use, observer composition, and explicit grouping are covered. |
| 8 — Documentation and example | Maintained example, consumer/internal documentation, and the completed canonical GoDoc reflect the feature; finding 1 is resolved. |
No runtime correctness defect, additional architectural gap, or inadvertently strengthened caller restriction or guarantee was identified in this review.
Validation
- The complete package suite passed under
go test -race ./...when combining the initial run with successful reruns ofinternal/configurationandinternal/llm. Those two packages initially failed solely because the sandbox disallowed their localhttptestlisteners; their reruns permitted listeners. go vet ./...passed using Go 1.27.3 withGOTOOLCHAIN=local,GOWORK=off, and offline module resolution.- Changed Go files are formatted.
git diff --checkand local document-target checks passed. No implementation, dependency, policy, or release files were changed during the review.