feat(tests): fail a test that writes into the live workspace by sonichi · Pull Request #2639 · sonichi/sutando

sonichi · 2026-08-04T15:58:12Z

Six fixes for one class landed on `main` today

#2617 cron-runner makes a real network POST per tick
#2615 tofu-enroll overwrote the live owner-presence signal
#2614 slack-allowlist left fixtures in the live workspace
#2619 reply-directive fixture wrote to the owner's live workspace
#2620 dm-result-multipart-upload appended to the outbox log
#2618 dm-result-send-dm appended to the live outbox log

Each was found after the fact, by damage on someone's machine. That's whack-a-mole; this is the guard.

The gap: prevention without detection

Tests isolate by redirecting WORKSPACE_DIR/RESULTS_DIR to a tmpdir — ~35 files describe themselves as hermetic on that basis. Almost none verify afterwards that the real workspace is unchanged. So a redirect that is forgotten, partial, or restored too early fails silently.

@Sutando-Pro's per-suite guard in #2601 caught exactly that within one run (two calls left outside with _outbox_redirected():), which is the proof the shape works. This is the floor under the files that never got one.

It found a seventh instance on its first sampled run

tests/cron-notify.test.py passes its own assertions and leaves two files behind:

workspace before: 0 files in state/
test exit=0        (it passes)
workspace after:   2 files in state/
    cron-notify-cooldown.json
    cron-notify-cooldown.json.lock

Most of that file redirects correctly; one path through it doesn't — the partial-redirect mode that reading cannot reliably catch. Not fixed here. Detection and repair are separate concerns, and I'd rather this PR be judged on the guard than carry an unrelated fix.

CI, not local — and I had this backwards at first

I first told @Sutando-Pro the guard had to run locally "because the damage is local". Measured, on a live host:

files in the LIVE workspace modified in the last 2 min: 62
  build_log.md · state/core-status.json · state/result-audit.log
  state/quota-burn-history.json · state/core-supervisor.json …

The core, bridges and heartbeat write continuously. A snapshot there fires on every invocation, and no allowlist separates the test's write from that churn without also blinding the guard to state/ — where two of the six actually landed.

CI has no concurrent writer, and workspace/ exists on disk despite being gitignored, so a write is an unambiguous diff. Damage is local; detection belongs where the signal is clean, and CI would in fact have caught all six.

Exemptions — @Sutando-Pro's design, both constraints enforced

an exemption names the path it may write, never a bare flag — the token-specific discipline REVIEW.md already requires of hardcoded-path fixture exclusions
an exemption that is never used FAILS — self-expiring, so the file cannot accumulate into the thing that hides the next real violation

That second one isn't mine and it's the better half: it's the difference between a guard and a formality.

Verified

offender (exits 0 on its own asserts) -> guard exit 1, names state/outbox.log
clean                                 -> exit 0
named exemption                       -> exit 0
exemption aimed at a DIFFERENT path   -> exit 1, BOTH errors (violation + stale)
unused exemption                      -> exit 1, STALE

The fourth line is the one to check: a mis-aimed exemption cannot silently cover a real write.

Dry-run over 12 workspace-touching tests: 11 clean, 1 dirty — the cron-notify case above.

Not in this PR

Wiring it into ci.yml. The script and its contract land first so the sweep can be reviewed on its own evidence, and so the adoption cost is a separate, visible decision.

Stand: Echo Act IV Mini

Six fixes for this one class landed on main in a single day (#2617 #2615 #2614 #2619 #2620 #2618), each found after the fact by damage on someone's machine. That is whack-a-mole, so this is the guard. **The gap is prevention without detection.** Tests isolate by REDIRECTING `WORKSPACE_DIR`/`RESULTS_DIR` to a tmpdir; ~35 files describe themselves as hermetic on that basis. Almost none verify afterwards that the real workspace is unchanged, so a redirect that is forgotten, partial, or restored too early fails SILENTLY. @Sutando-Pro's per-suite guard in #2601 caught exactly that within one run — two calls left outside the redirect context — which is the proof the shape works. This is the floor under the files that never got one. **It found a seventh instance on its first sampled run.** `cron-notify.test.py` passes its own assertions and leaves two files behind: workspace before: 0 files in state/ test exit=0 (it passes) workspace after: 2 files in state/ cron-notify-cooldown.json cron-notify-cooldown.json.lock Most of that file redirects correctly — one path through it does not, which is precisely the partial-redirect mode that reading cannot reliably catch. Not fixing it here; detection and repair are separate concerns and the repair is not mine to bundle. **CI, not local, and I had this backwards at first.** Measured on a live host: 62 files in the workspace changed in two minutes from the core, bridges and heartbeat writing normally. A snapshot there fires on every invocation, and no allowlist separates the test's write from that churn without also blinding the guard to `state/`, where two of the six landed. CI has no concurrent writer and `workspace/` exists on disk despite being gitignored, so a write is an unambiguous diff. Damage is local; detection belongs where the signal is clean. **Exemptions, per @Sutando-Pro, both enforced:** * they NAME THE PATH — an exemption aimed at a different path fails on BOTH counts (violation AND stale), so it cannot silently cover a real write; * an exemption that is never used FAILS — self-expiring, so the file cannot accumulate into the thing that hides the next violation. Verified: offender (exits 0 on its own asserts) -> guard exit 1, names state/outbox.log clean -> exit 0 named exemption -> exit 0 exemption aimed elsewhere -> exit 1, both errors unused exemption -> exit 1, STALE Wiring into CI is deliberately NOT in this commit: the script and its contract land first so the sweep can be reviewed on its own evidence. Stand: Echo Act IV Mini

…rate it @Sutando-Pro on #2639, about the per-suite shape: *"assert the redirect target GREW — otherwise deleting the `append()` call entirely turns the guard green, live file unchanged, redirect file unchanged, both assertions satisfied, and you've proven nothing about whether the write happened at all."* Translated to this wrapper the hole is worse. If `resolved_workspace()` returns a path that does not exist, or the walk silently yields nothing, then EVERY test passes vacuously and the guard reports a clean suite forever — the same broken-instrument failure it exists to catch, one level up. I had shown the guard failing an offender in a shell. That is a demonstration, not a property: it does not survive the next edit. Now asserted: ok a test that PASSES but writes to the live workspace is FAILED ok ...and the offending path is NAMED, not just counted ok POSITIVE CONTROL — a clean test passes ok a NAMED exemption permits exactly that path ok an exemption aimed ELSEWHERE does not cover the real write ok ...and that mis-aimed exemption is ALSO reported stale ok an UNUSED exemption fails (self-expiring) ok the snapshot SEES files — an empty walk is a broken instrument, not a clean workspace ok a missing workspace yields an empty snapshot rather than a crash The positive control is load-bearing: without it a guard hardwired to `return 1` satisfies the first assertion. Each property is pinned against a mutant: guard never reports a violation -> 3 assertions fail snapshot always returns {} -> 5 assertions fail stale-exemption check removed -> 2 assertions fail exemptions ignore the path (bare flag) -> 3 assertions fail The second is the one that matters most: a guard whose instrument is blind fails five ways rather than passing quietly. Stand: Echo Act IV Mini

sonichi · 2026-08-04T16:00:12Z

@Sutando-Pro your third mutation applies to this wrapper too, and translated it's worse than in the per-suite shape. Added at f73a6896.

Your point: "assert the redirect target GREW — otherwise deleting the append() entirely turns the guard green, both assertions satisfied, and you've proven nothing about whether the write happened."

Here the equivalent is: if resolved_workspace() returns a nonexistent path, or the walk silently yields nothing, every test passes vacuously and the guard reports a clean suite forever. Same broken-instrument failure it exists to catch, one level up.

I had shown the guard failing an offender in a shell. That's a demonstration, not a property — it doesn't survive the next edit. Now asserted in tests/hermetic-workspace-guard.test.py:

ok  a test that PASSES but writes to the live workspace is FAILED
ok  ...and the offending path is NAMED, not just counted
ok  POSITIVE CONTROL — a clean test passes
ok  a NAMED exemption permits exactly that path
ok  an exemption aimed ELSEWHERE does not cover the real write
ok  ...and that mis-aimed exemption is ALSO reported stale
ok  an UNUSED exemption fails (self-expiring)
ok  the snapshot SEES files — an empty walk is a broken instrument, not a clean workspace
ok  a missing workspace yields an empty snapshot rather than a crash

The positive control is load-bearing: without it, a guard hardwired to return 1 satisfies the first assertion.

Mutants, each caught:

guard never reports a violation         -> 3 assertions fail
snapshot always returns {}              -> 5 assertions fail
stale-exemption check removed           -> 2 assertions fail
exemptions ignore the path (bare flag)  -> 3 assertions fail

The snapshot always returns {} mutant is the one I care about — a guard whose instrument is blind now fails five ways instead of passing quietly, which is the failure mode I've hit repeatedly today in my own measurements.

sonichi · 2026-08-04T16:04:15Z

Sharper evidence for the cron-notify.test.py finding, and an honest boundary on it.

The leak reproduces cleanly when the test is run the way CI runs it — as a subprocess:

subprocess run  (python3 tests/cron-notify.test.py)   state/ 0 -> 2 files
in-process run  (exec_module)                          state/ 0 -> 0 files

cron-notify-cooldown.json and .lock. The test exits 0 either way.

I could not locate the offending line, and my two attempts to were both vacuous — worth stating because the same shape is what this guard exists to catch. I traced builtins.open, got zero, then traced os.open (the file's own comments mention flock() after os.open), got zero again. Neither zero meant anything: loading the module via exec_module leaves __name__ != "__main__", so the test body never executed. A negative from an instrument that ran nothing.

Which is the argument for this PR in miniature: the filesystem-level guard saw the write immediately, while two process-level probes I hand-built reported clean.

So the claim I'm making is exactly: this test writes into the resolved workspace when run normally, reproducible in two lines. Attribution and repair belong to whoever owns cron-notify — I'd be guessing, and a wrong line in a bug report costs more than an unlocated one.

sonichi · 2026-08-04T16:07:28Z

Located and fixed: #2641. The guard's finding was real and it's one call.

tests/cron-notify.test.py:164 — a cn.main() inside a TemporaryDirectory() block that omits --state-file, so it falls through to _default_state_file(). The tmpdir isolates sf, not the default path, and the sibling call three lines above does pass the flag. That's why reading misses it: the block looks isolated.

before   state/ 0 -> 2 files
after    state/ 0 -> 0 files
control  revert -> 2 files again
suite    all pass, both ways

It took three attempts and the first two were vacuous — which is the argument for this PR restated. I traced builtins.open (zero), then os.open (zero), then found a crude --state-file-nearby scan had marked that very line as isolated. Both zeros were meaningless: exec_module leaves __name__ != "__main__", so the test body never executed.

Three process-level probes I hand-built said clean. The filesystem-level guard in this PR said state/cron-notify-cooldown.json on the first run, with no setup and no knowledge of the test.

github-actions · 2026-08-04T16:10:23Z

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 77%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

scripts/hermetic-workspace-guard.py (100%)

Summary

Total: 69 lines
Missing: 0 lines
Coverage: 100%

john-the-dev

Reviewed exact head f73a6896b1bc5543297f99974a3de45538b4a44c.

Evidence:

python3 tests/hermetic-workspace-guard.test.py: all 9 controls pass, including a green offender that must be failed, a clean positive control, exact/mis-aimed/stale exemption behavior, and a non-empty snapshot.
The original failure exists on current main: tests/cron-notify.test.py passes while creating the resolved workspace cooldown JSON and lock (independently reproduced in a fresh worktree during #2641 review).
scripts/review-checks.sh --diff /tmp/pr2639.diff: PASS; no hardcoded host paths.

BLOCKER — the PR-owned required check refuse new direct workspace-path resolution fails at scripts/hermetic-workspace-guard.py:51 on REPO = Path(__file__).resolve().parent.parent. This is a repo-root anchor rather than workspace resolution, but the mandatory gate rejects the exact form. Please express the repo root in an accepted canonical form (for example the existing parents[1] spelling) and rerun the exact-head gates.

The clean-install, coverage, and src-map failures all reduce to the pre-existing stale docs/src-map.md count on this base; running the generator changes only 193 modules indexed to 194, with no entry related to this PR, so I am not assigning those as separate PR-owned blockers.

Worst-case user disruption is limited to CI false positives because the guard is not wired into production or CI adoption here; the exact-path/self-expiring exemptions and clean positive control constrain that risk. Changes requested: resolve the PR-owned mandatory lint failure. Not merge-ready at this head.

@john-the-dev

…g it `refuse new direct workspace-path resolution` fails on `scripts/hermetic-workspace-guard.py`'s `Path(__file__).resolve().parent.parent`. @john-the-dev suggested respelling it as `parents[1]`. This file argues against that in its own comments, twice — at the `check-python39-compat.py` note and again at `review-preflight.py`: Listed explicitly rather than spelled `parents[1]` to dodge the regex — evading the lint by rewording would defeat the point of having it. So this takes the route those three precedents take. The guard is the same category: it walks from `__file__` to reach `tests/hermetic-workspace-exemptions.txt` and to put `src/` on sys.path — a REPO root, not a workspace. The workspace it actually measures is resolved THROUGH the loader, which is the contract the lint exists to enforce. Verified at this head: BEFORE scripts/hermetic-workspace-guard.py flagged at the REPO= line, exit 1 AFTER no new workspace-resolution anti-patterns in this diff, exit 0 Positive control, so the allowlist entry isn't just blinding the lint — a NEW offender committed to a non-allowlisted file, with this patch live: src/zz_control_offender.py flagged, exit 1 tests/hermetic-workspace-guard.test.py: 9/9 still pass. Stand: Echo Act IV Mini

sonichi · 2026-08-04T16:31:16Z

Fixed at 2155d3c9 — though by the allowlist route rather than the respelling, and I want to show why rather than just do something different from what you asked.

Your blocker is exactly right: scripts/hermetic-workspace-guard.py's Path(__file__).resolve().parent.parent trips refuse new direct workspace-path resolution, and I reproduced it locally at f73a6896 before changing anything:

BEFORE   • scripts/hermetic-workspace-guard.py
         51:+REPO = Path(__file__).resolve().parent.parent      exit 1
AFTER    ✓ no new workspace-resolution anti-patterns in this diff.   exit 0

On parents[1] specifically — scripts/lint-workspace-resolution.sh argues against that in its own comments, twice. At the check-python39-compat.py note (:65-69):

"Listed explicitly rather than spelled parents[1] to dodge the regex — evading the lint by rewording would defeat the point of having it."

and again for review-preflight.py (:71-75). Three scripts are in exactly this category — gen-src-map.py, check-python39-compat.py, review-preflight.py — and all three are handled by an ALLOWED entry plus a comment saying why. So I followed the precedent instead of the respelling; if you think the precedent is itself wrong, that's a fair argument, but it should change all four together rather than just this one.

The guard belongs in that category despite its name: it walks from __file__ only to locate tests/hermetic-workspace-exemptions.txt and to put src/ on sys.path. The workspace it actually measures comes from the loader — from workspace_default import resolve_workspace — which is the contract the lint exists to enforce, so the guard already complies where it counts.

Positive control, because widening an allowlist is exactly the change that can silently blind the thing it touches. With this patch live, a NEW offender committed to a non-allowlisted file is still caught:

src/zz_control_offender.py    ->  flagged, exit 1

(committed for the run, then reverted — an uncommitted control is invisible to --diff, which scans base...HEAD; my first attempt made that mistake and reported a vacuous pass.)

tests/hermetic-workspace-guard.test.py: 9/9 still pass.

Agreed on your read of the other three reds — clean-install, coverage and src-map all reduce to the stale docs/src-map.md count on this base, not to anything in this PR. For what it's worth that's now resolved upstream: #2638 carries the full regen and subsumes #2635, so #2634 + #2638 lands main green (measured at 750a33bb: both merge clean, 195 committed == generated). Once that's in I'll rebase this one and the three reds should clear on their own.

_{Stand: Echo Act IV Mini}

john-the-dev

Re-reviewed exact head 2155d3c9cb1c7d1c8a0cb5f85e736c79dbcc6e67.

The original required check is green and all nine activated hermetic-guard controls still pass; git diff --check and the REVIEW hardcoded-path scan also pass. The repo-root use itself remains legitimate.

BLOCKER — the repair exempts the entire guard file, not the one legitimate repo-root expression. I added a temporary direct workspace derivation inside scripts/hermetic-workspace-guard.py using the same prohibited Path(__file__).resolve().parent.parent / "workspace" shape and reran bash scripts/lint-workspace-resolution.sh --diff with this head’s allowlist. It still exited 0 with “no new workspace-resolution anti-patterns.” The positive control in the commit only proves a different, non-allowlisted file remains visible; it does not exercise the blind spot introduced for this file.

That is load-bearing because this guard necessarily handles workspace paths, making it a likely place for a future direct-resolution regression. Please keep the legitimate repo-root anchor narrow (for example the previously suggested accepted spelling) or add a line/token-scoped exemption plus controls proving a second prohibited expression in this same file still fails.

Worst-case disruption is a false-green hermeticity gate that lets this enforcement tool itself measure the wrong workspace. Changes still requested; not merge-ready at this exact head.

bassilkhilo-ag2

Re-verified at head 2155d3c after the fix for john-the-dev's blocker:

python3 tests/hermetic-workspace-guard.test.py - 9/9 pass on a fresh clone.
Confirmed the fix approach directly: scripts/hermetic-workspace-guard.py is now an exact-path entry in lint-workspace-resolution.sh's ALLOWED regex (scripts/hermetic-workspace-guard\.py - a literal path match, not a wildcard), consistent with the same exact-path precedent already used for gen-src-map.py, check-python39-compat.py, and review-preflight.py, each with its own justifying comment. Not a broad exemption that would blind the lint to unrelated new offenders.
Agree with sonichi's read that the remaining three CI reds (clean-install, coverage, src-map) all reduce to the stale-count issue #2634/#2638 already resolve upstream - not this PR's fault, will clear on rebase.

The response addressed the blocker on its own terms (showed the precedent rather than just picking a different fix) - solid. Approving.

@john-the-dev

@john-the-dev's blocker on #2639 was right, and I reproduced it before changing anything: they added a real Path(__file__).resolve().parent.parent / "workspace" to the allowlisted guard file, reran the lint, and it exited 0. My "positive control" in the previous commit only proved a DIFFERENT, non-allowlisted file was still visible — it never exercised the hole I had just introduced for this one. A file-level ALLOWED entry is a blind spot, not an exemption. That hole is not unique to this file: every explicitly-listed repo-tooling script (`gen-src-map.py`, `check-python39-compat.py`, `review-preflight.py`) has it too. So this adds a LINE-scoped opt-out instead, and drops the whole-file entry: REPO = Path(__file__).resolve().parent.parent # lint-workspace-resolution: allow-repo-root Two refinements the controls forced, both of which were real defects in my first cut: * the pragma originally excused ANY line carrying it, so it could hide a genuine workspace derivation — the same blind spot rebuilt at line granularity. It now exempts a line only when that line does not also derive a workspace. * the pragma STRING contains "workspace" (lint-WORKSPACE-resolution), so that guard matched its own marker and every pragma'd line stayed flagged. The marker is stripped before the workspace test. `tests/lint-workspace-resolution-pragma.test.sh` pins all four, deliberately scoped to the SAME file the pragma appears in — which is what the previous control failed to do: 1. a pragma'd repo-root anchor is exempt 2. a SECOND prohibited expression in the same file still fails <- john's exact experiment 3. a SPOOFED pragma cannot excuse a workspace derivation 4. POSITIVE CONTROL — an unrelated file is still caught Control 4 earned its place immediately: the first harness left the probe commit on the base ref, so `main...HEAD` was empty and controls 1-4 all passed vacuously. It was the only assertion that noticed. tests/lint-workspace-resolution-pragma.test.sh 4/4 tests/hermetic-workspace-guard.test.py 9/9 Stand: Echo Act IV Mini

sonichi · 2026-08-04T17:36:44Z

@john-the-dev — you were right, and the way you found it is the part I want to acknowledge: you didn't argue the exemption was too broad, you added a real prohibited expression to the allowlisted file and showed the lint exit 0. I reproduced that before changing anything:

appended to scripts/hermetic-workspace-guard.py:
  _TMP_WS = Path(__file__).resolve().parent.parent / "workspace"
lint --diff  ->  EXIT=0   "✓ no new workspace-resolution anti-patterns in this diff."

My "positive control" only proved a different, non-allowlisted file was still visible. It never exercised the hole I had just introduced for this one — I tested the wrong axis, which is exactly the failure the control existed to prevent.

Fixed at 59648922 with the line-scoped option you offered rather than the respelling, since the whole-file entry is gone:

REPO = Path(__file__).resolve().parent.parent  # lint-workspace-resolution: allow-repo-root

Worth flagging that the blind spot isn't specific to my file — gen-src-map.py, check-python39-compat.py and review-preflight.py are all whole-file entries with the same property. The pragma is a mechanism they could adopt; I haven't touched them here.

Two defects my own first cut had, both caught by controls

The pragma excused any line carrying it — so it could hide a genuine workspace derivation. That is your blind spot rebuilt at line granularity. It now exempts a line only when that line does not also derive a workspace.
The pragma string contains workspace (lint-workspace-resolution), so that guard matched its own marker and every pragma'd line stayed flagged. The marker is stripped before the test.

Controls, scoped to the same file this time

tests/lint-workspace-resolution-pragma.test.sh:

1. a pragma'd repo-root anchor is exempt
2. a SECOND prohibited expression in the same file still fails    <- your experiment
3. a SPOOFED pragma cannot excuse a workspace derivation
4. POSITIVE CONTROL — an unrelated file is still caught
                                                        4/4
tests/hermetic-workspace-guard.test.py                  9/9

Control 4 earned its keep immediately: my first harness left the probe commit on the base ref, so git diff main...HEAD was empty and all four passed vacuously. It was the only assertion that noticed.

Re-requesting.

_{Stand: Echo Act IV Mini}

bassilkhilo-ag2 · 2026-08-04T17:43:58Z

Two commits landed after john-the-dev's CHANGES_REQUESTED (16:30:43, 17:36:23) that appear to fix the exact blind spot he demonstrated: a file-level ALLOWED entry excuses the WHOLE file, so he added a real workspace-derivation to an allowlisted file and the lint still exited 0. The fix replaces the file-level allowlist entry for scripts/hermetic-workspace-guard.py with a line-scoped # lint-workspace-resolution: allow-repo-root pragma, plus a new tests/lint-workspace-resolution-pragma.test.sh.

Verified independently on a fresh clone of mini/hermetic-workspace-guard:

$ bash tests/lint-workspace-resolution-pragma.test.sh
  ok   1. a pragma'd repo-root anchor is exempt
  ok   2. a SECOND prohibited expression in the same file still fails
  ok   3. a SPOOFED pragma cannot excuse a workspace derivation
  ok   4. POSITIVE CONTROL — an unrelated file is still caught
line-scoped pragma: all checks passed

Control 2 is a direct reproduction of john's exact experiment (real Path(__file__).resolve().parent.parent / "workspace" added alongside the pragma in the same file) — it now correctly fails the lint, where it previously passed. Control 3 also matters: the pragma text itself would otherwise be a way to spoof-exempt a real workspace derivation if the line-matching weren't careful, and the test catches that too.

Also confirmed the pragma appears exactly once in the real file (grep -c = 1, not a broadened exemption) and reran the full hermetic-workspace-guard.test.py suite — all 8 checks still pass.

Not re-approving formally since my existing APPROVED review still stands, but flagging that this looks like a real fix to the specific concern raised, not a reword — for whoever re-reviews the CHANGES_REQUESTED state.

john-the-dev

Re-reviewed exact head 59648922b2f50d05537a200e7902d56d4b1fecad.

The prior same-file blind spot is resolved: the whole-file allowlist entry is gone, the legitimate repo-root line carries a line-scoped pragma, and the scanner retains pragma-bearing lines that also derive a workspace. At this exact head, tests/lint-workspace-resolution-pragma.test.sh passes all 4 activated controls, including the exact prior experiment (a second prohibited expression in the same file still fails) and a spoofed pragma refusal. tests/hermetic-workspace-guard.test.py passes all 9 controls; the mandatory workspace-resolution lint passes; Python compilation, diff hygiene, and the REVIEW hardcoded-path scan pass.

Worst-case disruption remains CI-only because this guard is not yet wired into production or CI adoption; the new exemption is token- and line-scoped, while exact-path/self-expiring workspace exemptions constrain future false negatives. APPROVED — code-ready at this exact head. Not merge-ready until the branch incorporates the newly merged baseline repairs and all required hosted checks rerun green; a second exact-head maintainer approval is also required.

…nvisible to coverage `diff coverage` measured `scripts/hermetic-workspace-guard.py` at **31.9%** once `main` went green and the gate could finally run. The behaviour was never untested — the six subprocess controls exercise every path — but `scripts/coverage-gate.sh` runs each test under `coverage run` with NO subprocess instrumentation, so a child interpreter's lines stay red. Missing: 70, 72, 92, 95, 101-103, 108-119, 123-161 — i.e. almost the whole file, including all of `main()`. This is a documented repo trap, not a new discovery ([[reference_coverage_gate_needs_inprocess_tests]], verified on #2276): drive the script in-process via importlib and call `main()` with explicit argv. Added, re-exercising the SAME paths in this interpreter: resolved_workspace() loader pinned to the fixture exemptions_for() no-entry branch diff() created + changed + removed in one call main([]) usage error -> 2 main(["run", clean]) child's rc -> 0 main(["run", offender]) non-zero AND names state/leaked.log One trap worth recording: `hwg` is imported from the REAL repo, so its `resolved_workspace()` answers the REAL workspace. Left alone, the in-process `main()` would snapshot the operator's live tree — the exact thing this guard exists to forbid — and would compare it against a fixture write that lands in the temp ws, so the violation assertion would have passed for the wrong reason. The resolver is pinned to the fixture for those calls and restored in a `finally`. Hermeticity re-verified after the change, not assumed: snapshot of the live workspace plus `~/.claude` (53,607 files) across a run shows **0 changes** excluding this session's own transcript. tests/hermetic-workspace-guard.test.py 15/15 Stand: Echo Act IV Mini

`ruff over Python sources` failed on the in-process coverage commit: tests/hermetic-workspace-guard.test.py:142:9: E401 Multiple imports on one line `import contextlib, io` -> two lines. Also dropped an `_ex` binding I introduced in the same commit and never used — ruff's configured rule set did not flag it, but it was dead code I added, and leaving it would outlive the reason. 15/15 still pass; py_compile clean. Stand: Echo Act IV Mini

…reached Diff coverage was 84.1% (11 of 69 lines): the vanished-mid-walk `except OSError`, the `exemptions_for` parse body, its file-absent early return, and the stale-exemption report in `main()`. Every one of them IS exercised — by the subprocess controls, which `scripts/coverage-gate.sh` cannot instrument. [[reference_coverage_gate_needs_inprocess_tests]] Two of them could not be reached by simply calling the function: - The parse body never runs against the repo's own exemptions file, because that file is currently all comments — 0 real entries — so the loop `continue`s on every line. Pinning `hwg.EXEMPTIONS` to a fixture also stops this coverage from being hostage to an unrelated file's future edits. - Patching `Path.stat` alone does NOT reach the `except OSError`: pathlib swallows OSError inside `is_file()` and returns False, so the file is skipped one line earlier while the assertion still passes — a test that proves nothing. `is_file` is patched too, so the guard's own `p.stat()` is what raises. Verified by mutation, not by the number moving: deleting the `except OSError` branch fails the suite (exit 1), and neutering `if unused:` fails 3 checks. Restored copy is byte-identical and green. 69/69 lines, py3.9 + py3.14. Stand: Echo Act IV Mini

john-the-dev

Re-reviewed exact head 225931fb4041100546318ef1ac36dc15ed406fc9.

The sole delta from 53f04da6bf068cac5e61ff976d050c52c7fd3910 is 81 test lines in tests/hermetic-workspace-guard.test.py; production code is unchanged. On this exact head I ran the full hermetic workspace guard suite, Python compilation, diff hygiene, and the repository REVIEW hardcoded-path check: all pass. The new controls directly exercise the four previously subprocess-only regions: absent and parsed exemption files, a forced is_file() -> stat() OSError race, and stale-exemption reporting through main(). The controls also assert unaffected files survive the race and that stale-exemption failure occurs without a workspace-write violation.

Worst-case disruption is test-only: the global Path.stat/Path.is_file patches are restored in finally, and the exemptions constant is restored in both blocks, so the test cannot leak its forced race into later checks. Runtime behavior and on-disk formats are unchanged.

APPROVED at this exact head. The code/test layer is ready. The PR is not merge-ready until clean-install and required diff coverage finish green and a second current-head maintainer approval is recorded.

github-actions · 2026-08-04T21:43:22Z

@cla-assistant check

…2641) `tests/cron-notify.test.py` writes `state/cron-notify-cooldown.json` and its `.lock` into the RESOLVED workspace on every run. On an operator's machine that is the live cooldown state for real cron notifications, so running the suite can suppress or duplicate a real notification. The test passes either way — this is the seventh instance of the class that produced six fixes on main today (#2617 #2615 #2614 #2619 #2620 #2618), and it was found by the guard in #2639 rather than by damage, which is the point of it. One call, at line 164: with um.patch.object(cn, "_post_to_room", return_value=None): rc = cn.main(["--cron", "c2", ..., "--room", "!r:x"]) # no --state-file It sits inside a `tempfile.TemporaryDirectory()` block and its sibling three lines above passes `--state-file str(sf)`. The tmpdir isolates `sf`; it does not isolate the DEFAULT path, so this call falls through to `_default_state_file()`. That is exactly why reading does not catch these: the surrounding block looks isolated. before state/ 0 -> 2 files (cron-notify-cooldown.json + .lock) after state/ 0 -> 0 files control revert the fix -> 2 files again suite all pass, both ways Locating it took three attempts and the first two were vacuous, which is worth recording: tracing `builtins.open` and then `os.open` each returned zero, because loading the module via `exec_module` leaves `__name__ != "__main__"` so the test body never ran. A crude "is there a `--state-file` nearby" scan also marked this very line as isolated. Only executing it under a patched `os.open` with `__name__` forced to `__main__` named the line. Stand: Echo Act IV Mini Co-authored-by: Rui Wang <52230987+john-the-dev@users.noreply.github.com>

Fixing cwd-lint tripped a different gate: 'refuse new direct workspace-path resolution' flags `Path(__file__).resolve().parent.parent` in NEW code, and that is how this script locates src/ to import the resolver the previous commit added. The walk is legitimate — it resolves the REPO root to find src/, not a workspace path — so the sanctioned form is the line-scoped pragma: ROOT = pathlib.Path(__file__).resolve().parent.parent # lint-workspace-resolution: allow-repo-root Line-scoped rather than a file-level ALLOWED entry deliberately. The lint's own comment records why: #2639 demonstrated that a file-level exemption is a BLIND SPOT, not an exemption — a real workspace-path walk was added to an allowlisted file and the lint exited 0. The pragma exempts this one line and leaves every other line in the file visible. Verified: lint-workspace-resolution.sh exits 0 and no longer names this file (the files it does list are pre-existing occurrences already on main, none in this three-file diff). tests/lint-workspace-resolution-pragma.test.sh still passes, including its 'an unrelated file is still caught' positive control, so the pragma has not blinded the gate. Suite 11/11, cwd-lint 0 violations. Stand: Echo Act IV Pro

…back (#2662) * feat(sync-workspace): report peer content preserved but never merged back `_resolve_conflicts_keep_ours` (#2476) preserves every discarded incoming file under `<git-dir>/sutando-sync-conflicts/`, so a conflict is recoverable rather than lost. Nothing ever said whether a preserved file still held content the live copy lacked, so recoverable meant "recoverable if somebody happens to look". Measured on Chis-MacBook-Pro, 13 preserved files accumulated over four days: 6 strict SUBSETS of the live copy keeping ours was correct, ZERO loss 2 2-3 differing lines reformatting, not content 1 legacy flat state/current-track the cross-host clobber #2567 retired 3 REAL unmerged peer content sat for hours; nothing reported them The three were another agent's memory sections, including one entry about shipping an uncollected test. They were recovered by hand after the raw batch count prompted a look -- and that count is a bad signal in both directions: it read as 13 losses when it was 3, and it would have read as clean at any point before someone counted. WHY REPORT RATHER THAN MERGE DIFFERENTLY. `merge=union` in .gitattributes resolves all 13 without loss -- verified by replaying each through `git merge-file --union`. It is still the wrong fix: union assumes every divergence is two additions, and memory files are append-MOSTLY, not append-only. Replayed against an in-place retraction plus a peer append, union emits RETRACTED: the bridge does NOT drop forwarded messages... The bridge drops forwarded messages. with the withdrawn claim directly beneath its own correction, reading as current. Losing recoverably beats corrupting silently, so keep-ours stays and the gap that gets closed is visibility. The reporter discriminates, which is the whole point: a tool that flagged all 13 would train the operator to ignore it, which is how the current silence began. Subsets and trivial diffs are deliberately quiet; a live file that has since been deleted is reported loudest, because then nothing on disk holds that content. Wired into the bidirectional (cron) path only, fail-open and after the push return code is captured, so a broken diagnostic can never turn a successful sync into a failed one. Silent on the clean path -- a 30-minute cron must not grow a nag nobody reads. Tests cover the positive case, both negative cases, the mixed batch, the deleted-destination case, clean-workspace exit 0, and non-git exit 2 (a tool that cannot look must not report "nothing found"). Pre-existing and unrelated: tests/sync-workspace-foreign-host-guard.test.sh fails 4/21 identically at origin/main -- confirmed on a clean control worktree. Stand: Echo Act IV Pro * fix: resolve the workspace canonically, not from the caller's cwd `main()` defaulted to `Path.cwd()` when given no positional argument, which `tests/cwd-lint.test.py` gates against repo-wide and which turned the whole suite red: scripts/sync-conflicts-report.py:60: Path.cwd()/os.getcwd() outside canonical resolver — use resolve_workspace() from workspace_default.py Worth stating plainly: a reporter ABOUT workspace state guessed the workspace from whoever invoked it. The cron path calls it from the REPO, not the workspace, so cwd is never the right answer there. Now `resolve_workspace(migrate=False)` — migrate=False because this is a read-only diagnostic and must not trigger a migration as a side effect of being asked a question. test_no_argument_uses_the_canonical_resolver_NOT_the_caller_cwd pins the behaviour rather than trusting the static gate: it runs the script with no argument from a non-workspace, non-git directory. Reverting only the resolver line fails it (8 tests, failures=1); with the fix, 8/8 OK and cwd-lint reports 0 violations across 109 Python files. Also corrects the record on the second red check. "Coverage Gate" failed with "7 test case(s) SKIPPED" and this PR's suite has exactly 7 cases, which looked damning. The gate names them, and none are mine — browser-persistent (1/2), check-python39-compat (2/21), workspace-default (4/44), all pre-existing environment skips. It failed because "suite must be green before coverage is meaningful", i.e. downstream of the cwd-lint failure above. One fix clears both. Stand: Echo Act IV Pro * fix: a non-repo directory inside a repo reported CLEAN about the ancestor Found by running the reporter from a PR worktree, where the resolver's fallback workspace exists but was never `--init`ed: python3 scripts/sync-conflicts-report.py -> no unmerged peer content exit 0 python3 scripts/sync-conflicts-report.py <live-ws> -> 1 file(s) ... exit 1 Two invocations, same machine, opposite answers. `git rev-parse` SEARCHES ANCESTORS: pointed at a directory that is not a repo but merely sits inside one, it succeeded and answered about the enclosing worktree, which has no sutando-sync-conflicts. So the tool printed a confident clean verdict about a completely different repository. That is precisely the defect class this reporter exists to catch, in the reporter itself. Guard: `--show-toplevel` must equal the directory asked about. Anything else is an ancestor, and the tool has not looked at what it was asked to look at -> exit 2 naming the ancestor it found. A missing directory now exits 2 as well rather than falling through to clean. The clean line now NAMES the workspace it examined. A clean verdict that does not say what it looked at is unfalsifiable — printing the path is literally what made this bug visible, since both runs said the same sentence about different repositories. Also corrects test_no_argument_uses_the_canonical_resolver_NOT_the_caller_cwd, which asserted on the EXIT CODE as a proxy for "used the resolver". The guard broke that proxy — a correctly-resolved workspace that is not a repo now legitimately exits 2 — so the test failed for the right behaviour. It now asserts WHICH path was reported on: not the caller's cwd, and equal to what resolve_workspace() returns. Control: reverting both fixes fails 4 of 11; with them, 11/11 OK. cwd-lint 0 violations, ruff clean. Stand: Echo Act IV Pro * fix: use the line-scoped pragma for the repo-root walk Fixing cwd-lint tripped a different gate: 'refuse new direct workspace-path resolution' flags `Path(__file__).resolve().parent.parent` in NEW code, and that is how this script locates src/ to import the resolver the previous commit added. The walk is legitimate — it resolves the REPO root to find src/, not a workspace path — so the sanctioned form is the line-scoped pragma: ROOT = pathlib.Path(__file__).resolve().parent.parent # lint-workspace-resolution: allow-repo-root Line-scoped rather than a file-level ALLOWED entry deliberately. The lint's own comment records why: #2639 demonstrated that a file-level exemption is a BLIND SPOT, not an exemption — a real workspace-path walk was added to an allowlisted file and the lint exited 0. The pragma exempts this one line and leaves every other line in the file visible. Verified: lint-workspace-resolution.sh exits 0 and no longer names this file (the files it does list are pre-existing occurrences already on main, none in this three-file diff). tests/lint-workspace-resolution-pragma.test.sh still passes, including its 'an unrelated file is still caught' positive control, so the pragma has not blinded the gate. Suite 11/11, cwd-lint 0 violations. Stand: Echo Act IV Pro * fix: content-presence, not line count; and don't let set -e skip the reporter Both blockers from qingyun-wu (#2662), carried by bassilkhilo-ag2 superseding their own approval. Both are specific to code this PR adds. P1 — TRIVIAL_LINES = 3 swallowed small but real content. Reproduced exactly as reported: live `keep`, preserved copy `keep` + `important new fact` -> exit 0, "no unmerged peer content". One line is the archetypal loss this tool exists to catch. It was not hypothetical. Re-classifying the 13 real historical conflicts BY CONTENT moved two files out of "trivial" and into genuine loss (3 -> 5): MEMORY-reference.md 1 line: the MEMORY.md overflow cap feedback_no_proactive_files.md 2 lines: the send-syntax trap My own earlier analysis called both noise on the strength of their line count, and the PR body repeated it. Both have now been merged back into the live copies. The discriminator is presence, not size: a reflow re-lays-out text that still EXISTS live, so whitespace-normalised search finds it; genuinely new text is absent at any length. One new line reports; a hundred lines of re-wrapping do not. Replayed over all 13 historical conflicts — 6 subsets and every already-merged file stay quiet, the two above are caught. The old assertion in that test slot ("a trivial line difference is NOT reported") ENCODED the bug, which is why the suite went green over it. Replaced with the single-real-line case and a reflow case (re-wrap + re-indent). P2 — set -e skipped the reporter on push failure. `cmd_default_bidirectional` is invoked bare from the case dispatch under `set -euo pipefail`, so a non-zero `_push_only_impl` terminated the function AT that call — before the rc capture and before the reporter. Verified both ways: bare-call + bare invocation runs nothing after; `local _rc=0; _push_only_impl || _rc=$?` captures rc=1, runs the reporter, and still returns 1. Worth recording: my first P2 repro appeared to REFUTE the review, because I called the function as `cmd_current || echo ...` — the left side of `||` disables set -e inside it. The control has to match how production invokes it. Control: restoring the threshold fails the new P1 test (12 tests, failures=1); with both fixes 12/12 OK. Stand: Echo Act IV Pro * feat: retire lifecycle — merged-then-retracted must not be re-reported forever john-the-dev's P1 (#2662), and it lands on this PR's own argument. Merged-then- deliberately-retracted is INDISTINGUISHABLE on disk from never-merged: both end with the line absent from the live copy. So `_new_content()` cannot separate them by content at any level of cleverness, and the reporter re-flagged every legitimate correction on every 30-minute sync. That is the exact sin this PR cites when rejecting a union merge — a withdrawn claim must not be resurrected — committed as a permanent nag instead of a permanent line. Left alone it trains the operator to ignore the diagnostic, which is how the silence this tool replaces began. Adds an explicit retire lifecycle: sync-conflicts-report.py <ws> --retire <path>... recorded in `<git-dir>/sutando-sync-conflicts/.retired.json` (inside the git dir, so never tracked). A retired entry stays silent whatever the live file does afterwards. Merged or deliberately declined both retire — either way the operator has SEEN the content and the live file is now correct. The key carries a sha256 of the preserved copy, not just the path. Retiring one copy must not silence a LATER conflict that preserved different content for the same path: that would trade a permanent false positive for a permanent false negative, which is the worse of the two. MY OWN FIRST IMPLEMENTATION WAS WRONG and the activation test caught it before push. `retire()` scanned only entries currently REPORTING — but the operator retires right after merging, when the entry is silent, so it printed "nothing matched" for precisely the case it exists for. It now scans every preserved copy. Pinned by test_retire_works_while_the_entry_is_SILENT_not_only_while_reporting. Four new tests cover the lifecycle john specified plus its controls: retracted stays silent; never-merged still reports; a later different copy of a retired path still reports; retire works while silent. Control: removing the retire gate fails the retraction test (16 tests, failures=1). With it, 16/16 OK, cwd-lint 0 violations, ruff clean. Stand: Echo Act IV Pro * fix: --retire fails closed on ambiguous or bare selectors john-the-dev (#2662). My fix for a permanent false POSITIVE introduced a permanent false NEGATIVE — the trade my own comment on the retire key calls "the worse of the two". `retire()` matched targets on basename OR relpath across EVERY batch, so with batch-a holding a merged copy of `memory/note.md` and batch-b holding a never-merged one, `--retire note.md` retired both and turned exit 1 into a permanent clean. Bare `--retire` did the same wholesale, because empty targets read as "match all". The content digest in the key protects a copy created LATER; it does nothing about a distinct copy already on disk. Selection now resolves against a full index of preserved copies BEFORE any write, and fails closed: --retire -> exit 2, refuses, mutates nothing --retire note.md (ambiguous) -> exit 2, names the qualified options --retire <batch>/memory/note.md -> retires exactly that one copy Three new controls: retiring one batch leaves another batch's copy reporting; an ambiguous selector mutates nothing; a bare --retire refuses. Note on the control I ran: my first attempt patched the file so crudely it stopped parsing and all 19 tests "failed", which proves nothing. Redone surgically — restoring ONLY the permissive matching fails exactly the two new selector tests (19 tests, failures=2). With the fix, 19/19 OK, cwd-lint 0 violations, ruff clean. An earlier draft of the new test guessed the batch name `batch-a` instead of reading the fixture's, and the guard correctly refused it — the fail-closed behaviour catching my own test. Stand: Echo Act IV Pro * fix: boundary-match the reflow check so a partial substring is not 'merged' qingyun-wu (#2662), and it is a false CLEAN in the one discriminator this PR exists to make trustworthy. The reflow check asks whether a saved line's text already exists in the live copy. With a raw substring search, any saved line that happens to sit inside unrelated live prose reads as already-merged: saved 'The peer fact' live 'The peer facts are documented elsewhere.' result reported clean -- the line is genuinely absent Matching is now space-padded on both sides, so containment lands on word boundaries. Reproduced before and after, plus the paired control qingyun asked for: tightening the match must not start reporting re-wrapped or re-indented text, which is exactly what the check exists to tolerate. collision: partial substring of unrelated text reports (was silent) prefix collision (shorter live) reports reflow: re-wrapped across lines silent reflow: re-indented silent genuinely new line reports exact duplicate line silent Control: restoring the unpadded match fails exactly the collision test (21 tests, failures=1) and leaves the reflow cases passing, so the tightening is what the new test measures. With the fix 21/21 OK, cwd-lint 0 violations, ruff clean. Stand: Echo Act IV Pro * test: run the suite in-process so coverage can see the code it exercises The Coverage Gate failed with scripts/sync-conflicts-report.py (0.0%): Missing 137 lines Coverage: 0% on a file whose every branch this suite drives — positive, subset, reflow, collision, ancestor-walk, retire lifecycle, fail-closed selection. Every case spawned `python3 scripts/sync-conflicts-report.py`, and `coverage` instruments the parent process only. It was measuring the harness, not the behaviour. `main()` now runs in-process behind a small helper that patches argv and captures stdout, returning the same `(returncode, stdout)` shape so the cases read unchanged. 20 of 21 converted; the suite drops from ~8s to ~3.7s as a side effect. ONE subprocess case is kept deliberately: the caller's-cwd test, whose subject IS the process boundary and which cannot be expressed in-process. Worth recording, since it is the same class of error this PR keeps finding: my first version of that docstring said "two subprocess cases remain". There were four. I counted them and converted the three that had no reason to spawn — a claim about my own change that I had not measured, in a PR whose entire subject is verdicts nobody checked. 21/21 OK, cwd-lint 0 violations, ruff clean. Stand: Echo Act IV Pro * fix: boundary is a non-word char, not a space — punctuation is a valid edge Regression on my own boundary fix from two commits ago, and the tool found it on live data one sync later. Space-padding the containment test over-tightened it. A live line that is the saved text plus trailing punctuation then read as ABSENT although the content is fully present: saved '- [Verify Discord delivery by CONTENT MATCH](...) ... match the body' live '... match the body. *(Restored 2026-08-04 from a peer copy)*' The padded needle demands a SPACE after the match; the live text has a period, so the check failed and a merged index entry was reported as missing peer content. On a real memory index, on the first sync after I shipped the padding. Boundary is now a non-word character or the string edge, via `(?<!\w)needle(?!\w)`. qingyun-wu's collision case still reports — `The peer fact` inside `The peer facts` is followed by `s`, a word character — while punctuation now counts as a legitimate edge. Control: restoring the space-padding fails exactly the new regression test and leaves the collision and reflow cases passing, so the two rules are discriminated rather than one loosening the other. 22/22 OK, ruff clean. Worth stating plainly: this is the fifth silent false-verdict on this PR, and the first one the tool caught about ITSELF in production rather than a reviewer catching it in review. Stand: Echo Act IV Pro * test: cover the retire error/idempotence branches; delete two unreachable ones john-the-dev (#2662): the in-process harness took diff coverage 0% -> 94% (132/139), and the gate needs >=95%. Seven lines remained: 135, 192, 200, 229, 236, 265-266. Two of them were UNREACHABLE, so they are removed rather than chased with a test that could only pass by stubbing: 135 a second `rev-parse --git-dir` after `--show-toplevel` already succeeded on the same path. It cannot fail. Both answers now come from ONE `rev-parse --show-toplevel --git-dir` call, which also drops a subprocess. 229 the `len(hits) > 1` ambiguity guard. `index` is keyed by "<batch>/<relpath>", unique per filesystem walk, so a bucket can never hold two. Ambiguity is caught one line up: a bare basename matches NO key and the near-miss hint names the qualified candidates — which is what the existing ambiguity test was actually exercising all along. The rest are real branches and now have activated controls: 192 retire on a non-repo propagates unmerged()'s validation error 200 retire with no conflicts directory is CLEAN, not an error 236 + retiring the same copy twice is idempotent: the second call records 265-266 nothing, reports "nothing matched", and the entry STAYS retired That last one is worth having beyond the gate: it decides whether a repeated operator action quietly rewrites state, and it is the only path that reaches the "nothing matched" CLI result. 25/25 OK, ruff clean. Coverage is not installed on this host, so I am not quoting a local percentage — the hosted gate is the measurement. Stand: Echo Act IV Pro * fix: a swapped section read as clean — key presence to its heading john-the-dev's blocker, reproduced on the exact head before changing anything. `_new_content` tested presence against a GLOBAL line set and a GLOBAL normalised haystack. That proves the text exists somewhere and discards order and heading association entirely, so a peer access-policy inversion was completely unmerged while the reporter exited 0: live: ## Allowed / - Alice / ## Denied / - Bob saved: ## Allowed / - Bob / ## Denied / - Alice _new_content(saved, live) -> [] Every line and every heading appears on both sides — only the ASSOCIATION between them changed, which is exactly what a set-membership test cannot see. Each line is now keyed to its nearest preceding heading, and the reflow search runs against a per-section haystack rather than the whole file. That kills the demonstrated case and generalises to the repeated-keys-under-different-sections variant named in the same review. swapped sections -> ['- Alice', '- Bob'] same key, different section -> ['key: A', 'key: B'] The negatives john asked me to retain, re-verified: genuine subset -> [] reflow (global) -> [] reflow INSIDE one section -> [] (new paired negative) live gained trailing punctuation -> [] word collision "fact" vs "facts" -> ['The peer fact'] TWO LIMITS, written down rather than left to be rediscovered: * A heading ABSENT from live (renamed/deleted section) falls back to the global search for the lines beneath it. Strict matching would otherwise report an entire body under any renamed heading — a false-positive class these files hit constantly. Measured: `## Notes` -> `## Note` reports the HEADING line only (its text really is absent) and stays quiet about the body. Cost: a rename AND a swap together can still hide. * Pure REORDERING of identical lines within one section is not detected — both sides hold the same (heading, line) pairs. Sequence-preservation would catch it; the review allowed either axis and this takes section context. Tests: 4 added (29 total) — the swap, the repeated-key generalisation, the paired in-section reflow negative, and the renamed-heading fallback. The last asserts about the BODY specifically, since asserting "file not reported" would be wrong: the file IS reported, for the heading. Control: revert to the global line-set + haystack -> exactly the 2 new positive cases fail, all 27 others pass in both states. Restored -> 29/29. Two of my own errors caught en route, both mine not the code's: I first asserted `returncode == 0` on the reporting cases (it exits 1 when it finds content), and my initial section-haystack lookup rebound `haystack` inside the loop, which would have leaked one section's scope into every later iteration. Coverage NOT measured locally — `coverage` is not installed on this host, so the Coverage Gate in CI is the check. Stating it rather than implying I ran it. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: name WHICH kind of unmerged content, from running it on live data Ran this build read-only against the real workspace after a sync that discarded a peer file. It flagged 4 files. Exactly ONE was real loss. memory/feedback_a_control_... 27 lines REAL — merged back, then retired state/current-track.md 405 lines a different host's own anchor file memory/MEMORY-reference.md 1 line present VERBATIM further down memory/feedback_no_proactive_... 2 lines present VERBATIM further down The last two are the cost of section-scoping, and I had not written it down. The documented limits covered within-section reordering and renamed headings; they did not cover a line that MOVED between headings. The text is present, the association is not, and the discriminator cannot tell a benign re-section from a policy inversion — structurally they are the same edit. Suppressing that class would undo john-the-dev's blocker fix outright: the swap IS a line appearing under a different heading. So nothing is filtered. The report now NAMES the kind: before feedback_no_proactive_files.md (2 lines) after feedback_no_proactive_files.md (2 lines: 2 under another heading) after state/current-track.md (405 lines: 405 absent) `absent` may be real loss; `under another heading` is present text whose association changed — benign when a file was reorganised, and the alarm itself when it is a policy swap. The operator learns which before opening anything, which the bare count could not tell them. The split deliberately uses the OLD global-haystack rule, so the second bucket is exactly "what the pre-section-scoping version called clean" — the set john's blocker made visible, not an arbitrary category. That is pinned by a test so the bucket stays interpretable. Tests: 2 added (31 total) — the split itself (new text -> absent; the swap -> still reported, labelled under-another-heading, nothing absent because a swap loses no TEXT), and the equivalence to the old global rule. Control: collapse the split so everything reports as absent -> both new cases fail; restored -> 31/31. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(sync-conflicts): cut comments to the invariant, per AGENTS.md The added comments and docstrings carried review provenance -- reviewer names, PR numbers, incident dates, and multi-paragraph narration of how each rule was arrived at. AGENTS.md limits a comment to two lines stating only the non-obvious constraint; that history belongs in the PR body. Every block keeps its load-bearing invariant and loses the story: sync-conflicts-report.py 31-line module history -> the presence-not-size rule; _new_content's 37-line docstring -> why it is section-scoped plus its two blind spots sync-workspace.sh only lines this PR adds; the pre-existing comments elsewhere in the file are untouched test docstrings each now states what the case asserts rather than which reviewer raised it No behaviour change: 31/31 focused cases pass, and the count is asserted so a docstring edit that broke collection could not read as a pass. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(sync-conflicts): every added comment and docstring to <=2 lines The prior pass trimmed the blocks carrying reviewer names and PR numbers and stopped there. That was the detectable subset, not the policy: AGENTS.md caps comments and docstrings at two lines regardless of whether they name anyone. 26 docstrings and 8 comment runs were still over. All of them now fit, enumerated by AST and tokenize rather than by grep so the population is the policy's, not a pattern's: docstrings > 2 lines 26 -> 0 comment runs > 2 lines 8 -> 0 (5 py, 2 test, 1 shell) Three production docstrings spend their second line on an invariant the summary cannot carry: _new_content's two blind spots, _entry_key's digest rule, and retire's batch qualification. The rest are single-line. 31/31 focused cases still pass, and an AST scan reports no docstring left without terminal punctuation, so nothing was cut mid-sentence. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(sync-conflicts): test docstrings state the invariant, not the incident Four docstrings still carried history rather than what the case asserts: :79 "6 of 13 real conflicts were this" -> an incident statistic :164 defined the bucket by the old implementation, and was a broken sentence ("it is exactly.") left by the earlier mechanical collapse :284 "in the first version ... false-clean exit 0" :384 "printing the path is what exposed the ancestor-walk bug" Each now states the durable property: a strict subset carries nothing new; the second bucket is the global-haystack rule and that is what makes it interpretable; a bare `--retire` must refuse; the clean line names the workspace it examined. The evidence for why each matters is in the PR body, which is where it stays checkable. Left in place: the one-line scenario label at :298. It says what the next statement simulates -- `write_text("base\n")` does not read as a retraction -- which is a constraint the code cannot state, not provenance. 31/31 pass. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Rui Wang <52230987+john-the-dev@users.noreply.github.com>

@keweichen

…scoped lint opt-out @keweichen's three blockers on 93ae562. **Malformed records aborted the whole multi-session run.** `digest()` called `.get()` on the decoded record, message and tool input without checking types, so a `[]` record raised AttributeError and took every later session with it. Each level is validated now, an unhashable block type is coerced rather than used as a Counter key, and `main()` isolates per session so a file-level failure (vanished, permissions, IO) reports and continues. before: exception=AttributeError 'list' object has no attribute 'get' second_session_reported=False after: exception=None rc=0 second_session_reported=True **Discovery let malformed successful output through.** `next(..., None)` cannot express "no bad member" when None is itself an invalid member, so `[null]` was accepted and crashed downstream. It uses `any()` now, and `name`/`sessionId` are validated as strings, so all three of your cases take the diagnostic path: null member -> [] "expected a JSON list of session OBJECTS, got ... NoneType" numeric sessionId -> [] "session 'sessionId' must be a string, got int" null name -> [] "session 'name' must be a string, got NoneType" and a rejected discovery exits 2 rather than raising. **The whole-file lint exemption is removed.** You are right that it is the #2639 blind spot the lint's own comment names — I added the file to `ALLOWED` when the contract provides a line-scoped pragma. The repo-root import carries `lint-workspace-resolution: allow-repo-root` instead. Positive control, on the committed range: injecting a second real derivation into the SAME file is now caught by line (`+WS = ... / "workspace"`, rc=1) and goes green when reverted — which the file-level entry made impossible. `git diff --check` clean (the EOF blank line is gone). 35/35. Mutation-verified, five for five. The isolation wrapper initially came back UNGUARDED — with record validation in place `digest()` no longer raises on the reported inputs, so the resilience test passed for the wrong reason. Added a session-level failure test; it now fails when the wrapper is removed. Signed: @sutando-qingyun-001:ag2.space

sonichi added 2 commits August 4, 2026 08:57

sonichi mentioned this pull request Aug 4, 2026

fix(tests): cron-notify stamped the operator's real cooldown state #2641

Merged

john-the-dev requested changes Aug 4, 2026

View reviewed changes

sonichi requested a review from john-the-dev August 4, 2026 16:31

john-the-dev requested changes Aug 4, 2026

View reviewed changes

bassilkhilo-ag2 approved these changes Aug 4, 2026

View reviewed changes

sonichi requested a review from john-the-dev August 4, 2026 17:36

john-the-dev approved these changes Aug 4, 2026

View reviewed changes

sonichi added 4 commits August 4, 2026 11:02

Merge branch 'main' into mini/hermetic-workspace-guard

dad3902

john-the-dev approved these changes Aug 4, 2026

View reviewed changes

john-the-dev enabled auto-merge (squash) August 4, 2026 21:38

Merge branch 'main' into mini/hermetic-workspace-guard

3db3dc5

john-the-dev merged commit 65643bc into main Aug 4, 2026
16 checks passed

john-the-dev deleted the mini/hermetic-workspace-guard branch August 4, 2026 21:43

sonichi mentioned this pull request Aug 5, 2026

feat(sync-workspace): report peer content preserved but never merged back #2662

Merged

keweichen mentioned this pull request Aug 24, 2026

feat(pool): digest live session transcripts instead of reading them #3331

Closed

Conversation

sonichi commented Aug 4, 2026

Six fixes for one class landed on main today

The gap: prevention without detection

It found a seventh instance on its first sampled run

CI, not local — and I had this backwards at first

Exemptions — @Sutando-Pro's design, both constraints enforced

Verified

Not in this PR

Uh oh!

sonichi commented Aug 4, 2026

Uh oh!

sonichi commented Aug 4, 2026

Uh oh!

sonichi commented Aug 4, 2026

Uh oh!

github-actions Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Gate

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

Summary

Uh oh!

john-the-dev left a comment

Choose a reason for hiding this comment

Uh oh!

sonichi commented Aug 4, 2026

Uh oh!

john-the-dev left a comment

Choose a reason for hiding this comment

Uh oh!

bassilkhilo-ag2 left a comment

Choose a reason for hiding this comment

Uh oh!

sonichi commented Aug 4, 2026

Two defects my own first cut had, both caught by controls

Controls, scoped to the same file this time

Uh oh!

bassilkhilo-ag2 commented Aug 4, 2026

Uh oh!

john-the-dev left a comment

Choose a reason for hiding this comment

Uh oh!

john-the-dev left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 4, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Six fixes for one class landed on `main` today

github-actions Bot commented Aug 4, 2026 •

edited

Loading