diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md new file mode 100644 index 0000000..5adbd3c --- /dev/null +++ b/docs/roadmap/remediation.md @@ -0,0 +1,66 @@ +# Execution Controls Review + +Reviewed implementation stages 1–8 against commit `57b62d3`, the +[implementation plan](implementation.md), the accepted +[feature roadmap](execution-controls.md), and the applicable architecture, +testing, documentation, and agent policies. + +## 1. Complete The Public Prepared-Notification Contract + +Priority: P3. Status: open. Owning stage: Stage 8. + +Location: [RunRequest.OnPrepared GoDoc](../../types.go#L97). + +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](../../internal/usecase/runner.go#L186) and covered +by the notification, retry/repair, and coordinator contract tests. They are also +expressly selected in the roadmap's +[accepted callback decision](execution-controls.md#3-what-is-the-prepared-details-callback-contract). +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. + +## 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 and consumer/internal documentation reflect the feature; complete the canonical GoDoc as described in finding 1. | + +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 of `internal/configuration` and + `internal/llm`. Those two packages initially failed solely because the sandbox + disallowed their local `httptest` listeners; their reruns permitted listeners. +- `go vet ./...` passed using Go 1.27.3 with `GOTOOLCHAIN=local`, `GOWORK=off`, + and offline module resolution. +- Changed Go files are formatted. `git diff --check` and local document-target + checks passed. No implementation, dependency, policy, or release files were + changed during the review.