From 7a737eeb499c2e4551665747bddeaf78cb75aba1 Mon Sep 17 00:00:00 2001 From: TolaMironcenko Date: Thu, 15 Oct 2026 01:47:20 +0700 Subject: [PATCH] S00-014: Make successful runner process-tree cleanup idempotent Changes: Record completed termination on the Popen instance after all cleanup steps succeed; avoid repeated signals to the reaped group. Add deterministic repeated-EPERM and failed-descendant-kill retry regressions without suppressing permission errors. Validation: Four focused cleanup tests passed; make check passed 72 tests and plan/snapshot validation. git diff --check passed. Natural prior EPERM did not recur; injected EPERM reproduced the pre-fix failure. No application or parent-task acceptance is claimed. Task: S00-014 Agent: auto-ci-cd-14343249da62 Role: ci-cd Action: claim Status: review Task title: Fix repeated runner cleanup after a submitted parent is reaped --- planning/TASK_MATRIX.md | 4 +-- planning/stages/S00.md | 2 +- planning/task_matrix.csv | 2 +- scripts/runner.py | 8 +++++ .../auto-ci-cd-14343249da62.txt | 4 +++ .../S00-014-auto-ci-cd-14343249da62.md | 25 +++++++++++++ state/handoffs/S00-014.md | 8 +++++ state/tasks.json | 35 ++++++++++++++++--- tests/tooling/test_automation.py | 27 +++++++++++++- 9 files changed, 106 insertions(+), 9 deletions(-) create mode 100644 state/commit_messages/auto-ci-cd-14343249da62.txt create mode 100644 state/evidence/S00-014-auto-ci-cd-14343249da62.md create mode 100644 state/handoffs/S00-014.md diff --git a/planning/TASK_MATRIX.md b/planning/TASK_MATRIX.md index 9d3d78b..145dc6a 100644 --- a/planning/TASK_MATRIX.md +++ b/planning/TASK_MATRIX.md @@ -1,6 +1,6 @@ # Матрица задач -Generated; state revision **19**. Canonical status: state/tasks.json. +Generated; state revision **21**. Canonical status: state/tasks.json. | ID | Этап | Оператор | Reviewer | Статус | Owner | Зависимости | Задача | | --- | --- | --- | --- | --- | --- | --- | --- | @@ -16,7 +16,7 @@ Generated; state revision **19**. Canonical status: state/tasks.json. | [S00-011](tasks/S00-011.md) | S00 | ci-cd | security | done | auto-ci-cd-d105e8027730 | S00-001 | Restore verified npm registry access for the stack spike | | [S00-012](tasks/S00-012.md) | S00 | ci-cd | qa | blocked | auto-ci-cd-28868645424b | S00-001, S00-011, S00-013, S00-014 | Fix runner prompt delivery after partial stdin timeout | | [S00-013](tasks/S00-013.md) | S00 | ci-cd | qa | done | auto-ci-cd-5f68f03dd33b | S00-001, S00-011 | Bound oversized S21-006 runner context without truncating requirements | -| [S00-014](tasks/S00-014.md) | S00 | ci-cd | qa | todo | | S00-001, S00-011, S00-013 | Fix repeated runner cleanup after a submitted parent is reaped | +| [S00-014](tasks/S00-014.md) | S00 | ci-cd | qa | review | auto-ci-cd-14343249da62 | S00-001, S00-011, S00-013 | Fix repeated runner cleanup after a submitted parent is reaped | | [S00-010](tasks/S00-010.md) | S00 | qa | orchestrator | todo | | S00-001, S00-002, S00-003, S00-004, S00-005, S00-006, S00-007, S00-008, S00-009 | Gate: Профиль и проверка стека | | [S01-001](tasks/S01-001.md) | S01 | orchestrator | security | todo | | S00-010 | Описать identity/model contracts | | [S01-002](tasks/S01-002.md) | S01 | orchestrator | security | todo | | S00-010, S01-001 | Описать async account context | diff --git a/planning/stages/S00.md b/planning/stages/S00.md index 9e99d2e..a2756ee 100644 --- a/planning/stages/S00.md +++ b/planning/stages/S00.md @@ -18,7 +18,7 @@ - [S00-011](../tasks/S00-011.md): Restore verified npm registry access for the stack spike — ci-cd; done. - [S00-012](../tasks/S00-012.md): Fix runner prompt delivery after partial stdin timeout — ci-cd; blocked. - [S00-013](../tasks/S00-013.md): Bound oversized S21-006 runner context without truncating requirements — ci-cd; done. -- [S00-014](../tasks/S00-014.md): Fix repeated runner cleanup after a submitted parent is reaped — ci-cd; todo. +- [S00-014](../tasks/S00-014.md): Fix repeated runner cleanup after a submitted parent is reaped — ci-cd; review. - [S00-010](../tasks/S00-010.md): Gate: Профиль и проверка стека — qa; todo. ## Граница этапа diff --git a/planning/task_matrix.csv b/planning/task_matrix.csv index dd51446..e0f2aca 100644 --- a/planning/task_matrix.csv +++ b/planning/task_matrix.csv @@ -11,7 +11,7 @@ S00-009,S00,orchestrator,security,todo,,"S00-003, S00-004, S00-005, S00-006, S00 S00-011,S00,ci-cd,security,done,auto-ci-cd-d105e8027730,S00-001,Restore verified npm registry access for the stack spike S00-012,S00,ci-cd,qa,blocked,auto-ci-cd-28868645424b,"S00-001, S00-011, S00-013, S00-014",Fix runner prompt delivery after partial stdin timeout S00-013,S00,ci-cd,qa,done,auto-ci-cd-5f68f03dd33b,"S00-001, S00-011",Bound oversized S21-006 runner context without truncating requirements -S00-014,S00,ci-cd,qa,todo,,"S00-001, S00-011, S00-013",Fix repeated runner cleanup after a submitted parent is reaped +S00-014,S00,ci-cd,qa,review,auto-ci-cd-14343249da62,"S00-001, S00-011, S00-013",Fix repeated runner cleanup after a submitted parent is reaped S00-010,S00,qa,orchestrator,todo,,"S00-001, S00-002, S00-003, S00-004, S00-005, S00-006, S00-007, S00-008, S00-009",Gate: Профиль и проверка стека S01-001,S01,orchestrator,security,todo,,S00-010,Описать identity/model contracts S01-002,S01,orchestrator,security,todo,,"S00-010, S01-001",Описать async account context diff --git a/scripts/runner.py b/scripts/runner.py index f7b431b..6b97e23 100644 --- a/scripts/runner.py +++ b/scripts/runner.py @@ -335,6 +335,10 @@ The coordinator commits each transition before launching another agent. def stop_process_tree(process): + # Transition/deadline handling and finally share this Popen instance. Once + # cleanup succeeds, do not signal its reaped (possibly reused) group again. + if getattr(process, '_luma_tree_stopped', False) is True: + return if os.name == 'posix': try: os.killpg(process.pid, signal.SIGTERM) @@ -360,6 +364,10 @@ def stop_process_tree(process): except ProcessLookupError: pass + # Set only after all termination steps succeed. Permission errors must + # propagate, including when the parent exited but a descendant is live. + process._luma_tree_stopped = True + def task_scope(tracker, task_id): return tracker.task(task_id)['scope'] diff --git a/state/commit_messages/auto-ci-cd-14343249da62.txt b/state/commit_messages/auto-ci-cd-14343249da62.txt new file mode 100644 index 0000000..45db4ea --- /dev/null +++ b/state/commit_messages/auto-ci-cd-14343249da62.txt @@ -0,0 +1,4 @@ +Make successful runner process-tree cleanup idempotent + +Changes: Record completed termination on the Popen instance after all cleanup steps succeed; avoid repeated signals to the reaped group. Add deterministic repeated-EPERM and failed-descendant-kill retry regressions without suppressing permission errors. +Validation: Four focused cleanup tests passed; make check passed 72 tests and plan/snapshot validation. git diff --check passed. Natural prior EPERM did not recur; injected EPERM reproduced the pre-fix failure. No application or parent-task acceptance is claimed. diff --git a/state/evidence/S00-014-auto-ci-cd-14343249da62.md b/state/evidence/S00-014-auto-ci-cd-14343249da62.md new file mode 100644 index 0000000..207f7bb --- /dev/null +++ b/state/evidence/S00-014-auto-ci-cd-14343249da62.md @@ -0,0 +1,25 @@ +# S00-014 executor evidence + +Agent: auto-ci-cd-14343249da62; role: ci-cd; date: 2026-10-15. +Result: repeated process-tree cleanup correction, pending independent QA review. + +Input: assigned card/context and exact NFR-007, startup/workflow documents, S00-012 preserved implementation/evidence/handoff, S00-012/S00-014 blocker reports, accepted ADR 0000 and direct dependency evidence/reviews/handoffs. Output: idempotent successful process-tree termination for the same Popen instance. Acceptance: no repeated signal after successful cleanup, real descendant termination on timeout/submit, and termination errors still propagate. + +## Actual change + +stop_process_tree records completion on the Popen object only after its existing termination steps all succeed. The transition/deadline call and finally therefore share a completed cleanup result. The first call still sends SIGTERM, waits/reaps the parent and sends SIGKILL to descendants even if the parent exited. An inaccessible live group raises PermissionError; no completion marker is set on failure, and a later retry executes termination again. No permission error is suppressed. Existing monitoring, stdin transport, prompt budgets and workflow transitions are unchanged. + +Added two deterministic signal regressions: a completed cleanup is followed by injected EPERM if another signal is attempted; a failed final SIGKILL must propagate and allow a complete retry. Existing real subprocess regressions launch a SIGTERM-ignoring descendant that would write source after 1.5 seconds, and check that both submit and timeout cleanup prevent the write. + +## Actual commands and results + +- python3 scripts/tasks.py resume --agent auto-ci-cd-14343249da62 and show S00-014: exit 0; already assigned in_progress, no repeated claim. +- Initial python3 -m unittest discover -s tests/tooling -p test_automation.py -k submit_transition_stops_source_writing_descendant -v: exit 0, 26 tests in 11.180s. The custom load_tests ignores -k and ran the entire automation suite. The previous sandbox EPERM did not recur naturally in this session; no claim of a fresh real-syscall reproduction is made. +- Before runner change: PYTHONPATH=tests/tooling python3 -m unittest test_automation.RunnerTests.test_completed_cleanup_does_not_signal_reaped_group_again test_automation.RunnerTests.test_failed_descendant_kill_is_not_marked_complete -v: exit 1; two tests, completion test errored with injected PermissionError at runner.py os.killpg(SIGTERM), failure/retry test passed. This deterministically reproduces the repeated-signal cause without launching a model. +- After runner change: PYTHONPATH=tests/tooling python3 -m unittest test_automation.RunnerTests.test_completed_cleanup_does_not_signal_reaped_group_again test_automation.RunnerTests.test_failed_descendant_kill_is_not_marked_complete test_automation.RunnerTests.test_submit_transition_stops_source_writing_descendant test_automation.RunnerTests.test_timeout_stops_source_writing_descendant -v: exit 0; 4 tests in 3.981s. Real descendants cannot perform the delayed source write, submit remains review, timeout creates its valid remediation, and scope violations are empty. +- make check > /private/tmp/luma-S00-014-check.log 2>&1: exit 0; 72 tests in 17.247s; validated 279 tasks, 22 stages, 221 requirements, 211 allocated Web requirements, revision 20. Includes complete delayed-reader 140024-byte UTF-8/EOF, fresh subprocess chain, transition/deadline, scope, prompt budgets, snapshot and CLI checks. +- git diff --check: exit 0; read-only source diff inspected. Source changes limited to scripts/runner.py and tests/tooling/test_automation.py. Initial state/tasks.json change was coordinator claim metadata. + +## Limits + +No model, product server, browser or application readiness check was performed. The earlier EPERM policy cause remains unproven; its failure condition is covered by deterministic signal injection and the preserved real subprocess checks. Windows-specific cleanup was not exercised on this macOS/Python 3.9.6 harness. Successful SIGKILL retains the existing operating-system termination guarantee; no new process enumeration or permission bypass is introduced. No dependency, immutable spec, contract, Git configuration or production transport changed. Coordinator owns the implementation commit and fresh independent reviewer. Parent S00-012 and root S00-002 are not accepted by this task. diff --git a/state/handoffs/S00-014.md b/state/handoffs/S00-014.md new file mode 100644 index 0000000..a9f5170 --- /dev/null +++ b/state/handoffs/S00-014.md @@ -0,0 +1,8 @@ +# S00-014 handoff + +Executor: auto-ci-cd-14343249da62 (ci-cd). +Result: successful stop_process_tree cleanup is recorded on the Popen object, preventing repeated signals from transition/deadline handling and finally. Completion is recorded only after all existing termination steps succeed; PermissionError still propagates and failed cleanup can retry. Source scope: scripts/runner.py and tests/tooling/test_automation.py. + +Ready for independent QA review. Inspect actual source diff and commit message; repeat make check and the two deterministic signal tests. Verify initial SIGTERM/final SIGKILL ordering, no repeated signaling after success, no suppression/marking on inaccessible live descendants, and both preserved real SIGTERM-ignoring descendant regressions. The natural prior EPERM did not recur in this fresh session; deterministic injected EPERM failed before the fix and passes after it. Do not describe it as a fresh real-syscall reproduction. + +Focused checks: 4 tests passed, exit 0. make check: 72 tests passed in 17.247s, exit 0, including delayed UTF-8/EOF, deadline, transition, fresh sessions, scope and unchanged prompt budgets. Evidence: state/evidence/S00-014-auto-ci-cd-14343249da62.md. No blocker or app/parent-task acceptance claimed. Coordinator must commit implementation and launch a fresh QA reviewer; executor does not set done, continue S00-012, or mutate Git. diff --git a/state/tasks.json b/state/tasks.json index 5cc3768..6f5beb1 100644 --- a/state/tasks.json +++ b/state/tasks.json @@ -1,6 +1,6 @@ { "schema_version": 1, - "revision": 19, + "revision": 21, "tasks": { "S00-001": { "status": "done", @@ -1165,10 +1165,14 @@ "completed_at": "2026-10-14T18:42:03+00:00" }, "S00-014": { - "status": "todo", - "owner": null, + "status": "review", + "owner": "auto-ci-cd-14343249da62", "external_wait": false, - "reason": null + "reason": null, + "started_at": "2026-10-14T18:45:17+00:00", + "updated_at": "2026-10-14T18:47:20+00:00", + "evidence": "state/evidence/S00-014-auto-ci-cd-14343249da62.md", + "handoff": "state/handoffs/S00-014.md" } }, "events": [ @@ -1406,6 +1410,29 @@ "from": "in_progress", "to": "blocked", "reason": "Submit-transition descendant regression fails: repeated stop_process_tree after parent reap raises EPERM at os.killpg; launch OSError handling then attempts invalid escalation from review state." + }, + { + "revision": 20, + "at": "2026-10-14T18:45:17+00:00", + "action": "claim", + "task": "S00-014", + "agent": "auto-ci-cd-14343249da62", + "role": "ci-cd", + "from": "todo", + "to": "in_progress", + "reason": null + }, + { + "revision": 21, + "at": "2026-10-14T18:47:20+00:00", + "action": "submit", + "task": "S00-014", + "agent": "auto-ci-cd-14343249da62", + "role": null, + "from": "in_progress", + "to": "review", + "reason": null, + "evidence": "state/evidence/S00-014-auto-ci-cd-14343249da62.md" } ], "retired_agents": [ diff --git a/tests/tooling/test_automation.py b/tests/tooling/test_automation.py index f915443..7f4f63c 100644 --- a/tests/tooling/test_automation.py +++ b/tests/tooling/test_automation.py @@ -6,7 +6,7 @@ import subprocess import sys import unittest import uuid -from unittest.mock import patch +from unittest.mock import Mock, patch from test_tasks import WorkflowTests, ROOT, MODULE @@ -319,6 +319,31 @@ else: def test_submit_transition_stops_source_writing_descendant(self): self._check_descendant_cleanup(transition=True) + @unittest.skipUnless(sys.platform != 'win32', 'POSIX process-group cleanup') + def test_completed_cleanup_does_not_signal_reaped_group_again(self): + process = Mock(pid=12345, returncode=-15) + process.poll.return_value = -15 + with patch.object(RUN_MODULE.os, 'killpg') as killpg: + RUN_MODULE.stop_process_tree(process) + self.assertEqual(killpg.call_args_list, + [unittest.mock.call(12345, RUN_MODULE.signal.SIGTERM), + unittest.mock.call(12345, RUN_MODULE.signal.SIGKILL)]) + killpg.side_effect = PermissionError('Reaped process group inaccessible') + RUN_MODULE.stop_process_tree(process) + self.assertEqual(killpg.call_count, 2) + + @unittest.skipUnless(sys.platform != 'win32', 'POSIX process-group cleanup') + def test_failed_descendant_kill_is_not_marked_complete(self): + process = Mock(pid=12345, returncode=-15) + process.poll.return_value = -15 + with patch.object(RUN_MODULE.os, 'killpg', + side_effect=[None, PermissionError('Live descendant inaccessible')]) as killpg: + with self.assertRaises(PermissionError): + RUN_MODULE.stop_process_tree(process) + killpg.side_effect = None + RUN_MODULE.stop_process_tree(process) + self.assertEqual(killpg.call_count, 4) + def test_scope_matcher_checks_modified_created_deleted_files(self): self.assertEqual(RUN_MODULE.scope_violations({'a': 'old', 'gone': 'old'}, {'a': 'new', 'created': 'new'}, ['a']), ['created', 'gone'])