Repository navigation
fix(python): join captured preflight and physical generation teardown - #6499
Conversation
Preflight waits and mutable initializer ownership hid live work from cleanup. Publish captured retirement futures before callbacks and retain failed resources for retry, while keeping a surviving shared initializer owner alive. Lore-id: 40f9b713 Constraint: legacy string ownership is not private identity or append authority Rejected: whole-owner cleanup cache | misses newly admitted physical work Confidence: high Scope-risk: bounded Tested: 52 tests across six Python suites, 412 assertions; coding-agent checks Not-tested: Windows and Darwin native execution
The publication guard found an upstream fixture repair after qualification. Join it without replacing the reviewed Python source or native provenance. Lore-id: b9a1d548 Constraint: preserve the exact reviewed Python production blobs Scope-risk: bounded Tested: Python source identity before upstream join
|
Architect source advisory 161: CLEAR for exact head 964f6d4 against dev 1281c9a; 799 production lines, no discounts. Shared initializer retirement membership is separate from live ownership; A reentry joins its original physical initializer without detaching surviving B. All captured physical cleanup futures are published before callbacks, fresh owner/global snapshots include newly admitted work, and failed/unconfirmed resources remain retryable. The four source blobs match the reviewed source exactly after the unrelated upstream fixture join. This is a bounded source advisory, not runtime/platform/full-stack certification or maintainer approval. |
|
Joined QA advisory 164: CLEAR for exact head 964f6d4. Parent ran six Python suites: 52 pass, 0 fail, 412 assertions, 37.68s; vendor/Biome/types/diff checks pass with four visible warnings. Genuine process tests cover synchronous reentry, shared initializer B survival, new-B capture, original failure/retry, zero-budget actual confirmed:false and same-resource retry, cold-process registration, and captured generations. Separate shutdown/append holds discriminate the nested cleanup and clear/new-session transition joins. Qualifications remain: outer cleanup pending is not separately asserted; held append is not separate SDK disposal proof; zero-budget nonconfirmation is not kill-refusal or broad forced-kill/platform proof. Native49 addon reuse is source-proven Linux reuse, not a new build or Windows/Darwin execution. No private string-ABA or filesystem append authority is claimed. Exact-head write-access maintainer approval remains required; no autonomous merge. |
probepark
left a comment
There was a problem hiding this comment.
Code review (CI still running on 964f6d4; approval decision follows once CI finishes).
CI: 게이트 대기 — Affected path validation / native-build is pending, Affected path validation has not reported yet, and the approval gate reports that cd packages/coding-agent && bun run check was not included in the head CI plan.
범위: +2436 / -643, 9 files — Python executor lifecycle, SDK session cleanup admission, Python tool generation/transcript tracking, docs and changelog fragment, regression tests.
규약: CHANGELOG fragment 있음, generated 파일 없음, 라벨 없음.
주목할 곳:
packages/coding-agent/src/eval/py/executor.ts— checked captured preflight cancellation, queued execution cancellation, owner-retained shutdown retry, and generation/session cleanup ordering; no blocking candidate found in the reviewable source.packages/coding-agent/src/tools/python.ts— checked per-generation context capture and transcript append tracking across clear/new-session transitions; no blocking candidate found in the reviewable source.
blocking: 없음
ocr: blocking 0 / nit 0
Approve held: CI plan did not run cd packages/coding-agent && bun run check on 964f6d4; run it at this head (or add it to the plan) and re-request review.
probepark
left a comment
There was a problem hiding this comment.
Review (head 964f6d4, gajae-reviewer on behalf of probepark)
Blocking finding:
packages/coding-agent/test/core/python-executor-owner-cleanup.test.ts:766-776— the newly added unconfirmed-shutdown regression fails on this head. The first shutdown usestimeoutMs: 0; the subsequent process-alive assertion expectstrue, but CI observesfalse. An unconfirmed shutdown does not prove that the process remains alive until the assertion runs. Make the test discriminate retained-resource retry without relying on this process-exit race, and pass the affected suite before re-requesting review.
Evidence: https://github.com/Yeachan-Heo/gajae-code/actions/runs/37726801335/job/113150597306
CI: PR-caused failure — the new regression fails; aggregate validation and evidence production also fail. The latest dev run succeeded (https://github.com/Yeachan-Heo/gajae-code/actions/runs/37727495837). This is not an approval-contract gate failure.
Scope: +2436 / -643, 9 files — Python executor lifecycle, SDK cleanup, Python tool generations, regression tests, docs and changelog fragment.
Conventions: changelog fragment present; no generated files; no labels.
Prior source review: #6499 (review) — no blocking source findings. This gate-only follow-up examined the failed CI evidence, not the full diff again.
ocr: blocking 0 / nit 0
Blocking: 1 CI regression; no additional source findings.
Verification: python3 /opt/data/scripts/gajae-approve-gate.py 6499 --head 964f6d48938adc4e66d9ea0c91b9f84a3620d3c3 returned CI_FAILED (exit 12) at 2026-10-08T04:41:30Z. Pending checks: none. Coding-agent check coverage is now present and passed. The failed test log is tied to this exact head. Local builds/tests were not run in this review pod.
Verdict: gajae.pr-review-verdict.v1 needs-human sha256:f1a7a8035d0c57a3ca3279cf542b4adee1c12c94f65bd37e096379a9f694523d reviewer:critic reviewer-id:gajae-reviewer evidence:ci-new-unconfirmed-shutdown-regression-fails-at-line-776
PR body verdict line count=0, not updated.
Body verdict line is owned by absent; not edited. Suggested verdict line: gajae.pr-review-verdict.v1 needs-human sha256:f1a7a8035d0c57a3ca3279cf542b4adee1c12c94f65bd37e096379a9f694523d reviewer:critic reviewer-id:gajae-reviewer evidence:ci-new-unconfirmed-shutdown-regression-fails-at-line-776
An unconfirmed zero-budget shutdown can already have killed the process by the next assertion. Require a fresh disposal to reenter the exact retained kernel after confirmed PID exit, and join its held genuine retry instead of asserting continued process life. Lore-id: 8a92c516 Constraint: preserve genuine shutdown forwarding and original failure precedence Tested: six Python suites, 52 pass and 415 assertions; coding-agent checks and diff check Not-tested: cross-platform termination behavior Confidence: high Reversibility: trivial
External merge6500 installs the exact reviewed GC consumer source. Preserve the Python repair and join current storage before fresh aggregate verification. Lore-id: 20d738a4 Tested: complete installed tree equals c474 descriptor prerequisite Confidence: high Reversibility: trivial
|
Repair evidence for exact head
Docs/changelog need no further source edit: neither promises PID liveness after nonconfirmation. Production semantics remain unchanged, with no private-owner/append-authority/Task activation claim. Both maintainers have been re-requested on this head; no inherited approval or autonomous merge. |
probepark
left a comment
There was a problem hiding this comment.
Code review (CI still running on 69228c0; approval decision follows once CI finishes).
Blocking: no blocking source findings in step 3. The previous failed regression has been repaired; current-head CI has not finished validating it.
CI: pending tests, TypeScript build, and aggregate affected-path validation. The coding-agent check passed. No failed check was reported by the approval gate.
Scope: +2477 / -645, 9 files. OCR selected four production files, totaling 799 added/deleted lines. Reviewed Python executor, SDK startup cleanup, descriptor wiring, and standalone Python generations.
Conventions: release fragment present; no generated-file changes; no labels. Released changelog sections are unchanged.
ocr: blocking 0 / nit 0
Notable:
packages/coding-agent/src/eval/py/executor.ts:322-365,735-847— requests are registered before preflight. Disposal snapshots active requests and initialization, publishes shutdown futures before abort callbacks, and awaits captured completions. Failed shutdown resources remain available for retries.packages/coding-agent/src/tools/python.ts:121-151,235-317— invocation context is captured before preflight. Generation cleanup retains its callback until success and joins invocation completion through transcript append.packages/coding-agent/src/sdk/session.ts:6069-6074,6109-6112,6134-6138— startup failure captures Python cleanup before asynchronous teardown and awaits that captured cleanup in both branches.packages/coding-agent/src/tools/descriptors.ts:351-363— the descriptor forwards current session metadata, admission checks, invocation tracking, and cleanup unregistration.packages/coding-agent/test/core/python-executor-owner-cleanup.test.ts:782-824— the repaired regression no longer infers PID liveness from an unconfirmed shutdown. It waits for PID exit, holds the same retained kernel's second shutdown, checks that cleanup remains pending, and awaits confirmation. This addresses review 5451509790's finding, subject to current-head CI.
Spec axis: the source matches the PR's bounded lifecycle claims. Legacy string-label ABA and filesystem append authority remain outside those claims, as documented.
Checked and clean: cancellation checks, queued-work admission, reentrant cleanup publication, retained shutdown retry, captured-generation transcript lifetime, and release-note scope. git diff --check passed.
Not assessed: full test-file review, runtime execution in this review pod, Windows/Darwin qualification, and final current-head CI results. Test-file names and the repaired regression were inspected; no local builds or tests were run.
Verification: python3 /opt/data/scripts/gajae-approve-gate.py 6499 --head 69228c03ed496233e0f72b91646d0dc1e1c133fe returned HOLD_CI (exit 10) at 2026-10-08T06:18:06Z. Coding-agent check coverage is present; no local-command gap was reported. Evidence is bound to this head, not the previous failed head. Re-run the approval gate after CI finishes.
Diff SHA-256: a14fbcc329ad0cec85668b487726f4ce698b65b463475ce9d78c4c1db7e6032f.
PR body has no verdict line and is author-owned; not edited. Approval is held, not granted. The previous requested-changes review remains until its regression is validated on this head.
Blocking findings resolved; superseded by the follow-up verdict.
probepark
left a comment
There was a problem hiding this comment.
Review (head 69228c0, gajae-reviewer on behalf of probepark)
Blocking: none. CI finished; no blocking findings remain.
Prior exact-head code review: #6499 (review)
This gate-only follow-up reused that source review; it did not re-review the diff.
CI: green. Run https://github.com/Yeachan-Heo/gajae-code/actions/runs/37735084526 succeeded on this exact head. The repaired owner-cleanup regression, session cleanup, Python tool, descriptor tests, coding-agent check, TypeScript build, and virtual integration passed.
Resolved finding: packages/coding-agent/test/core/python-executor-owner-cleanup.test.ts:782-824 repairs the invalid PID-liveness inference. Current-head CI validates the repaired regression: https://github.com/Yeachan-Heo/gajae-code/actions/runs/37735084526/job/113177278181
Repair context: #6499 (comment)
The previous probepark requested-changes review is superseded because its sole blocking finding is resolved.
Scope: +2477 / -645, 9 files; four reviewable production files total 799 added/deleted lines.
Conventions: release fragment present; no generated-file changes; no labels. Released changelog sections unchanged.
ocr: blocking 0 / nit 0
Verification: python3 /opt/data/scripts/gajae-approve-gate.py 6499 --head 69228c03ed496233e0f72b91646d0dc1e1c133fe returned ALLOW (exit 0) during this follow-up at 2026-10-08T06:43-06:44Z. No pending checks, failed checks, or missing local commands were reported. Required check:@gajae-code/coding-agent coverage is present. CI run head and OPEN PR head were rechecked at 2026-10-08T06:44:13Z; neither is stale. OCR rules/stamp were refreshed successfully. The binary/full-index diff digest was recomputed against base 3561a3cb4c68e5b8f6a289b129b602006141d5ae.
Not assessed here: local runtime execution, Windows/Darwin qualification, or full-stack claims outside the prior review boundary. No builds/tests ran in this review pod.
Verdict: gajae.pr-review-verdict.v1 merge-approved sha256:a14fbcc329ad0cec85668b487726f4ce698b65b463475ce9d78c4c1db7e6032f reviewer:human reviewer-id:probepark evidence:exact-head-ci-green;prior-source-review-blocking-zero;retry-regression-passed;coding-agent-check-covered
PR body verdict line count=0, not updated.
Body verdict line is owned by absent; not edited. Suggested verdict line: gajae.pr-review-verdict.v1 merge-approved sha256:a14fbcc329ad0cec85668b487726f4ce698b65b463475ce9d78c4c1db7e6032f reviewer:human reviewer-id:probepark evidence:exact-head-ci-green;prior-source-review-blocking-zero;retry-regression-passed;coding-agent-check-covered
Functional prerequisite
Register actual Python operations before availability, initialization and queue waits. Owner and global cleanup capture fresh physical work, publish all captured retirement futures before callbacks, join outstanding initialization/execution, and retain failed resources for real retries. Shared initializer retirement membership is separate from live ownership, so reentrant cleanup joins the original initialization without retiring another owner.
Standalone Python captures invocation cwd/session/settings before preflight and remains tracked through transcript append. Clearing A retains its cleanup callback while B runs; parent disposal joins both captured generations. SDK construction failure captures Python cleanup before asynchronous startup teardown.
Review boundary
Production cost: 799 added+deleted lines across four files against dev 3561a3c. Affected tests, docs and release fragment are included. Source review 161 and repair QA 169 are freshly bound CLEAR to head 69228c0; original joined QA 164 applies to unchanged contracts, not the revised retry fixture. These are advisories, not maintainer approval.
Parent verification
Limits and preserved evidence
Owner APIs remain legacy string labels. This prerequisite does not establish private owner authority, solve string-label ABA, grant transcript/audit append permission, activate persistent Task owners, or qualify Windows/Darwin native execution. Original reference, interrupted partials, authentic runtime/type/formatter failures and earlier over-budget candidates are preserved in the continuation ledger and artifacts. Full-stack completion remains open.
QA qualifications: the reentrant discriminator separately asserts the nested cleanup remains pending while B is held, not both cleanup callers; held append covers clear and actual new-session transition, not separate SDK disposal or isolation from Eval joins; zero-budget nonconfirmation proves retention/retry, not native termination refusal or broad forced-kill correctness. Agent 157's earlier own-test staging was not authorized in advance; parent staging and final verification do not backdate that permission.
Zero-budget shutdown races genuine exit observation against a timer; it does not universally guarantee an unconfirmed first result. The revised test still requires the actual unconfirmed branch, and no result is fabricated. This source caveat is distinct from the repaired post-result PID assertion; fresh head CI and maintainer approval remain required.
No autonomous merge: exact-current-head approving review by a write-access maintainer is required.