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
This commit is contained in:
wt committed 2026-10-15 01:47:20 +07:00
1 parent 49c054aaf9
commit 7a737eeb49
9 files changed
+106 -9

No files matched your search

+2 -2
View File
@@ -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 |
+1 -1
View File
@@ -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.
## Граница этапа
+1 -1
View File
@@ -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
1 id stage operator reviewer status owner depends_on title
11 S00-011 S00 ci-cd security done auto-ci-cd-d105e8027730 S00-001 Restore verified npm registry access for the stack spike
12 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
13 S00-013 S00 ci-cd qa done auto-ci-cd-5f68f03dd33b S00-001, S00-011 Bound oversized S21-006 runner context without truncating requirements
14 S00-014 S00 ci-cd qa todo review auto-ci-cd-14343249da62 S00-001, S00-011, S00-013 Fix repeated runner cleanup after a submitted parent is reaped
15 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: Профиль и проверка стека
16 S01-001 S01 orchestrator security todo S00-010 Описать identity/model contracts
17 S01-002 S01 orchestrator security todo S00-010, S01-001 Описать async account context
+8
View File
@@ -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']
@@ -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.
@@ -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.
+8
View File
@@ -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.
+31 -4
View File
@@ -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": [
+26 -1
View File
@@ -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'])