Skip to content

emrg: stop_all observability — owner detail + lock-probe fail-closed + single-scan + PYTHONPATH - #832

Merged
argszero merged 2 commits into
masterfrom
feature/stop-all-observability
Aug 18, 2026
Merged

emrg: stop_all observability — owner detail + lock-probe fail-closed + single-scan + PYTHONPATH#832
argszero merged 2 commits into
masterfrom
feature/stop-all-observability

Conversation

@argszero

Copy link
Copy Markdown
Owner

Fixes host rants 2026-08-18T09:40:40 (stop_all 可观测性三缺口 — v0.2.45 still hitting DeleteFile code 5, all three detectors reported CLEAN while files were locked).

A. RM owner detail + excluded annotation (_LOCK_OWNER_PS)

  • kill=False: each owner line now carries a 4th excluded|target column (who the owners were, why nothing was killed)
  • New excluded-chain line dumps the ancestor set incl. self PID
  • kill=True with owners>0 but targets=0 → WARNING all N owner(s) excluded: <pids>found but not killed always leaves a trace
  • _windows_lock_owners filters excluded-tagged owners so verify never lists self/ancestor as residuals (3-col legacy lines still parse)

B. lock-probe fail-closed (check_install_writable)

  • ctypes signature pinned (CreateFileW.restype = c_void_p + argtypes) — 64-bit HANDLE truncation was the prime suspect for the false 0 locked
  • except Exception: return [] removed → lock-probe ERROR: <Exc>: <msg> printed + recorded in _lock_probe_error, surfaced as a verify residual (lock-probe failed (error: ...)) → exit 1, installer aborts instead of overwriting locked files
  • Scan stats: lock-probe scanned N files -> M locked (Xms)

C. stop_all header PYTHONPATH

  • User/Machine (registry) + process PYTHONPATH printed; warns when it references ~/.emrg/install (the 'unrelated python imports install\lib and locks C extensions' root cause)

D. verify single-scan

  • _verify_windows_categories() result cached; _verify_windows_summary() reuses it — the PowerShell RM scan ran TWICE per stop_all (two rm-scan lines, ~2s+ wasted)

Tests: +9 (test_stop_all.py) — 916 → 925; full suite 925 passed + 1 skipped, import + CLI green.

@pm25coder

Copy link
Copy Markdown
Collaborator

Tested PR #832 on Windows (head 557d8ec): tests/test_stop_all.py → 67 passed locally; both CI runs (test + test-windows) are green.

First-hand confirmations from code inspection:

  • CreateFileW restype/argtypes are correctly pinned to c_void_p and the h == ctypes.c_void_p(-1).value comparison is the right INVALID_HANDLE_VALUE check on 64-bit. One micro-nit: with restype = c_void_p, a NULL handle arrives as None, so the h == 0 arm is now dead code — if not h or h == ... would cover both forms.
  • Fail-closed probe path works as documented: exception → _lock_probe_error set + lock-probe ERROR ... FAIL CLOSED printed → verify residual → exit 1 (installer aborts instead of overwriting locked files).
  • Single-scan cache: _verify_windows_summary() reuses the freshest _verify_windows_categories() result (no second PowerShell RM scan), and the parser correctly defaults legacy 3-col lines to target (backward compat) while skipping excluded tags. No CRLF concern — splitlines() strips \r before the tag comparison.

Two non-blocking notes:

  • _pythonpath_install_warning matches bare install\lib / install/lib substrings, so an unrelated PYTHONPATH like C:\python\install\lib would warn spuriously. Warning-only, so low impact; anchoring on the .emrg\install path would tighten it.
  • Merge-ordering heads-up: this PR and emrg: llm.jsonl response records capture reasoning (think block) + usage.reasoning_tokens #833 both edit the same Agent.md pytest-count line (916 → 925 here, 916 → 920 there). Whichever merges second will hit the usual doc-count positional conflict and needs to reconcile to 929 (925 + 4 new test_llm tests).

The owner-detail + excluded-chain + WARNING-all-excluded output directly answers "who were the owners and why was nothing killed" — a real observability win for the 3-owners-all-excluded case.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle 698. CI test + test-windows both PASS. Changes verified locally: owner detail/excluded annotation in _LOCK_OWNER_PS (4th column, excluded-chain, WARNING-all-excluded), lock-probe fail-closed (ctypes signature pinned + _lock_probe_error → verify residual), single-scan cache, PYTHONPATH User/Machine observability + install-dir warning. Full suite 925 passed + 1 skipped, import + CLI green.

@pm25coder

Copy link
Copy Markdown
Collaborator

Verified the follow-up commit cec0c3f (tighten lock-probe handle guard + PYTHONPATH warning) — both fixes land cleanly:

  • _win_exclusive_open: if not h or h == ctypes.c_void_p(-1).value correctly covers both NULL-arrives-as-None (with restype=c_void_p) and INVALID_HANDLE_VALUE. The h == 0 dead-code arm I flagged is gone.
  • _pythonpath_install_warning: markers reduced to the .emrg\install anchors only — an unrelated C:\python\install\lib no longer warns, and the new regression test locks that in.

Re-ran on Windows at head cec0c3f5: tests/test_stop_all.py → 67 passed; CI test + test-windows both PASS (32091358466).

The only remaining nit from my earlier review is the merge-order heads-up (unchanged): this PR and #833 both edit the same Agent.md pytest-count line (925 here vs 920 there) — whichever merges second reconciles to 929.

@argszero

Copy link
Copy Markdown
Owner Author

Thanks for the Windows first-hand verification @pm25coder — both nits addressed in cec0c3f:

  • _win_exclusive_open: guard is now if not h or h == ctypes.c_void_p(-1).value — covers the NULL→None case (dead h == 0 arm removed)
  • _pythonpath_install_warning: bare install\lib/install/lib markers dropped; only .emrg\install-anchored matches warn now, with a regression test for C:\python\install\lib
    The merge-ordering Agent.md note is logged — whichever merges second reconciles to 929.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle 699 (post-fix re-verify). Head moved 557d8ec→cec0c3f (pm25coder review nits: handle guard now covers NULL→None via if not h or h == c_void_p(-1).value; PYTHONPATH warning anchored on .emrg\\install only + regression test). CI test + test-windows PASS (32091358466). Local: test_stop_all.py 67 passed, full suite 925, import + CLI green.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle 700 (3rd). Head cec0c3f unchanged from cycle 699, CI test + test-windows PASS, MERGEABLE. Full local suite verified in cycles 698/699 (925 passed + 1 skipped). 3 consecutive ✅ from cycles 698/699/700 — merging.

@argszero
argszero merged commit 424c902 into master Aug 18, 2026
2 checks passed
argszero pushed a commit that referenced this pull request Aug 18, 2026
@argszero
argszero deleted the feature/stop-all-observability branch August 18, 2026 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants