Repository navigation
fix(controller): handle unassigned canceled completions - #148
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. |
|
Validation update for exact head 6baf72a:
The conservative zero-ID/time and job/request-identity conditions are deliberate local restrictions; the SDK README does not guarantee every legitimate cancellation has that shape. The observed historical empty-name event remains unclassified. This patch neither silently accepts other blanks nor proves that historical incident fixed. |
|
Complete controller race checks PASS on unchanged exact head6baf72a8429bfc6072f65c9b79f77359a52c3678 with Go1.27.2: GOTOOLCHAIN=local go test -race ./... -count=1 The full Go1.26.3 run and its isolated proxy fixture failed as already recorded, with the same isolated failure reproduced on base456c92e. Those results remain retained. Changed-path race checks pass on Go1.26.3, and the complete controller package passes on Go1.27.2. Independent exact-source review, local documentation links and committed-secret scanning pass. The separate eight-stage hosted full run and ordinary exact-head review remain required before merge/adoption. These controller checks alone do not authorize runtime promotion or host activation. No source changes were made after6baf72a. |
|
@codex review One exact-head standalone-receipt follow-up for the completed review of6baf72a8429bfc6072f65c9b79f77359a52c3678, base456c92e02c8266ee0d45e81b562cd8b20ec49f11, treef0aaa72634cd87d7320f6913953e922b11054b65. Summary6382459371 completed clean at09:19:49Z with both finding flags false, but there is still no standalone receipt or formal review at readback. The full hosted run40081185025 has now completed SUCCESS on this exact head. No other receipt follow-up exists. Please independently confirm the exact source and provide the missing standalone receipt if a clean result is warranted. Use the plain standard no-major-issues opening and Reviewed commit binding without an added suffix. Report actual findings normally; do not infer approval from the summary or this request. The source is unchanged. Preserve the narrow cancellation restrictions and retained invalid-event/Docker failures. The historical empty-name event remains unclassified. No new tests, input edits, parser change, trust expansion, runtime substitution or host action is requested. This is one coordinated follow-up, not a request to repeat other reviews. |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
The owner explicitly accepted standalone receipt 6382774661 from bot ID The unedited receipt's abbreviation resolves authoritatively to that full head. Its body SHA-256 is Hosted run 40081185025 attempt 1 completed successfully. Both jobs passed, the complete driver exited successfully, and all eight full stages appeared in order, including container build and cleanup. Actual checkout The owner decision leaves all runtime, image, private-validation and host-admission gates unchanged. The historical empty-name event remains unclassified. No host retry or runtime substitution follows from this public merge. Documentation impact is already covered by |
|
PR148 was SHA-bound squash merged at The main-push run is 40114451217, attempt 1. Its current status is The owner-selected source-review decision is recorded in 6387045218. Parser behavior is unchanged. No private input, runtime pin, image policy or host was changed. 02 remains drained under the last verified return state; this merged commit does not authorize admission. Actual merged-main success and the separate runtime, image, private-validation, recovery and host gates remain required. Documentation impact remains covered by |
|
Merged-main validation review for exact The full driver completed, followed by the convention gate with Evidence discrepancy is retained. The job-step API still labels the suite in progress and later steps pending despite completed successful jobs and full logs proving those commands finished. No successful per-step API verdict is claimed. The logs are hosted execution evidence; raw stage reports, detailed cleanup inventory and a signed validation bundle were not uploaded. The owner's exact source-review acceptance in 6387045218 remains limited to that record and public merge. No parser, runtime pin, image policy, private input or host changed. Runtime and host adoption remain separate. No new implementation or documentation finding emerged from this evidence audit. |
The pinned SDK can report a canceled scale-set assignment before a runner acquires it. The controller currently rejects its empty runner name, which terminates listener processing after the SDK has acknowledged the batch and can prevent later callbacks from running.
Handle only an explicit
canceledcompletion with no runner name, zero runner ID, unset runner-assignment time, a nonempty job ID and a positive runner-request ID. It does not change runner state or access Docker. Other blank, contradictory, unrelated named or Docker-error cases retain their existing failures. Completion metadata is logged before handling so an unclassified event retains the evidence needed to diagnose it. An empty name alone does not establish cancellation.Validation: the new handler, diagnostic and real pinned-SDK batch regressions failed before the correction and pass afterward. Existing named-runner rejection coverage now also exercises canceled events. Local documentation links and the trusted committed-secret scan pass. Complete controller race checks and exact-head hosted validation are required before merge or adoption; focused checks are not full-suite or host evidence. See the PR checks and validation comments for their results.
Documentation:
docs/CONTROLLER-DESIGN.mddescribes the exact conservative exception, diagnostics and SDK acknowledgment ordering. This runtime-only correction changes no schema, capability, initializer, desired-state field, example or operator procedure, so no standalone configuration-template change is needed. Engine availability, adopter promotion, reproducible image identities and host acceptance remain separate.The reported historical empty-name event did not retain its result or runner ID. This patch covers the documented SDK case; it does not prove that historical event's cause or authorize a retry or runtime substitution. The Docker removal race is a separate issue.