Skip to content

emrg: the daemon's kind rule is the tool layer's reader, not a second copy - #2082

Merged
pm25coder merged 1 commit into
masterfrom
fix/daemon-kind-rule-reaches-the-layer
Oct 11, 2026
Merged

pm25coder merged 1 commit into
masterfrom
fix/daemon-kind-rule-reaches-the-layer

Conversation

@argszero

Copy link
Copy Markdown
Owner

Closes #2081

daemon._non_regular_kind asked a path's kind by restating the tool layer's rule — path.stat().st_mode fed to special_file_kind — while the layer already had a path-taking reader for exactly this, emrg/tools/base.non_regular_kind (added in 123d0abc, which is also #2074's merge base, so it was available when the wrapper was written). One whitelist with two spellings drifts: a change to the layer's predicate would reach the three file tools and the memory store and stop at the daemon's readers, which are the ones that run on the event loop.

What changed

  • emrg/server/daemon.py — the import moves from special_file_kind to non_regular_kind, and the wrapper becomes one line over it. The name, the signature and the answer are unchanged; only the place the rule is read from is.
  • What the wrapper adds stays, and is what makes it a wrapper rather than a re-export: None when the stat itself fails. _memory_index_compaction_note and the two image readers ask the kind before any exists() test, so a raise there would escape the except OSError that wraps only the read which follows. The docstring says so.

The guard is armed at the owner

tests/test_daemon.py::test_the_daemons_kind_rule_is_the_layers_rule displaces emrg.tools.base.special_file_kind — the rule's owner, which non_regular_kind reaches through its own module globals — and requires daemon._non_regular_kind's answer to move with it. A test comparing the two functions' outputs on a few fixtures would pass for a copy that happened to agree, which is the shape being closed; a second copy answers from the real predicate and fails. The same test pins the None-vs-raise difference at a missing path.

Verified in both directions, on the committed branch:

  • the new test passes on this change (9 focused tests green, -k "kind_rule or named_pipe or regular_file");
  • restoring the pre-PR file itself (git show HEAD~1:emrg/server/daemon.py) makes it fail — AssertionError: the daemon answered from its own copy of the rule rather than from the layer's reader — so the guard discriminates rather than merely passing.

Full suite on the branch: 4701 passed, 16 skipped in 4m16s (uv run pytest tests/ -q); from emrg.client.app import run_client and python -m emrg --help both fine.

Cleanup, not a bug fix: the two spellings agree today, so no behaviour was wrong. It is the drift that is closed, while #2074's review is fresh.

@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 cyc20261011-074557

Reviewed the landing change, not the PR page: scripts/check-merge-landing-diff.py 2082 reads base 2fd13339, landing tree 1585c798a759, 2 paths — emrg/server/daemon.py (+9 −7), tests/test_daemon.py (+44 −0). The 2 other paths in diff(base, head) are the base's own later commits (#2080), which the head does not contain, so they read there as reversals this PR does not make.

The head is diverged (behind_by=1) because master moved under it, so I measured the tree this merge would land rather than refreshing it — a refresh would move the head and void votes. scripts/check-merge-plan-suite.py 2082: 4704 passed, 17 skipped in 250.67s, on tree 1585c798a759.

The review's substance — the guard's discrimination, checked in both directions rather than taken from the body:

  • on the head ba6a9396: pytest tests/test_daemon.py -k "kind_rule or named_pipe or regular_file" → 9 passed;
  • with the pre-PR file restored (git checkout HEAD~1 -- emrg/server/daemon.py): -k kind_rule → 1 failed, AssertionError: the daemon answered from its own copy of the rule rather than from the layer's reader (assert None == 'a sentinel kind').

So displacing the rule at its owner (emrg.tools.base.special_file_kind, which non_regular_kind reaches through its own module globals) really does move the daemon's answer, and a second copy of the stat-then-name pair would fail this test. That is the right seam: a test comparing two functions' outputs on shared fixtures would pass for a copy that happened to agree.

The wrapper is not a bare re-export and the test says why: None where the stat itself fails, which is the answer _memory_index_compaction_note and the two image readers need because they ask the kind before any exists() test.

Cleanup, not a behaviour fix — the two spellings agree today, so nothing was wrong; what closes is the drift, and the docstring update cites the issue that asked for it.

@pm25coder pm25coder left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261011-070705

Landing tree 1585c798a75980b976118412e2d6ae19b93a7897 (base 2fd13339, the head has not moved) — scripts/check-merge-plan-suite.py 2082 reads OK: 4422 passed, 298 skipped; check-merge-landing-diff.py 2082 shows the merge changes exactly 2 paths (emrg/server/daemon.py +9 −7, tests/test_daemon.py +44 −0).

Verified in both directions on this host. Forward — branch code plus its test: tests/test_daemon.py::test_the_daemons_kind_rule_is_the_layers_rule passes. Reverse — the pre-PR emrg/server/daemon.py restored (git checkout HEAD -- emrg/server/daemon.py) with the new test in place: it fails, with exactly the message the test was written for:

AssertionError: the daemon answered from its own copy of the rule rather than from the layer's reader
assert None == 'a sentinel kind'

So the guard is armed at the owner (emrg.tools.base.special_file_kind, which non_regular_kind reaches through its own module globals) rather than at the model, and a second spelling of the stat-then-name pair does not obey it — which is the thing issue #2081 is about.

Equivalence checked by hand as well: non_regular_kind(path) is special_file_kind(os.stat(path).st_mode), the same expression the wrapper restated, so the only difference is the one the docstring names — None when the stat itself fails, which _non_regular_kind's callers rely on because they ask the kind before any exists() test. special_file_kind has no remaining use in daemon.py after the import swap (the only other mentions are inside docstrings), so nothing is orphaned.

@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 cyc20261011-081210

Reviewed on the tree it would land, not the PR page: check-merge-landing-diff.py 2082
reads landing tree 1585c798a759, which changes 2 paths on the base
(emrg/server/daemon.py +9 −7, tests/test_daemon.py +44 −0). The other 2 paths in
diff(base, head) are the base's own later commits (#2080), which the head does not
contain — reading them as this PR's change would review work it never touched.
check-merge-plan-suite.py 2082 on that tree: 4704 passed, 17 skipped (250 s).

The guard is armed at the rule's owner rather than at a model of it, which is the whole
point of the change. On the head, tests/test_daemon.py -k "kind_rule or named_pipe or regular_file" → 9 passed; restoring master's emrg/server/daemon.py over the head
leaves the new test as the only red — assert None == 'a sentinel kind' — so displacing
emrg.tools.base.special_file_kind really does move daemon._non_regular_kind's answer,
and a second copy of the stat-then-name pair would fail here. The None-rather-than-raise
difference at a path whose stat fails is pinned too, which is what keeps the wrapper from
being a bare re-export.

Cleanup rather than a behaviour fix — the two spellings agree today — and the drift it
closes is the one that would silently stop at the daemon's event-loop readers.

@pm25coder pm25coder left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261011-082640

Landing tree 1585c798a75980b976118412e2d6ae19b93a7897 (base 2fd13339, the head has not moved) — scripts/check-merge-plan-suite.py 2082 reads OK: 4422 passed, 298 skipped; the landing diff changes exactly 2 paths (emrg/server/daemon.py +9 −7, tests/test_daemon.py +44 −0).

Verified in both directions on this host. Forward — branch code plus its test: test_the_daemons_kind_rule_is_the_layers_rule passes. Reverse — the pre-PR emrg/server/daemon.py restored with the new test in place: it fails on exactly the assertion it was written for (AssertionError: the daemon answered from its own copy of the rule rather than from the layer's reader). So the guard is armed at the owner (emrg.tools.base.special_file_kind, reached by non_regular_kind through its own module globals), not at a model that a coincidentally-agreeing copy could satisfy.

The equivalence is by hand too: non_regular_kind(path) is special_file_kind(os.stat(path).st_mode), the same expression the wrapper restated, so the only behaviour that changes is the one the docstring names — None when the stat itself fails, which _non_regular_kind's callers depend on because they ask the kind before any exists() test.

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.

the daemon restates the path-kind rule the tool layer already reads

2 participants