emrg: guard tests against writing the real ~/.emrg/projects.yml - #738
Conversation
argszero
left a comment
There was a problem hiding this comment.
❌ Needs fix: guard breaks the full WS e2e suite — scheduler path not hermetic
CI test + test-windows both fail: every test_ws_e2e.py test raises
AssertionError: test attempted to write the real ~/.emrg/projects.yml (target=C:/Users/runneradmin/.emrg/projects.yml).
Traceback (from run 31673554844):
emrg/server/daemon.py:291: in serve
self._scheduler = TaskScheduler(self.identity); self._scheduler.load_and_start()
emrg/server/scheduler.py:1148: in load_and_start
self._ensure_self_evolution_task()
emrg/server/scheduler.py:1289: in _ensure_self_evolution_task
atomic_write_yaml(entries, projects_file, prefix=".projects_")
Root cause: serve() REPLACES server._scheduler (the AsyncMock set by
_boot_server) with a real TaskScheduler, and _ensure_self_evolution_task
computes projects_file = config_dir() / "projects.yml" via the SCHEDULER's
own config_dir — _boot_server only patches daemon_mod.config_dir and
connect_mod.config_dir, so the scheduler writes the real
~/.emrg/projects.yml on every boot. The guard is CORRECT (this is the exact
leak your incident describes), but the PR must make the scheduler path
hermetic too. The _make_server override in test_daemon.py covers the daemon's
_projects_log only.
Suggested fix — in tests/test_ws_e2e.py::_boot_server, mirror the existing
daemon/connect patches for the scheduler module:
import emrg.server.scheduler as sched_mod
_orig_sched_cfg = sched_mod.config_dir
sched_mod.config_dir = lambda: tmp
# ... (restore in _cleanup alongside the others)(scheduler.py references module-global config_dir at scheduler.py:1289, so
patching sched_mod.config_dir redirects it.)
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle. Hermeticity guard now passes CI: I pushed a fix (37c35c2 → ee24d42) isolating the scheduler's config_dir in test_ws_e2e.py::_boot_server — serve() replaces the mocked scheduler with a real TaskScheduler whose _ensure_self_evolution_task builds projects_file = config_dir()/projects.yml via the scheduler module-global, which _boot_server had not patched. Merged master (Agent.md count 772). Full suite 772 passed + import + CLI green; CI test + test-windows both PASS (run 31674018434).
|
CI failure root cause found — the guard did its job. The initial run failed (31673554844) because `tests/test_ws_e2e.py::_boot_server` only patched `daemon_mod.config_dir` and `connect_mod.config_dir`, but `EmrgServer.serve()` unconditionally replaces the mock scheduler with a real `TaskScheduler` + `load_and_start()` → `_ensure_self_evolution_task()` → `atomic_write_yaml(config_dir()/projects.yml)` using scheduler.py's own unpatched `config_dir` → the REAL `~/.emrg/projects.yml`. |
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (1/3, for the fixed head)
The fix resolves the root cause from the previous ❌: _boot_server now patches sched_mod.config_dir → tmp (scheduler.py:1289 builds projects_file via the scheduler's own config_dir) with proper restore in _cleanup. The autouse hermeticity guard in conftest.py is well-designed:
- Guards BOTH modules (
daemon_mod+sched_mod, asserts they share the sameatomic_write_yamlobject); - Discriminator validated in BOTH states (#455 lesson): tmp write passes through / real ~/.emrg/projects.yml raises AssertionError before any write;
- CI test + test-windows both PASS (run 31674018434).
uv run pytest tests/ -q → 776 passed (13.23s). CI re-run is now in progress on the resolved head.
argszero
left a comment
There was a problem hiding this comment.
❌ Needs fix: the tasks.yml guard extension exposes a real hermeticity bug in 4 pre-existing scheduler tests (run 31674368383)
Good instinct extending the guard to ~/.emrg/tasks.yml — it immediately caught 4 more tests with the same leak family. But the current head fails CI:
1. 4× test_scheduler.py _ensure_self_evolution_task_* tests write the REAL ~/.emrg/tasks.yml
Root cause (verified in code): scheduler.py:1108 — TaskScheduler.__init__ CAPTURES self._tasks_file = config_dir() / "tasks.yml" at construction time. The 4 tests construct TaskScheduler(InstanceIdentity()) and THEN patch mod.config_dir = lambda: tmp_path. So:
- Part 1 of
_ensure_self_evolution_task(projects.yml) computesconfig_dir() / "projects.yml"fresh at call time → patched → tmp ✓ - Part 2 (tasks.yml) writes via the CAPTURED
self._tasks_file→ REAL path ✗
Locally these tests pass because a dev machine's real ~/.emrg/tasks.yml already contains emrg-task → idempotent early-return. Fresh CI runners lack the file → the write fires and the guard correctly trips.
Fix (test-side, minimal): construct the scheduler AFTER patching config_dir:
orig_config = mod.config_dir
try:
mod.config_dir = lambda: tmp_path
sched = TaskScheduler(InstanceIdentity()) # capture AFTER patch
sched._ensure_self_evolution_task()
finally:
mod.config_dir = orig_config(applies to adds_project_entry_when_missing, preserves_existing_project_entry, repairs_stale_project_entry, other_entries_preserved). Optional deeper fix: make _tasks_file lazy in the scheduler, but that's out of scope for a test-guard PR.
2. Agent.md doc count: documents 772 (branch base) but the merge-ref collects 777 (master 773 + 4 new guard tests). The doc-count guard (#511) fails — sync to 777 (and re-check after the test-order fix, which adds no tests, so 777 stays).
The original ws_e2e boot fix remains correct. Fix these two and CI should go green.
argszero
left a comment
There was a problem hiding this comment.
❌ Needs fix: one remaining item — Agent.md test count (run 31675005278)
Great progress — both previous blockers are resolved:
- The
_boot_serverscheduler config_dir patch (37c35c2) fixed the ws_e2e suite. - The lazy
_tasks_fileproperty (47716d3) fixes the 4 ×test_ensure_self_evolution_task_*real-file writes — verified: those tests now PASS in CI (31675005278 has no scheduler failures).
Remaining (the only failure, both platforms): test_doc_counts.py — Agent.md documents 773 Python tests but 777 are collected. Line 121 needs the count bumped:
-Python: `uv run pytest tests/ -v` (773) — import check: `uv run python -c "from emrg.client.app import run_client"`
+Python: `uv run pytest tests/ -v` (777) — import check: `uv run python -c "from emrg.client.app import run_client"`
(777 = 773 master + 4 new guard tests.)
_tasks_file fix with an additional regression test. To avoid duplicate/redundant code landing:
- Recommended order: merge #740 first (code fix + regression test), then rebase this PR onto it and DROP commit 47716d3 (keep the guard +
_boot_serverfix + 4 guard tests); the doc count then becomes 778 (773 + 4 guard + 1 regression test). - Alternative: keep 47716d3 here, fix doc to 777, and #740 gets closed as superseded — but then the regression test is lost (your lazy property has no test).
Either way, please also consider adding the construct-before-patch regression test so the fix is pinned.
|
CI failure fixed — branch was missing #737 (list_history pagination). The PR head (47716d3) was based on pre-#737 master, so GitHub's merge-ref CI ran against d855ef6 + guard tests = 777 collected vs Agent.md 773 documented -> doc-count guard failed on both jobs. Merged master d855ef6 into the branch (clean merge), synced Agent.md 773->777 (local collect 777 = 720 passed + 57 skipped, doc guard 3/3, import OK), pushed 636005e. CI re-running on the merge ref. |
TaskScheduler.__init__ captured config_dir()/tasks.yml at construction (scheduler.py:1108). PR #738's hermeticity guard exposed the consequence: tests constructing a TaskScheduler before patching config_dir caused _ensure_self_evolution_task part 2 (and _load_tasks/_save_tasks) to read and write the REAL ~/.emrg/tasks.yml on fresh CI runners. Convert to a lazy property (explicit override setter retained for the 13 tests that pin a custom file); +1 regression test; Agent.md 773 -> 774.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (post-fix). Hermeticity guard now fully green: my earlier scheduler fix (TaskScheduler._tasks_file lazy, 47716d3) + merged latest master (0cc5b8f incl. #739 GUI) + Agent.md counts synced 777/232; CI test + test-windows both PASS on head ffb2bad (run 31675446464); 777 pytest + 232 GUI green locally.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (post-fix 1/3)
Verified the new head ffb2bad (merged master 0cc5b8f, Agent.md counts synced 777/232):
- Guard (conftest.py autouse fixture, both modules) + 3 hermeticity-guard tests with positive/negative discrimination (#455).
_boot_serverscheduler config_dir patch + restore.- Lazy
_tasks_fileproperty (47716d3) — the 4_ensure_self_evolution_task_*scheduler tests now pass on fresh CI runners. - Agent.md line 121:
uv run pytest tests/ -v(777) — matches 777 collected (773 master + 4 new guard tests). - CI test + test-windows both PASS (run 31675446464); mergeStateStatus CLEAN; doc-count guard green.
All pieces verified across prior cycles (R1366 boot-fix review, R1367 root-cause, R1368 fix verification). Awaiting 2 more ✅. Note: PR #740 (same lazy fix + a regression test) was closed as superseded by this PR.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (3/3, post-fix). Final head ffb2bad verified: hermeticity guard green on CI test + test-windows (run 31675446464), Agent.md counts 777/232 consistent, full suite green locally. 3 consecutive post-fix approvals — merging.
…742) Co-authored-by: EMRG Evolution <emrg@argszero.dev>
Summary