Scaled-Tech-Consulting/Gitea-Tools
2
0
Fork 0
Code Issues 66 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

runtime_recovery_guard segment splitter misses & and subshell forms, so a real daemon kill classifies as clean #787

New Issue
Closed
opened 2026-07-21 18:07:57 -05:00 by jcwalker3 · 2 comments
jcwalker3 commented 2026-07-21 18:07:57 -05:00
Owner
Copy Link
Copy Source

Follow-up defect against #630, landed by PR #786 at merge commit 35e94e107c.

Problem

runtime_recovery_guard._SEGMENT_SPLIT_RE splits a compound command line so each simple command can be classified independently:

_SEGMENT_SPLIT_RE = re.compile(r"(?:\|\||&&|\||;|\n)")

The comment above it states that it splits "on shell separators", but the background-job separator & is not in the alternation, and a subshell wrapper ( ... ) is never stripped before tokenising. _analyse_kill_segment inspects only the first command token of a segment, so in both forms the kill verb is never reached and the command is reported as containing no process kill at all.

Observed behavior

Measured directly against the merged implementation:

sleep 1 & pkill -f mcp_server.py   ->  process_kill False, contamination False
(pkill -f mcp_server.py)           ->  process_kill False, contamination False
pkill -f mcp_server.py             ->  process_kill True,  contamination True

All three kill the daemons. The first is an ordinary command line, not an evasion attempt.

Impact

  • #630 acceptance criterion 1 is unmet for these inputs: the detector reports no process kill for a command that kills the MCP daemons.
  • The miss propagates to acceptance criterion 3. The contamination marker is what drives _enforce_runtime_recovery_contamination_gate; when no marker is written, review, merge, close and completion mutations stay reachable after a real kill. The guard silently reports a clean session.
  • gitea_record_daemon_process_kill_attempt returns contaminated: false and writes nothing, so there is also no forensic record for a later reconciler audit.

Scope

  1. Add the background separator to _SEGMENT_SPLIT_RE while keeping the two-character logical form matched first, for example (?:\|\||&&|[|;&\n]).
  2. Strip a leading ( and a trailing ) from a segment before tokenising, so a subshell-wrapped kill reaches the classifier.
  3. Add cases to tests/test_issue_630_runtime_recovery_guard.py proving both forms classify as contamination. The file currently contains no case with a background separator or a subshell.
  4. Re-confirm the existing no-false-positive cases still hold, in particular pkill -u mcpuser -f myapp, ps aux | grep mcp_server, a bare kill of an unknown pid, and a commit message quoting the kill string.

Non-goals

  • Do not change the contamination model, the gated-task set, the marker lifecycle, or the reconciler-only clearing path. Those were verified correct at head 1ec4672fad.
  • Do not widen detection to automatic ingestion from other tools. That is a separate concern, recorded below.

Acceptance criteria

  1. sleep 1 & pkill -f mcp_server.py classifies as contamination with reason class manual_daemon_kill.
  2. (pkill -f mcp_server.py) classifies as contamination with reason class manual_daemon_kill.
  3. A command whose only kill is aimed elsewhere, and a bare kill of a pid not known to be a daemon, keep their current non-contaminating outcomes.
  4. gitea_record_daemon_process_kill_attempt writes a durable marker for both newly detected forms.
  5. Tests cover both forms plus the four no-false-positive cases named in scope item 4.

Provenance

Found during independent review of PR #786 at head 1ec4672fad. The reviewer verdict recording this finding could not be posted: a concurrent session held the reviewer lease (session 90916-0215a28b2122, lease comment 13774) and approved and merged the PR before the finding could be submitted. The lease was not stolen. This issue is the sanctioned route for the finding.

Related observation, not in this scope

Detection is opt-in: a marker is written only when a session calls gitea_record_daemon_process_kill_attempt. The incident behind #630 ran a kill and then closed an issue, and such a session has no reason to call the tool. gitea_record_pre_review_command and gitea_assess_gitea_operation_path already receive the same command strings and already classify them, so feeding the classifier from those two points would make detection automatic. That deserves its own issue rather than being folded in here.

Follow-up defect against #630, landed by PR #786 at merge commit 35e94e107c32cfdc4b9ab9a95cb4e94df94077d4. ## Problem `runtime_recovery_guard._SEGMENT_SPLIT_RE` splits a compound command line so each simple command can be classified independently: ```python _SEGMENT_SPLIT_RE = re.compile(r"(?:\|\||&&|\||;|\n)") ``` The comment above it states that it splits "on shell separators", but the background-job separator `&` is not in the alternation, and a subshell wrapper `( ... )` is never stripped before tokenising. `_analyse_kill_segment` inspects only the first command token of a segment, so in both forms the kill verb is never reached and the command is reported as containing no process kill at all. ## Observed behavior Measured directly against the merged implementation: ``` sleep 1 & pkill -f mcp_server.py -> process_kill False, contamination False (pkill -f mcp_server.py) -> process_kill False, contamination False pkill -f mcp_server.py -> process_kill True, contamination True ``` All three kill the daemons. The first is an ordinary command line, not an evasion attempt. ## Impact * #630 acceptance criterion 1 is unmet for these inputs: the detector reports no process kill for a command that kills the MCP daemons. * The miss propagates to acceptance criterion 3. The contamination marker is what drives `_enforce_runtime_recovery_contamination_gate`; when no marker is written, review, merge, close and completion mutations stay reachable after a real kill. The guard silently reports a clean session. * `gitea_record_daemon_process_kill_attempt` returns `contaminated: false` and writes nothing, so there is also no forensic record for a later reconciler audit. ## Scope 1. Add the background separator to `_SEGMENT_SPLIT_RE` while keeping the two-character logical form matched first, for example `(?:\|\||&&|[|;&\n])`. 2. Strip a leading `(` and a trailing `)` from a segment before tokenising, so a subshell-wrapped kill reaches the classifier. 3. Add cases to `tests/test_issue_630_runtime_recovery_guard.py` proving both forms classify as contamination. The file currently contains no case with a background separator or a subshell. 4. Re-confirm the existing no-false-positive cases still hold, in particular `pkill -u mcpuser -f myapp`, `ps aux | grep mcp_server`, a bare `kill` of an unknown pid, and a commit message quoting the kill string. ## Non-goals * Do not change the contamination model, the gated-task set, the marker lifecycle, or the reconciler-only clearing path. Those were verified correct at head 1ec4672fad897a7391026ace5dd19a47351cde16. * Do not widen detection to automatic ingestion from other tools. That is a separate concern, recorded below. ## Acceptance criteria 1. `sleep 1 & pkill -f mcp_server.py` classifies as contamination with reason class `manual_daemon_kill`. 2. `(pkill -f mcp_server.py)` classifies as contamination with reason class `manual_daemon_kill`. 3. A command whose only kill is aimed elsewhere, and a bare `kill` of a pid not known to be a daemon, keep their current non-contaminating outcomes. 4. `gitea_record_daemon_process_kill_attempt` writes a durable marker for both newly detected forms. 5. Tests cover both forms plus the four no-false-positive cases named in scope item 4. ## Provenance Found during independent review of PR #786 at head 1ec4672fad897a7391026ace5dd19a47351cde16. The reviewer verdict recording this finding could not be posted: a concurrent session held the reviewer lease (session `90916-0215a28b2122`, lease comment 13774) and approved and merged the PR before the finding could be submitted. The lease was not stolen. This issue is the sanctioned route for the finding. ## Related observation, not in this scope Detection is opt-in: a marker is written only when a session calls `gitea_record_daemon_process_kill_attempt`. The incident behind #630 ran a kill and then closed an issue, and such a session has no reason to call the tool. `gitea_record_pre_review_command` and `gitea_assess_gitea_operation_path` already receive the same command strings and already classify them, so feeding the classifier from those two points would make detection automatic. That deserves its own issue rather than being folded in here.
jcwalker3 added status:pr-open and removed status:ready labels 2026-07-21 19:57:03 -05:00
jcwalker3 commented 2026-07-21 19:58:40 -05:00
Author
Owner
Copy Link
Copy Source

[THREAD STATE LEDGER] Issue #787 — author implementation published as PR #789 at head 6b58f04

What is true now:

  • Server-side decision state: no review verdict exists on PR #789, no merge event has occurred, and issue #787 remains in open state now carrying status:pr-open.
  • Local verdict/state: author implementation complete on branch fix/issue-787-kill-segment-separators; published head 6b58f04d396b881d5d242eacac45bcfdfefe2d00, parent 35e94e107c32cfdc4b9ab9a95cb4e94df94077d4; the author issue lock for #787 is still held by this session.
  • Latest known validation: focused suite tests/test_issue_630_runtime_recovery_guard.py 55 passed; full suite 4181 passed with 11 failures proven pre-existing on unmodified master by a stash-and-rerun at 35e94e10.

What changed:

  • runtime_recovery_guard._SEGMENT_SPLIT_RE widened to (?:\|\||&&|[|;&\n]), adding the background-job separator while keeping the two-character logical forms first in the alternation so && and || are consumed whole.
  • New runtime_recovery_guard._strip_subshell(), applied inside _split_segments(), removes a leading ( and a trailing ) (nested wrappers too) before tokenising.
  • tests/test_issue_630_runtime_recovery_guard.py grew from 46 to 55 cases: both forms named in the issue, four further background/subshell variants, an invariant test that &&/|| still split whole, two record-tool cases proving a durable marker is written for each new form, and the two no-false-positive cases the issue named that previously had no coverage.
  • PR #789 was opened against master and the issue label set moved to status:pr-open.

What is blocked:

  • Blocker classification: no blocker
  • Nothing prevents review. PR #789 is open at head 6b58f04d396b881d5d242eacac45bcfdfefe2d00 against base master 35e94e107c32cfdc4b9ab9a95cb4e94df94077d4, which is the branch parent, so no rebase is outstanding.

Who/what acts next:

  • Next actor: reviewer
  • Required action: perform a fresh native review of PR #789 at head 6b58f04d396b881d5d242eacac45bcfdfefe2d00 from the reviewer namespace, and record the verdict through the review API.
  • Do not do: the author role must not review, approve, or merge this PR; do not carry over any review verdict from PR #786; do not advance the branch head before a verdict is recorded against the current head.

Canonical Issue State

STATE:
author-implementation-published

WHO_IS_NEXT:
reviewer

NEXT_ACTION:
Fresh native review of PR #789 at head 6b58f04d39, verifying acceptance criteria 1 through 5 against the diff and the test file.

NEXT_PROMPT:

Review PR #789 in Scaled-Tech-Consulting/Gitea-Tools (remote prgs) at head
6b58f04d396b881d5d242eacac45bcfdfefe2d00, base master
35e94e107c32cfdc4b9ab9a95cb4e94df94077d4.

Use only the gitea-reviewer MCP namespace. Do not use the author, merger, or
reconciler namespaces. Do not merge.

Verify against issue #787 acceptance criteria:
1. `sleep 1 & pkill -f mcp_server.py` classifies as contamination with reason
   class manual_daemon_kill.
2. `(pkill -f mcp_server.py)` classifies as contamination with reason class
   manual_daemon_kill.
3. A kill aimed elsewhere, and a bare `kill` of an unknown pid, keep their
   non-contaminating outcomes.
4. `gitea_record_daemon_process_kill_attempt` writes a durable marker for both
   newly detected forms.
5. Tests cover both forms plus the four no-false-positive cases in scope item 4.

Check specifically that the widened alternation cannot break `&&` or `||` into
single characters, and that `_strip_subshell` cannot swallow a legitimate
operand. Confirm the contamination model, gated-task set, marker lifecycle, and
reconciler-only clearing path are untouched.

Record the verdict through the review API at the head SHA above.

WHAT_HAPPENED:
The segment splitter in runtime_recovery_guard omitted the background-job separator & and never stripped a subshell wrapper, so _analyse_kill_segment saw a non-kill token in command position and reported no process kill for two command lines that do kill the daemons. Both fixes are in place, the classifier now reports contamination with reason class manual_daemon_kill for each, and the record tool writes a durable marker for both. Direct-command behaviour is unchanged.

WHY:
The missing marker propagated to _enforce_runtime_recovery_contamination_gate. With no marker written, review, merge, close and completion mutations stayed reachable after a real daemon kill and the session was reported as clean, which defeats the #630 guard for these inputs and leaves no forensic record for a later reconciler audit.

RELATED_PRS:

  • PR #789 — this fix, open at head 6b58f04d396b881d5d242eacac45bcfdfefe2d00, base master, awaiting a first review verdict.
  • PR #786 — landed the #630 guard at master 35e94e10; this issue is the follow-up defect found during its review and is the sanctioned route for that finding.

BLOCKERS:
none

VALIDATION:

  • Pre-fix behaviour reproduced from a copy of runtime_recovery_guard.py at 35e94e10: sleep 1 & pkill -f mcp_server.py and (pkill -f mcp_server.py) both gave process_kill False, contamination False, reason_class None, while pkill -f mcp_server.py gave process_kill True, contamination True, reason_class manual_daemon_kill. This matches the observed behaviour recorded in the issue.
  • Post-fix on head 6b58f04: both forms give process_kill True, contamination True, reason_class manual_daemon_kill, ambiguous False. pkill -u mcpuser -f myapp stays non-contaminating and unambiguous, ps aux | grep mcp_server stays a non-kill, kill 31337 stays ambiguous and non-contaminating, and a commit message quoting the kill string stays a non-kill.
  • Focused suite: python3.14 -m pytest tests/test_issue_630_runtime_recovery_guard.py -q -s gave 55 passed in 0.82s.
  • Full suite on the branch: 11 failed, 4181 passed, 6 skipped, 493 subtests passed in 79.62s.
  • Pre-existing-failure proof: the two changed files were stashed inside the locked worktree, leaving the tree at 35e94e10; re-running the five affected modules gave the identical 11 failures (11 failed, 254 passed) — 6 in tests/test_commit_payloads.py, 2 in tests/test_issue_702_review_findings_f1_f6.py, 1 in tests/test_mcp_server.py::TestPreflightVerification, 1 in tests/test_post_merge_moot_lease.py, 1 in tests/test_reconciler_supersession_close.py. The stash was then restored and the focused suite re-run at 55 passed.
  • Publication integrity: the remote commit 6b58f04 carries tree b1522636eef0ed6920195ac33373349c55981283, matching the locally tested commit 84526e9 exactly, with parent 35e94e10.

LAST_UPDATED_BY:
prgs-author (jcwalker3), MCP author namespace, daemon pid 84100

[THREAD STATE LEDGER] Issue #787 — author implementation published as PR #789 at head 6b58f04 What is true now: - Server-side decision state: no review verdict exists on PR #789, no merge event has occurred, and issue #787 remains in open state now carrying `status:pr-open`. - Local verdict/state: author implementation complete on branch `fix/issue-787-kill-segment-separators`; published head `6b58f04d396b881d5d242eacac45bcfdfefe2d00`, parent `35e94e107c32cfdc4b9ab9a95cb4e94df94077d4`; the author issue lock for #787 is still held by this session. - Latest known validation: focused suite `tests/test_issue_630_runtime_recovery_guard.py` 55 passed; full suite 4181 passed with 11 failures proven pre-existing on unmodified master by a stash-and-rerun at `35e94e10`. What changed: - `runtime_recovery_guard._SEGMENT_SPLIT_RE` widened to `(?:\|\||&&|[|;&\n])`, adding the background-job separator while keeping the two-character logical forms first in the alternation so `&&` and `||` are consumed whole. - New `runtime_recovery_guard._strip_subshell()`, applied inside `_split_segments()`, removes a leading `(` and a trailing `)` (nested wrappers too) before tokenising. - `tests/test_issue_630_runtime_recovery_guard.py` grew from 46 to 55 cases: both forms named in the issue, four further background/subshell variants, an invariant test that `&&`/`||` still split whole, two record-tool cases proving a durable marker is written for each new form, and the two no-false-positive cases the issue named that previously had no coverage. - PR #789 was opened against master and the issue label set moved to `status:pr-open`. What is blocked: - Blocker classification: no blocker - Nothing prevents review. PR #789 is open at head `6b58f04d396b881d5d242eacac45bcfdfefe2d00` against base master `35e94e107c32cfdc4b9ab9a95cb4e94df94077d4`, which is the branch parent, so no rebase is outstanding. Who/what acts next: - Next actor: reviewer - Required action: perform a fresh native review of PR #789 at head `6b58f04d396b881d5d242eacac45bcfdfefe2d00` from the reviewer namespace, and record the verdict through the review API. - Do not do: the author role must not review, approve, or merge this PR; do not carry over any review verdict from PR #786; do not advance the branch head before a verdict is recorded against the current head. ## Canonical Issue State STATE: author-implementation-published WHO_IS_NEXT: reviewer NEXT_ACTION: Fresh native review of PR #789 at head 6b58f04d396b881d5d242eacac45bcfdfefe2d00, verifying acceptance criteria 1 through 5 against the diff and the test file. NEXT_PROMPT: ```text Review PR #789 in Scaled-Tech-Consulting/Gitea-Tools (remote prgs) at head 6b58f04d396b881d5d242eacac45bcfdfefe2d00, base master 35e94e107c32cfdc4b9ab9a95cb4e94df94077d4. Use only the gitea-reviewer MCP namespace. Do not use the author, merger, or reconciler namespaces. Do not merge. Verify against issue #787 acceptance criteria: 1. `sleep 1 & pkill -f mcp_server.py` classifies as contamination with reason class manual_daemon_kill. 2. `(pkill -f mcp_server.py)` classifies as contamination with reason class manual_daemon_kill. 3. A kill aimed elsewhere, and a bare `kill` of an unknown pid, keep their non-contaminating outcomes. 4. `gitea_record_daemon_process_kill_attempt` writes a durable marker for both newly detected forms. 5. Tests cover both forms plus the four no-false-positive cases in scope item 4. Check specifically that the widened alternation cannot break `&&` or `||` into single characters, and that `_strip_subshell` cannot swallow a legitimate operand. Confirm the contamination model, gated-task set, marker lifecycle, and reconciler-only clearing path are untouched. Record the verdict through the review API at the head SHA above. ``` WHAT_HAPPENED: The segment splitter in `runtime_recovery_guard` omitted the background-job separator `&` and never stripped a subshell wrapper, so `_analyse_kill_segment` saw a non-kill token in command position and reported no process kill for two command lines that do kill the daemons. Both fixes are in place, the classifier now reports contamination with reason class `manual_daemon_kill` for each, and the record tool writes a durable marker for both. Direct-command behaviour is unchanged. WHY: The missing marker propagated to `_enforce_runtime_recovery_contamination_gate`. With no marker written, review, merge, close and completion mutations stayed reachable after a real daemon kill and the session was reported as clean, which defeats the #630 guard for these inputs and leaves no forensic record for a later reconciler audit. RELATED_PRS: - PR #789 — this fix, open at head `6b58f04d396b881d5d242eacac45bcfdfefe2d00`, base master, awaiting a first review verdict. - PR #786 — landed the #630 guard at master `35e94e10`; this issue is the follow-up defect found during its review and is the sanctioned route for that finding. BLOCKERS: none VALIDATION: - Pre-fix behaviour reproduced from a copy of `runtime_recovery_guard.py` at `35e94e10`: `sleep 1 & pkill -f mcp_server.py` and `(pkill -f mcp_server.py)` both gave process_kill False, contamination False, reason_class None, while `pkill -f mcp_server.py` gave process_kill True, contamination True, reason_class manual_daemon_kill. This matches the observed behaviour recorded in the issue. - Post-fix on head `6b58f04`: both forms give process_kill True, contamination True, reason_class manual_daemon_kill, ambiguous False. `pkill -u mcpuser -f myapp` stays non-contaminating and unambiguous, `ps aux | grep mcp_server` stays a non-kill, `kill 31337` stays ambiguous and non-contaminating, and a commit message quoting the kill string stays a non-kill. - Focused suite: `python3.14 -m pytest tests/test_issue_630_runtime_recovery_guard.py -q -s` gave 55 passed in 0.82s. - Full suite on the branch: 11 failed, 4181 passed, 6 skipped, 493 subtests passed in 79.62s. - Pre-existing-failure proof: the two changed files were stashed inside the locked worktree, leaving the tree at `35e94e10`; re-running the five affected modules gave the identical 11 failures (11 failed, 254 passed) — 6 in `tests/test_commit_payloads.py`, 2 in `tests/test_issue_702_review_findings_f1_f6.py`, 1 in `tests/test_mcp_server.py::TestPreflightVerification`, 1 in `tests/test_post_merge_moot_lease.py`, 1 in `tests/test_reconciler_supersession_close.py`. The stash was then restored and the focused suite re-run at 55 passed. - Publication integrity: the remote commit `6b58f04` carries tree `b1522636eef0ed6920195ac33373349c55981283`, matching the locally tested commit `84526e9` exactly, with parent `35e94e10`. LAST_UPDATED_BY: prgs-author (jcwalker3), MCP author namespace, daemon pid 84100
jcwalker3 commented 2026-07-21 20:49:56 -05:00
Author
Owner
Copy Link
Copy Source

Author remediation of review #497 — PR #789 at new head aa4fe1cc

Issue #787 · PR #789 · previous head 6b58f04d396b881d5d242eacac45bcfdfefe2d00 · new head aa4fe1cc7bd2d6f07ea35775a5c5292d94a938e0

Review #497 and disposition of each finding

Blocking review #497 (REQUEST_CHANGES, reviewer sysadmin, recorded against head 6b58f04) is undismissed but now stale against the live head, and author_pushed_after_request_changes reads true.

  • F1 — BLOCKING, remediated. Quote-unaware splitting on & made quoted text carrying an ampersand plus a kill verb classify as a manual daemon kill. Fixed at the class level with quote-aware segmentation, not by special-casing the three named strings. New coverage asserts contamination False for each of them, including the full sleep 1 & pkill -f mcp_server.py string quoted inside a non-executing command, which is the AC5 case the reviewer found unprotected.
  • F2 — MINOR, corrected. The prior body claimed 46 cases before the change and 9 added. Re-measured in dedicated detached worktrees: base 35e94e10 47, rejected head 6b58f04 55, so the rejected head added 8, not 9. The PR body now carries 47 / 55 / 64 and the corrected arithmetic.
  • F3 — INFORMATIONAL, fixed rather than deferred. Both observations were correct and neither changed a verdict at the rejected head, so neither was load-bearing for this issue. They are corrected here only because F1 required reworking the same splitter, which is the condition under which the reviewer asked for them to be tightened. _strip_subshell now removes a trailing ) only when it closes a leading ( the same call stripped, so kill $(pgrep -f myapp) is no longer mangled; _is_redirection keeps 2>&1, >&2 and &>log out of separator position. No unrelated expansion beyond these two.

Quote-aware parsing behaviour

_SEGMENT_SPLIT_RE is replaced by a character scan. _iter_active() yields only the positions where a shell metacharacter is syntactically active — outside single and double quotes, and not backslash-escaped — and _split_segments() separates only at those positions. A backslash does not escape inside single quotes, matching POSIX. An unterminated quote swallows the rest of the line exactly as the shell does, and a shell rejects such a command as a syntax error rather than running its tail, so nothing executable hides behind one.

Logical-separator precedence is preserved: && and || are consumed whole, and ;, |, newline and standalone & keep their prior behaviour. _split_segments("a && b || c") is ['a','b','c'], _split_segments("a & b") is ['a','b'], _split_segments("a; b\nc | d") is ['a','b','c','d'].

True-positive reproductions at the new head

sleep 1 & pkill -f mcp_server.py                       contamination=True  reason_class=manual_daemon_kill
(pkill -f mcp_server.py)                               contamination=True  reason_class=manual_daemon_kill
pkill -f mcp_server.py                                 contamination=True  reason_class=manual_daemon_kill
pkill -f gitea_mcp_server                              contamination=True  reason_class=manual_daemon_kill
killall mcp_server                                     contamination=True  reason_class=manual_daemon_kill
sudo pkill -f mcp_server.py                            contamination=True  reason_class=manual_daemon_kill
true && false || pkill -f mcp_server.py                contamination=True  reason_class=manual_daemon_kill
pkill -f mcp_server.py &                               contamination=True  reason_class=manual_daemon_kill
((pkill -f gitea_mcp_server))                          contamination=True  reason_class=manual_daemon_kill
(ps aux | grep mcp_server) & pkill -f mcp_server.py    contamination=True  reason_class=manual_daemon_kill
kill $(pgrep -f mcp_server.py)                         contamination=True  reason_class=manual_daemon_kill
pkill -f mcp_server.py 2>&1                            contamination=True  reason_class=manual_daemon_kill
pkill -f python                                        contamination=True  reason_class=broad_process_kill

False-positive reproductions, rejected head versus new head

command                                                             6b58f04   aa4fe1cc
git commit -m "block sleep 1 & pkill -f mcp_server.py as recovery"  True      False
echo "docs: sleep 1 & pkill -f mcp_server.py is now detected"       True      False
grep -rn "sleep 1 & pkill -f mcp_server.py" docs/                   True      False
git commit -m 'sleep 1 & pkill -f mcp_server.py stays quoted'       True      False
echo a \& pkill -f mcp_server.py                                    True      False
git commit -m "fix; pkill -f mcp_server.py"                         True      False   (pre-existing class)
git commit -m "fix | pkill -f mcp_server.py"                        True      False   (pre-existing class)

Unchanged at both heads: ps aux | grep mcp_server non-kill; pkill -u mcpuser -f myapp kill, non-contaminating, unambiguous; kill 31337 ambiguous, non-contaminating; kill $(pgrep -f myapp) ambiguous, non-contaminating; npm run dev & sleep 2 and systemctl restart myapp && echo ok non-kills; a 2>&1 non-kill.

Marker behaviour is asserted directly: a new record-tool case drives gitea_record_daemon_process_kill_attempt with each of the three F1 commands and asserts contaminated False, marked False, and that no marker loads afterwards. The two pre-existing record-tool cases for the genuine background-separator and subshell forms still assert contaminated and marked true with a loaded marker.

Test evidence

Focused suite at the new head, cwd /Users/jasonwalker/Development/Gitea-Tools/branches/fix-issue-787-kill-segment-separators, interpreter /opt/homebrew/bin/python3.14:

/opt/homebrew/bin/python3.14 -m pytest tests/test_issue_630_runtime_recovery_guard.py -q -s
64 passed in 0.78s

Focused counts re-measured per F2, each in its own detached worktree, both removed afterwards:

commit cwd focused cases
base 35e94e10 branches/verify-789-base 47
rejected head 6b58f04 branches/verify-789-rejected 55
new head aa4fe1cc branches/fix-issue-787-kill-segment-separators 64

Nine cases are added on top of the rejected head.

Broader suite at the new head:

/opt/homebrew/bin/python3.14 -m pytest -q -s
11 failed, 4190 passed, 6 skipped, 1 warning, 493 subtests passed in 80.38s

The same five modules at base 35e94e10 in branches/verify-789-base:

/opt/homebrew/bin/python3.14 -m pytest -q -s tests/test_commit_payloads.py tests/test_issue_702_review_findings_f1_f6.py tests/test_mcp_server.py tests/test_post_merge_moot_lease.py tests/test_reconciler_supersession_close.py
11 failed, 254 passed in 15.08s

The failing identifiers match test for test at both commits: 6 in tests/test_commit_payloads.py, 2 in tests/test_issue_702_review_findings_f1_f6.py, 1 in tests/test_mcp_server.py::TestPreflightVerification, 1 in tests/test_post_merge_moot_lease.py, 1 in tests/test_reconciler_supersession_close.py. They predate this branch and do not bear on this remediation.

Command-path proof

  • Transport: native MCP stdio daemon, entrypoint mcp_server, pid 52938, mode production, token fingerprint 001cc43cc20d517e.
  • Namespace/profile: gitea-tools / prgs-author, identity jcwalker3 (user_id 1), role author.
  • Server parity: startup_head == current_head == 35e94e107c32cfdc4b9ab9a95cb4e94df94077d4, stale false, mutation gate enforced.
  • Every Gitea mutation went through a native tool after the gitea_whoami -> gitea_resolve_task_capability -> mutate order, with nothing between resolve and mutation: gitea_lock_issue (task lock_issue), gitea_commit_files (task commit_files), gitea_edit_pr (task edit_pr), gitea_create_issue_comment (task comment_issue).
  • Publication used gitea_commit_files with local_path sources; content_source_proof reports local_path for both files. No raw-git push, no direct API call, no CLI fallback.
  • Issue lock: #787 to fix/issue-787-kill-segment-separators, worktree /Users/jasonwalker/Development/Gitea-Tools/branches/fix-issue-787-kill-segment-separators, owner prgs-author, pid 52938, freshness live. It was re-acquired through the sanctioned gitea_lock_issue dead-session recovery path: prior pid 84100 was dead, recovery_mode published_owning_pr, head_relation equal, adoption_decision ADOPT. No lock file was hand-edited, no lease was stolen, no process was killed.
  • Tree proof: remote head aa4fe1cc carries tree 9b12bc5f52b57ff15ac12da2009d1a203a3643ab, identical to the tree of the locally tested commit 9e89efcf, and git diff HEAD FETCH_HEAD is empty. Its parent is the rejected head 6b58f04, so PR #789 was updated in place and no replacement PR exists.

[THREAD STATE LEDGER] Issue #787 / PR #789 — author remediation published at head aa4fe1cc

What is true now:

  • Server-side decision state: PR #789 remains in open state at head aa4fe1cc7bd2d6f07ea35775a5c5292d94a938e0; issue #787 remains in open state; review #497 REQUEST_CHANGES is undismissed but now stale against the live head, has_blocking_change_requests true, approval_at_current_head false, and no merge event has occurred.
  • Local verdict/state: author remediation of F1, F2 and F3 is complete and published; the author has not reviewed this PR, has recorded no verdict on it, and holds no reviewer capability.
  • Latest known validation: focused suite 64 passed at the new head against 47 at base and 55 at the rejected head; the five broader modules give an identical 11-failure signature at base and at the new head.

What changed:

  • Two files changed on branch fix/issue-787-kill-segment-separators: runtime_recovery_guard.py and tests/test_issue_630_runtime_recovery_guard.py.
  • Remote head advanced 6b58f04 -> aa4fe1cc via gitea_commit_files; the PR #789 title and body were edited in place with the corrected F2 counts.
  • The #787 issue lock was recovered from a dead prior session through gitea_lock_issue; no lock, lease, or reviewer artifact was altered by hand. Two temporary verification worktrees were created and removed; the reviewer's review-pr-789 and review-789-baseline worktrees were left untouched.

What is blocked:

  • Blocker classification: no blocker
  • Nothing is outstanding on the author side. PR #789 cannot advance further without a fresh independent review verdict at the new head, which is a reviewer action outside author capability.

Who/what acts next:

  • Next actor: reviewer
  • Required action: perform an independent review of PR #789 at head aa4fe1cc7bd2d6f07ea35775a5c5292d94a938e0, confirming that the three F1 commands classify as non-contaminating, that the required true positives still classify as manual_daemon_kill, that logical-separator precedence is intact, and that the corrected F2 counts match a fresh measurement.
  • Do not do: do not treat stale review #497 as a verdict on this head; do not merge without a fresh verdict at aa4fe1cc; do not remove the author worktree or release the #787 lock while the PR is in open state.

Canonical Issue State

STATE:
author-remediation-published-awaiting-review

WHO_IS_NEXT:
reviewer

NEXT_ACTION:
A fresh independent reviewer examines PR #789 at head aa4fe1cc7bd2d6f07ea35775a5c5292d94a938e0 and records a verdict.

NEXT_PROMPT:

Review PR #789 (issue #787) in Scaled-Tech-Consulting/Gitea-Tools, remote prgs,
at head aa4fe1cc7bd2d6f07ea35775a5c5292d94a938e0. Use only the gitea-reviewer
namespace. The author is jcwalker3; you must not be the same identity.

This head remediates REQUEST_CHANGES review #497, which was recorded against
the prior head 6b58f04d396b881d5d242eacac45bcfdfefe2d00 and is now stale.

Confirm F1 is resolved at the class level, not by special-casing: segment
splitting is quote-aware, so a separator only separates outside single and
double quotes and outside backslash escapes. Check these classify as
non-contaminating:
  git commit -m "block sleep 1 & pkill -f mcp_server.py as recovery"
  echo "docs: sleep 1 & pkill -f mcp_server.py is now detected"
  grep -rn "sleep 1 & pkill -f mcp_server.py" docs/
and that the single-quoted and backslash-escaped variants behave the same.

Confirm the true positives still classify as manual_daemon_kill:
  sleep 1 & pkill -f mcp_server.py
  (pkill -f mcp_server.py)
  pkill -f mcp_server.py, pkill -f gitea_mcp_server, killall mcp_server,
  sudo pkill -f mcp_server.py, true && false || pkill -f mcp_server.py
and that pkill -f python still classifies as broad_process_kill.

Confirm precedence: _split_segments("a && b || c") == ['a','b','c'] and
_split_segments("a & b") == ['a','b'].

Confirm F2: focused suite counts are 47 at base 35e94e10, 55 at 6b58f04, 64 at
this head; the PR body states 8 cases were added by the rejected head.

Confirm F3: _strip_subshell leaves kill $(pgrep -f myapp) intact and a 2>&1 is
not split on the redirection.

Re-run the focused suite and the five broader modules at base and at this head
in detached worktrees, then record a verdict.

WHAT_HAPPENED:
Blocking review #497 found that adding & to a quote-unaware splitter created a false-positive class in which any quoted mention of the canonical kill string classified as a manual daemon kill, and that the AC5 commit-message test omitted the ampersand so it could not catch the regression. The author replaced the regex alternation with a quote-aware character scan that recognises a separator only where it is syntactically active, added nine focused cases covering the three F1 commands plus double-quoted, single-quoted and backslash-escaped ampersands, the POSIX single-quote rule, the pre-existing ; and | class, the two F3 forms, and a record-tool case asserting no marker is written for a quoted mention. The F2 counts were re-measured and the PR body corrected. The remediation was published to PR #789 in place at head aa4fe1cc.

WHY:
A false contamination marker fails review, merge, close and completion mutations closed and by design only a reconciler may clear it, so a false positive is an operator-visible stall rather than a warning. The regressing string is the exact example this issue is written around, which made a documentation line or commit message about #787 a trigger for it. Fixing the quote-unawareness rather than the three named strings also retires the ; and | instances of the same class that predate this issue.

ISSUE:
787

HEAD_SHA:
aa4fe1cc7b

RELATED_PRS:
789

REVIEW_STATUS:
request_changes at the prior head 6b58f04, now stale; no verdict exists at head aa4fe1cc

MERGE_READY:
no

BLOCKERS:
No blocker classification applies on the author side. PR #789 awaits a fresh independent review verdict at head aa4fe1cc, which only a reviewer may record.

VALIDATION:
Focused suite 64 passed at head aa4fe1cc, 55 at rejected head 6b58f04, 47 at base 35e94e10, each measured in its own worktree with /opt/homebrew/bin/python3.14. Broader suite at the new head: 11 failed, 4190 passed, 6 skipped, 493 subtests passed. The five affected modules at base 35e94e10: 11 failed, 254 passed, with identifiers matching test for test, so those failures predate the branch. A 28-command classification probe was run against the new head and the rejected head; all required true positives are preserved and all seven quoted-text false positives are eliminated.

LAST_UPDATED_BY:
prgs-author (jcwalker3), gitea-tools MCP namespace, native stdio transport, pid 52938

Controller Handoff

  • Task: author remediation of REQUEST_CHANGES review #497 on PR #789 for issue runtime_recovery_guard segment splitter misses `&` and subshell forms, so a real daemon kill classifies as clean (#787)
  • Repo: Scaled-Tech-Consulting/Gitea-Tools (remote prgs)
  • Role: author
  • Identity: jcwalker3 / prgs-author
  • Active profile: prgs-author
  • Runtime context: native MCP stdio daemon, entrypoint mcp_server, pid 52938, mode production, server in parity at 35e94e107c (startup_head == current_head, stale false)
  • Selected issue: 787
  • Linked PR: 789
  • Prior head: 6b58f04d39
  • New head: aa4fe1cc7b
  • New tree: 9b12bc5f52, identical to locally tested commit 9e89efcf
  • Parent of new head: 6b58f04d39
  • Target branch: master
  • Target branch SHA: 35e94e107c
  • Blocking review addressed: #497 REQUEST_CHANGES by sysadmin at head 6b58f04, undismissed, now stale against the live head
  • Finding disposition: F1 remediated at the class level; F2 counts corrected to 47 base / 55 rejected head / 64 new head; F3 fixed rather than deferred, with rationale recorded above
  • Author worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/fix-issue-787-kill-segment-separators, on branch fix/issue-787-kill-segment-separators, clean before and after validation
  • Issue lock: live, owner prgs-author, pid 52938, recovered from dead prior pid 84100 through gitea_lock_issue dead-session recovery (recovery_mode published_owning_pr, head_relation equal, adoption_decision ADOPT)
  • Files changed: runtime_recovery_guard.py, tests/test_issue_630_runtime_recovery_guard.py
  • Validation: focused suite 64 / 55 / 47 across the three commits; broader suite 11 failed, 4190 passed at the new head with an identical 11-failure signature at base; 28-command classification probe at both heads. Commands, cwd and interpreter path are quoted under Test evidence.
  • Official validation integrity status: clean — no test weakened or skipped, no assertion loosened, all runs from worktrees under branches/, the project root checkout untouched
  • Gitea mutations: three — gitea_commit_files on branch fix/issue-787-kill-segment-separators, gitea_edit_pr on #789 (title and body), and this gitea_create_issue_comment on #787
  • Review mutations: none
  • Merge mutations: none
  • Cleanup mutations: none
  • Local mutations: one local commit 9e89efcf in the author worktree; the branch ref was moved onto the published head 6b58f04 before editing, discarding only a sibling local commit with an identical tree; two temporary verification worktrees created under branches/ and removed
  • Reviewer artifacts: branches/review-pr-789 and branches/review-789-baseline left untouched
  • External-state mutations: none
  • Read-only diagnostics: mcp_check_workflow_skill_preflight, gitea_whoami, gitea_resolve_task_capability, gitea_assess_master_parity, gitea_view_pr, gitea_get_pr_review_feedback
  • Blockers: no blocker on the author side
  • Current status: author remediation published at head aa4fe1cc; PR #789 in open state; issue #787 in open state
  • Next actor: reviewer
  • Next action: independent review of PR #789 at head aa4fe1cc7b
  • Next prompt: the paste-able text is in the NEXT_PROMPT block above
  • Safe next action: a fresh reviewer, not jcwalker3, records a verdict on PR #789 at head aa4fe1cc
  • Safety statement: no lease or lock was stolen, replaced, or hand-edited; no lock file touched; no process killed; no MCP bypass, raw-git publish, or direct API call; no reviewer worktree removed; no review, approval, merge, or reconciliation performed by this author session

WHO_IS_NEXT: reviewer

# Author remediation of review #497 — PR #789 at new head `aa4fe1cc` Issue #787 · PR #789 · previous head `6b58f04d396b881d5d242eacac45bcfdfefe2d00` · new head `aa4fe1cc7bd2d6f07ea35775a5c5292d94a938e0` ## Review #497 and disposition of each finding Blocking review #497 (REQUEST_CHANGES, reviewer `sysadmin`, recorded against head `6b58f04`) is undismissed but now stale against the live head, and `author_pushed_after_request_changes` reads true. - **F1 — BLOCKING, remediated.** Quote-unaware splitting on `&` made quoted text carrying an ampersand plus a kill verb classify as a manual daemon kill. Fixed at the class level with quote-aware segmentation, not by special-casing the three named strings. New coverage asserts contamination False for each of them, including the full `sleep 1 & pkill -f mcp_server.py` string quoted inside a non-executing command, which is the AC5 case the reviewer found unprotected. - **F2 — MINOR, corrected.** The prior body claimed 46 cases before the change and 9 added. Re-measured in dedicated detached worktrees: base `35e94e10` 47, rejected head `6b58f04` 55, so the rejected head added **8**, not 9. The PR body now carries 47 / 55 / 64 and the corrected arithmetic. - **F3 — INFORMATIONAL, fixed rather than deferred.** Both observations were correct and neither changed a verdict at the rejected head, so neither was load-bearing for this issue. They are corrected here only because F1 required reworking the same splitter, which is the condition under which the reviewer asked for them to be tightened. `_strip_subshell` now removes a trailing `)` only when it closes a leading `(` the same call stripped, so `kill $(pgrep -f myapp)` is no longer mangled; `_is_redirection` keeps `2>&1`, `>&2` and `&>log` out of separator position. No unrelated expansion beyond these two. ## Quote-aware parsing behaviour `_SEGMENT_SPLIT_RE` is replaced by a character scan. `_iter_active()` yields only the positions where a shell metacharacter is syntactically active — outside single and double quotes, and not backslash-escaped — and `_split_segments()` separates only at those positions. A backslash does not escape inside single quotes, matching POSIX. An unterminated quote swallows the rest of the line exactly as the shell does, and a shell rejects such a command as a syntax error rather than running its tail, so nothing executable hides behind one. Logical-separator precedence is preserved: `&&` and `||` are consumed whole, and `;`, `|`, newline and standalone `&` keep their prior behaviour. `_split_segments("a && b || c")` is `['a','b','c']`, `_split_segments("a & b")` is `['a','b']`, `_split_segments("a; b\nc | d")` is `['a','b','c','d']`. ## True-positive reproductions at the new head ``` sleep 1 & pkill -f mcp_server.py contamination=True reason_class=manual_daemon_kill (pkill -f mcp_server.py) contamination=True reason_class=manual_daemon_kill pkill -f mcp_server.py contamination=True reason_class=manual_daemon_kill pkill -f gitea_mcp_server contamination=True reason_class=manual_daemon_kill killall mcp_server contamination=True reason_class=manual_daemon_kill sudo pkill -f mcp_server.py contamination=True reason_class=manual_daemon_kill true && false || pkill -f mcp_server.py contamination=True reason_class=manual_daemon_kill pkill -f mcp_server.py & contamination=True reason_class=manual_daemon_kill ((pkill -f gitea_mcp_server)) contamination=True reason_class=manual_daemon_kill (ps aux | grep mcp_server) & pkill -f mcp_server.py contamination=True reason_class=manual_daemon_kill kill $(pgrep -f mcp_server.py) contamination=True reason_class=manual_daemon_kill pkill -f mcp_server.py 2>&1 contamination=True reason_class=manual_daemon_kill pkill -f python contamination=True reason_class=broad_process_kill ``` ## False-positive reproductions, rejected head versus new head ``` command 6b58f04 aa4fe1cc git commit -m "block sleep 1 & pkill -f mcp_server.py as recovery" True False echo "docs: sleep 1 & pkill -f mcp_server.py is now detected" True False grep -rn "sleep 1 & pkill -f mcp_server.py" docs/ True False git commit -m 'sleep 1 & pkill -f mcp_server.py stays quoted' True False echo a \& pkill -f mcp_server.py True False git commit -m "fix; pkill -f mcp_server.py" True False (pre-existing class) git commit -m "fix | pkill -f mcp_server.py" True False (pre-existing class) ``` Unchanged at both heads: `ps aux | grep mcp_server` non-kill; `pkill -u mcpuser -f myapp` kill, non-contaminating, unambiguous; `kill 31337` ambiguous, non-contaminating; `kill $(pgrep -f myapp)` ambiguous, non-contaminating; `npm run dev & sleep 2` and `systemctl restart myapp && echo ok` non-kills; `a 2>&1` non-kill. Marker behaviour is asserted directly: a new record-tool case drives `gitea_record_daemon_process_kill_attempt` with each of the three F1 commands and asserts `contaminated` False, `marked` False, and that no marker loads afterwards. The two pre-existing record-tool cases for the genuine background-separator and subshell forms still assert `contaminated` and `marked` true with a loaded marker. ## Test evidence Focused suite at the new head, cwd `/Users/jasonwalker/Development/Gitea-Tools/branches/fix-issue-787-kill-segment-separators`, interpreter `/opt/homebrew/bin/python3.14`: ``` /opt/homebrew/bin/python3.14 -m pytest tests/test_issue_630_runtime_recovery_guard.py -q -s 64 passed in 0.78s ``` Focused counts re-measured per F2, each in its own detached worktree, both removed afterwards: | commit | cwd | focused cases | |---|---|---| | base `35e94e10` | `branches/verify-789-base` | 47 | | rejected head `6b58f04` | `branches/verify-789-rejected` | 55 | | new head `aa4fe1cc` | `branches/fix-issue-787-kill-segment-separators` | 64 | Nine cases are added on top of the rejected head. Broader suite at the new head: ``` /opt/homebrew/bin/python3.14 -m pytest -q -s 11 failed, 4190 passed, 6 skipped, 1 warning, 493 subtests passed in 80.38s ``` The same five modules at base `35e94e10` in `branches/verify-789-base`: ``` /opt/homebrew/bin/python3.14 -m pytest -q -s tests/test_commit_payloads.py tests/test_issue_702_review_findings_f1_f6.py tests/test_mcp_server.py tests/test_post_merge_moot_lease.py tests/test_reconciler_supersession_close.py 11 failed, 254 passed in 15.08s ``` The failing identifiers match test for test at both commits: 6 in `tests/test_commit_payloads.py`, 2 in `tests/test_issue_702_review_findings_f1_f6.py`, 1 in `tests/test_mcp_server.py::TestPreflightVerification`, 1 in `tests/test_post_merge_moot_lease.py`, 1 in `tests/test_reconciler_supersession_close.py`. They predate this branch and do not bear on this remediation. ## Command-path proof - Transport: native MCP stdio daemon, entrypoint `mcp_server`, pid 52938, mode production, token fingerprint `001cc43cc20d517e`. - Namespace/profile: `gitea-tools` / `prgs-author`, identity `jcwalker3` (user_id 1), role author. - Server parity: `startup_head` == `current_head` == `35e94e107c32cfdc4b9ab9a95cb4e94df94077d4`, `stale` false, mutation gate enforced. - Every Gitea mutation went through a native tool after the `gitea_whoami` -> `gitea_resolve_task_capability` -> mutate order, with nothing between resolve and mutation: `gitea_lock_issue` (task `lock_issue`), `gitea_commit_files` (task `commit_files`), `gitea_edit_pr` (task `edit_pr`), `gitea_create_issue_comment` (task `comment_issue`). - Publication used `gitea_commit_files` with `local_path` sources; `content_source_proof` reports `local_path` for both files. No raw-git push, no direct API call, no CLI fallback. - Issue lock: #787 to `fix/issue-787-kill-segment-separators`, worktree `/Users/jasonwalker/Development/Gitea-Tools/branches/fix-issue-787-kill-segment-separators`, owner `prgs-author`, pid 52938, freshness live. It was re-acquired through the sanctioned `gitea_lock_issue` dead-session recovery path: prior pid 84100 was dead, `recovery_mode` `published_owning_pr`, `head_relation` `equal`, `adoption_decision` ADOPT. No lock file was hand-edited, no lease was stolen, no process was killed. - Tree proof: remote head `aa4fe1cc` carries tree `9b12bc5f52b57ff15ac12da2009d1a203a3643ab`, identical to the tree of the locally tested commit `9e89efcf`, and `git diff HEAD FETCH_HEAD` is empty. Its parent is the rejected head `6b58f04`, so PR #789 was updated in place and no replacement PR exists. [THREAD STATE LEDGER] Issue #787 / PR #789 — author remediation published at head aa4fe1cc What is true now: - Server-side decision state: PR #789 remains in open state at head `aa4fe1cc7bd2d6f07ea35775a5c5292d94a938e0`; issue #787 remains in open state; review #497 REQUEST_CHANGES is undismissed but now stale against the live head, `has_blocking_change_requests` true, `approval_at_current_head` false, and no merge event has occurred. - Local verdict/state: author remediation of F1, F2 and F3 is complete and published; the author has not reviewed this PR, has recorded no verdict on it, and holds no reviewer capability. - Latest known validation: focused suite 64 passed at the new head against 47 at base and 55 at the rejected head; the five broader modules give an identical 11-failure signature at base and at the new head. What changed: - Two files changed on branch `fix/issue-787-kill-segment-separators`: `runtime_recovery_guard.py` and `tests/test_issue_630_runtime_recovery_guard.py`. - Remote head advanced `6b58f04` -> `aa4fe1cc` via `gitea_commit_files`; the PR #789 title and body were edited in place with the corrected F2 counts. - The #787 issue lock was recovered from a dead prior session through `gitea_lock_issue`; no lock, lease, or reviewer artifact was altered by hand. Two temporary verification worktrees were created and removed; the reviewer's `review-pr-789` and `review-789-baseline` worktrees were left untouched. What is blocked: - Blocker classification: no blocker - Nothing is outstanding on the author side. PR #789 cannot advance further without a fresh independent review verdict at the new head, which is a reviewer action outside author capability. Who/what acts next: - Next actor: reviewer - Required action: perform an independent review of PR #789 at head `aa4fe1cc7bd2d6f07ea35775a5c5292d94a938e0`, confirming that the three F1 commands classify as non-contaminating, that the required true positives still classify as `manual_daemon_kill`, that logical-separator precedence is intact, and that the corrected F2 counts match a fresh measurement. - Do not do: do not treat stale review #497 as a verdict on this head; do not merge without a fresh verdict at `aa4fe1cc`; do not remove the author worktree or release the #787 lock while the PR is in open state. ## Canonical Issue State STATE: author-remediation-published-awaiting-review WHO_IS_NEXT: reviewer NEXT_ACTION: A fresh independent reviewer examines PR #789 at head `aa4fe1cc7bd2d6f07ea35775a5c5292d94a938e0` and records a verdict. NEXT_PROMPT: ```text Review PR #789 (issue #787) in Scaled-Tech-Consulting/Gitea-Tools, remote prgs, at head aa4fe1cc7bd2d6f07ea35775a5c5292d94a938e0. Use only the gitea-reviewer namespace. The author is jcwalker3; you must not be the same identity. This head remediates REQUEST_CHANGES review #497, which was recorded against the prior head 6b58f04d396b881d5d242eacac45bcfdfefe2d00 and is now stale. Confirm F1 is resolved at the class level, not by special-casing: segment splitting is quote-aware, so a separator only separates outside single and double quotes and outside backslash escapes. Check these classify as non-contaminating: git commit -m "block sleep 1 & pkill -f mcp_server.py as recovery" echo "docs: sleep 1 & pkill -f mcp_server.py is now detected" grep -rn "sleep 1 & pkill -f mcp_server.py" docs/ and that the single-quoted and backslash-escaped variants behave the same. Confirm the true positives still classify as manual_daemon_kill: sleep 1 & pkill -f mcp_server.py (pkill -f mcp_server.py) pkill -f mcp_server.py, pkill -f gitea_mcp_server, killall mcp_server, sudo pkill -f mcp_server.py, true && false || pkill -f mcp_server.py and that pkill -f python still classifies as broad_process_kill. Confirm precedence: _split_segments("a && b || c") == ['a','b','c'] and _split_segments("a & b") == ['a','b']. Confirm F2: focused suite counts are 47 at base 35e94e10, 55 at 6b58f04, 64 at this head; the PR body states 8 cases were added by the rejected head. Confirm F3: _strip_subshell leaves kill $(pgrep -f myapp) intact and a 2>&1 is not split on the redirection. Re-run the focused suite and the five broader modules at base and at this head in detached worktrees, then record a verdict. ``` WHAT_HAPPENED: Blocking review #497 found that adding `&` to a quote-unaware splitter created a false-positive class in which any quoted mention of the canonical kill string classified as a manual daemon kill, and that the AC5 commit-message test omitted the ampersand so it could not catch the regression. The author replaced the regex alternation with a quote-aware character scan that recognises a separator only where it is syntactically active, added nine focused cases covering the three F1 commands plus double-quoted, single-quoted and backslash-escaped ampersands, the POSIX single-quote rule, the pre-existing `;` and `|` class, the two F3 forms, and a record-tool case asserting no marker is written for a quoted mention. The F2 counts were re-measured and the PR body corrected. The remediation was published to PR #789 in place at head `aa4fe1cc`. WHY: A false contamination marker fails review, merge, close and completion mutations closed and by design only a reconciler may clear it, so a false positive is an operator-visible stall rather than a warning. The regressing string is the exact example this issue is written around, which made a documentation line or commit message about #787 a trigger for it. Fixing the quote-unawareness rather than the three named strings also retires the `;` and `|` instances of the same class that predate this issue. ISSUE: 787 HEAD_SHA: aa4fe1cc7bd2d6f07ea35775a5c5292d94a938e0 RELATED_PRS: 789 REVIEW_STATUS: request_changes at the prior head `6b58f04`, now stale; no verdict exists at head `aa4fe1cc` MERGE_READY: no BLOCKERS: No blocker classification applies on the author side. PR #789 awaits a fresh independent review verdict at head `aa4fe1cc`, which only a reviewer may record. VALIDATION: Focused suite 64 passed at head `aa4fe1cc`, 55 at rejected head `6b58f04`, 47 at base `35e94e10`, each measured in its own worktree with `/opt/homebrew/bin/python3.14`. Broader suite at the new head: 11 failed, 4190 passed, 6 skipped, 493 subtests passed. The five affected modules at base `35e94e10`: 11 failed, 254 passed, with identifiers matching test for test, so those failures predate the branch. A 28-command classification probe was run against the new head and the rejected head; all required true positives are preserved and all seven quoted-text false positives are eliminated. LAST_UPDATED_BY: prgs-author (jcwalker3), gitea-tools MCP namespace, native stdio transport, pid 52938 ## Controller Handoff - Task: author remediation of REQUEST_CHANGES review #497 on PR #789 for issue #787 - Repo: Scaled-Tech-Consulting/Gitea-Tools (remote prgs) - Role: author - Identity: jcwalker3 / prgs-author - Active profile: prgs-author - Runtime context: native MCP stdio daemon, entrypoint mcp_server, pid 52938, mode production, server in parity at 35e94e107c32cfdc4b9ab9a95cb4e94df94077d4 (startup_head == current_head, stale false) - Selected issue: 787 - Linked PR: 789 - Prior head: 6b58f04d396b881d5d242eacac45bcfdfefe2d00 - New head: aa4fe1cc7bd2d6f07ea35775a5c5292d94a938e0 - New tree: 9b12bc5f52b57ff15ac12da2009d1a203a3643ab, identical to locally tested commit 9e89efcf - Parent of new head: 6b58f04d396b881d5d242eacac45bcfdfefe2d00 - Target branch: master - Target branch SHA: 35e94e107c32cfdc4b9ab9a95cb4e94df94077d4 - Blocking review addressed: #497 REQUEST_CHANGES by sysadmin at head 6b58f04, undismissed, now stale against the live head - Finding disposition: F1 remediated at the class level; F2 counts corrected to 47 base / 55 rejected head / 64 new head; F3 fixed rather than deferred, with rationale recorded above - Author worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/fix-issue-787-kill-segment-separators, on branch fix/issue-787-kill-segment-separators, clean before and after validation - Issue lock: live, owner prgs-author, pid 52938, recovered from dead prior pid 84100 through gitea_lock_issue dead-session recovery (recovery_mode published_owning_pr, head_relation equal, adoption_decision ADOPT) - Files changed: runtime_recovery_guard.py, tests/test_issue_630_runtime_recovery_guard.py - Validation: focused suite 64 / 55 / 47 across the three commits; broader suite 11 failed, 4190 passed at the new head with an identical 11-failure signature at base; 28-command classification probe at both heads. Commands, cwd and interpreter path are quoted under Test evidence. - Official validation integrity status: clean — no test weakened or skipped, no assertion loosened, all runs from worktrees under branches/, the project root checkout untouched - Gitea mutations: three — gitea_commit_files on branch fix/issue-787-kill-segment-separators, gitea_edit_pr on #789 (title and body), and this gitea_create_issue_comment on #787 - Review mutations: none - Merge mutations: none - Cleanup mutations: none - Local mutations: one local commit 9e89efcf in the author worktree; the branch ref was moved onto the published head 6b58f04 before editing, discarding only a sibling local commit with an identical tree; two temporary verification worktrees created under branches/ and removed - Reviewer artifacts: branches/review-pr-789 and branches/review-789-baseline left untouched - External-state mutations: none - Read-only diagnostics: mcp_check_workflow_skill_preflight, gitea_whoami, gitea_resolve_task_capability, gitea_assess_master_parity, gitea_view_pr, gitea_get_pr_review_feedback - Blockers: no blocker on the author side - Current status: author remediation published at head aa4fe1cc; PR #789 in open state; issue #787 in open state - Next actor: reviewer - Next action: independent review of PR #789 at head aa4fe1cc7bd2d6f07ea35775a5c5292d94a938e0 - Next prompt: the paste-able text is in the NEXT_PROMPT block above - Safe next action: a fresh reviewer, not jcwalker3, records a verdict on PR #789 at head aa4fe1cc - Safety statement: no lease or lock was stolen, replaced, or hand-edited; no lock file touched; no process killed; no MCP bypass, raw-git publish, or direct API call; no reviewer worktree removed; no review, approval, merge, or reconciliation performed by this author session WHO_IS_NEXT: reviewer
sysadmin removed the status:pr-open label 2026-07-21 21:40:04 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
allocator
Controller-owned work allocator
 anti-stomp
Prevent concurrent LLM session stomping
 architecture
Architecture / structural design
 bug
chore
codex
OpenAI Codex client / workflow session surface
 concurrency
Concurrent session safety
 contamination
Workflow or session contamination incident
 control-plane
MCP control-plane coordination and allocation authority
 dashboard
MCP operational dashboard/queue view
 database
Internal coordination storage (SQLite/Postgres)
 design
Design / investigation, no implementation
 documentation
Docs / runbooks
 enhancement
New feature or improvement
 gitea
Gitea MCP workflow
 glitchtip
GlitchTip integration
 important
incident
Operational or process incident requiring durable audit trail
 incident-bridge
Sentry-to-Gitea incident bridging
 integration
Integration testing
 jenkins
Jenkins integration
 labels
Label taxonomy management
 leases
Lease adopt/release/expire lifecycle
 mcp
MCP server / tooling
 mcp-health
MCP namespace and runtime health
 mcp-menu
MCP menu surface
 multi-project
Work spanning multiple monitoring projects or Gitea repos
 mutating
Mutating action; requires gating
 nice-to-have
observability
Observability, metrics, traces, error reporting
 portability
Cross-platform / portability
 preflight
Shared preflight gates before mutation
 protected-branch
Protected branch / stable-branch policy concern
 queue
Work queue visibility and allocation
 read-only
Read-only, no mutation
 reconnect
MCP client reconnect/reload recovery path
 recovery
Recovery paths for stale/foreign leases
 refactor
Code refactor / restructure
 release
Release / versioning
 reliability
Reliability / failure handling
 resumable-review
Persist and resume prepared review verdicts across sessions
 reviewer
Reviewer workflow tooling
 roadmap
Roadmap / umbrella issue
 safety
Safety rails and fail-closed mutation guards
 security
Security / trust boundary
 self-hosted
Self-hosted infrastructure integration
 sentry
Sentry error monitoring integration
 stale-runtime
Stale backend daemon / runtime-vs-master parity failures
 status:blocked
Issue is blocked
 status:in-progress
Issue is being worked on
 status:pr-open
Issue has an open pull request
 status:ready
Issue is ready for work
 terminal-lock
Terminal review lock (#332) path
 testing
Tests / test coverage
 tracker
Issue tracker hygiene / meta
 type:bug
Bug or defect
 type:feature
Feature or enhancement
 type:feature
Feature or enhancement
 type:guardrail
Safety gate or guardrail
 visibility
Workflow state visibility for LLMs/operators
 workflow
Cross-tool workflow
 workflow-hardening
LLM workflow coordination hardening
 workflow-hardening
LLM workflow coordination hardening
No labels bug mcp-health stale-runtime type:guardrail workflow-hardening
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jcwalker3 sysadmin
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Scaled-Tech-Consulting/Gitea-Tools#787