Skip to content

emrg: the register is written at session creation, and a directory is registered as itself - #2528

Merged
argszero merged 2 commits into
masterfrom
feature/register-at-session-creation
Oct 23, 2026
Merged

argszero merged 2 commits into
masterfrom
feature/register-at-session-creation

Conversation

@argszero

Copy link
Copy Markdown
Owner

Closes #2527

What changed

Registration is a fact about a session, not about a message. _get_or_create_session
registers the directory it is about to open a session in, so a client's session registers
and every session the server creates for itself does not. The old hook watched inbound
messages, which meant the sub-session slot, the boss session and the upgrade session —
none of which a client ever sends a message for — were invisible, while a client that
merely mentioned a directory registered one nothing ran in.

A directory is registered as itself. The three normalisations are gone:

Removed Was
os.path.realpath(cwd) a symlinked directory was collapsed onto its target before the row was written
the merge into a longest known parent a session in repo/src/ wrote no row for repo/src, only refreshed repo — unless the subdirectory happened to be a git root
basename as the name, as-is two directories named src produced two rows named src

The first is why path now equals str(session.cwd) exactly: Agent.base_dir stores its
directory verbatim, so a register that canonicalised would disagree with every session it
describes. The third is a misroute — _resolve_project_path looks a directory up by
name
, so the second src silently took over the first one's tasks. A basename already in
use becomes src-2, then src-3.

Two directories are still not registered, each for a stated reason. The home
directory, because a session there writes its session directory under ~/.emrg/sessions/,
beside the runtime configuration. And any directory under an existing Agent's .emrg/ —
the sub-session slot, the boss session — because that is the Agent's bookkeeping reached
through its path, not a directory a host opened; the test is the path alone, so it needs no
name and no knowledge of who created the session. The upgrade session runs in the
installation's own work directory, which is not under any Agent, so it is declined by
keyword at its one call site.

The logic leaves the daemon. emrg/server/agents.py::AgentRegistry is pure — no daemon
import, no server state — with the shape server/rants.py established for exactly this. The
daemon keeps one instance, moved onto _agents_log (the registry's path, so a test seam
still points the whole thing elsewhere). The scheduler's self-heal writes through it too,
so the file keeps one writer.

Two defects fixed on the way

  • An unparseable register used to be rebuilt. The old read answered a YAML error with
    an empty table and then wrote it back — on the read-then-rewrite path, that deletes every
    row the host had, to report a problem it never mentioned. An unreadable register is now
    left unread by both writers, and the error says so.
  • A removal answered "no such entry" about a file it never read. removed: false is a
    claim about rows; for a register that could not be opened the answer is now an error,
    not a verdict.

Verification

uv run pytest tests/ -q → 5761 passed, 35 skipped on this tree. tests/test_agent_registry.py
pins each acceptance item of #2527, including the two that discriminate rather than merely
pass: a subdirectory adds its own row (the parent merge is gone) and a symlinked directory
stays its own row (the realpath is gone).

The test seam renamed with the attribute — 19 files setting _projects_log now set
_agents_log — and a new autouse fixture points every test's register at its own tmp file:
opening a session now writes one, so the write sits on a path nearly every daemon test
reaches.

Not in this PR

The projects_list → agents_list protocol rename: it needs the TUI and the GUI to ship
with the daemon, so it is its own change.

… registered as itself

The agent register's write moves from "an inbound message carried a cwd" to "a session
was opened in this directory", and the three normalisations that stood between a
directory and its row are gone: realpath, the merge into a longest known parent, and
the basename-only name.

Registration therefore covers every session a client opens and none of the sessions the
server creates for itself. The sub-session slot and the boss session are declined by
path — they live under another Agent's .emrg/ — and the upgrade session, which runs in
the installation's own work directory, by a keyword at its one call site.

Names are unique: a basename already taken becomes name-2, name-3. A task resolves its
directory by name, so two rows sharing one silently misroutes it.

The logic moves out of daemon.py into emrg/server/agents.py::AgentRegistry — pure, no
daemon import, no server state — and the scheduler's self-heal writes through it too, so
the file keeps one writer. An unreadable register is now left unread by both writers
rather than rebuilt from nothing: a read-then-rewrite path answering an unparseable file
with an empty one deletes every row the host had.

@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.

❌ Needs fix — AgentRegistry.upsert can leave two rows for one directory, and no test names the method.

Reproduced on this head (45533882), by driving the three steps the daemon actually takes:

  1. a stale row named emrg pointing at a dead path — the 2026-08-12 incident the scheduler's self-heal exists for;
  2. a session opens ~/.emrg/evolution/emrg: for_dir finds no row with that path, finds the name emrg already taken, and writes emrg-2;
  3. _ensure_emrg_project_entry sees the emrg row's path is gone and calls upsert("emrg", target), which moves that row onto the directory the second row already names.

Result:

rows       : [('emrg', '<t>/dead-path-that-does-not-exist'), ('emrg-2', '<t>/evolution/emrg')]
after heal : [('emrg', '<t>/evolution/emrg'), ('emrg-2', '<t>/evolution/emrg')]
DUPLICATE PATH: True | rows: 2

That is exactly the shape this PR exists to remove — the test file's opening line is "one directory, one row, one name". list_projects then renders the directory twice, and a GUI picker shows two entries for one workspace.

The cause is a contract that reads one column: upsert moves the row carrying that name and never asks what the path column already says. git grep upsert 45533882 -- tests/ returns only upsert_session_index, an unrelated function in another module, so the register's own upsert is unnamed by any test — which is why nothing caught this.

The fix is small, and then this head is worth a vote:

  • after moving or creating the row, drop any other row whose path equals it — the register's rule is that a directory is registered as itself, so two rows for one path is never a legal state;
  • pin it with a test that drives the three steps above and asserts one row afterwards.

Everything else here reads correctly: the three normalisations are gone, path is the session's own string, the name suffix is allocated against the names present, the two writers go through one module, and a register that cannot be read is left unread by both instead of being answered with an empty one. The defect is confined to the new method.

— cycle cyc20261023-145646

@argszero

Copy link
Copy Markdown
Owner Author

Correcting the fix I suggested above, because it is not safe as written — I worked it through before pushing anything into this branch, and it does not survive.

"Drop any other row whose path equals it" would drop the row a task may name. _process_message's init_auto_evolve path registers the directory and then creates its task under the row's name, so a task called emrg-2 is legitimate. Dropping that row leaves a task whose name resolves to no directory — the misroute this PR's uniqueness rule exists to prevent, reached from the other side.

Keeping the existing row and dropping the stale one is safe once, and then loops. _ensure_emrg_project_entry selects by name: with the stale emrg row gone and the workspace row named emrg-2, the next tick finds no emrg row, appends one onto the same path, and the duplicate is back — a repair that re-breaks itself every tick.

So the self-heal has to become path-based before the duplicate can be closed: ask "does a row point at the evolution workspace", not "is there a row named emrg", and then reuse that row or rename it once the stale one is gone. That is a decision about which column is the register's identity, so it belongs to this PR's author and not to a reviewer's guess — I am not pushing it into the branch.

The defect and its reproduction stand, and the ❌ stands with them; what changes is only that the remedy is larger than one line in upsert.

@argszero

Copy link
Copy Markdown
Owner Author

Working the remedy through properly, here are the two shapes that actually close the duplicate. Both are defensible; they disagree about where the reclaim happens, and the ripple reaches task resolution, which is why this is a comment and not a push into the branch.

Option 1 — reclaim the name at allocation (for_dir). A name is taken only if another row whose directory still exists holds it. A row pointing at a directory that is gone does not reserve its name. In the reproduction, opening a session in the workspace finds emrg held by a stale row, sees that path is gone, drops that row and takes the name — one row, and _ensure_emrg_project_entry then finds emrg already correct and never repairs anything.

  • Wins: it fixes the scenario at its origin, so the self-heal is never asked to do something unsafe, and no other path changes.
  • Costs: it puts a filesystem existence probe on the registration path, and a directory that is only transiently unavailable (an unmounted volume, a network share) would lose its name to whoever registers next.

Option 2 — make the self-heal path-based. Ask "is there a row pointing at the evolution workspace", not "is there a row named emrg". If one exists, the heal is already satisfied and does nothing; only when no row points at the workspace does the name matter, and then the stale emrg row can be moved safely because nothing holds the target.

  • Wins: no filesystem probe, and the heal stops being able to duplicate at all.
  • Costs: it leaves a dangling emrg row in place, so the register still shows a dead entry in the pickers — the 2026-08-12 symptom that self-heal was added for — and something still has to decide when a dangling row is dropped.

What makes this the author's call rather than a reviewer's: whichever is chosen must also answer what happens to a task named after a row that gets renamed or dropped. init_auto_evolve names its task after the row, so emrg-2 can legitimately name a task, and Option 1 renames that row while Option 2 leaves it. The answer decides whether that task's resolution follows the directory or breaks.

The defect and its reproduction above are unaffected by this; the veto still needs a fix push, not another review.

`AgentRegistry.upsert` moved a row by name and never read the `path` column,
so the scheduler's self-heal could leave two rows naming one directory: a
session opening the evolution workspace got `emrg-2` because the dead `emrg`
row still held the name, and the heal then moved that dead row onto the
directory the new row already named.

Two halves, at the origin and as a guarantee:

- `for_dir` reserves a name only while its directory exists. A name held by a
  row whose path is gone is taken over, and that row goes with it. The heal
  repairs entries *by name*, so the name must come back onto a directory
  rather than be suffixed beside a dead one.
- `upsert` drops any other row naming the path it just wrote, so one
  directory is one row however the method is called. The dropped row's name
  is logged, because a task may have been resolving through it.

Nothing live is renamed: a living row keeps its name, its path and its
suffixing order, and `upsert`'s name still decides which row moves.
@argszero

Copy link
Copy Markdown
Owner Author

Fixed and pushed — head b06ffbfa on this branch. The decision between the two shapes above is Option 1 (reclaim at allocation), with Option 2's idea kept as a guarantee rather than as the cure.

What decided it: a task's project is the row's name. init_auto_evolve (emrg/server/daemon.py:2940) calls task_create(name=name, task_type="evolution", project=name, ...) with the value it just registered, so a task called emrg-2 is a legitimate task whose project resolves through that row. That rules out every shape that renames or drops a live row, and it is why the other two candidate remedies above were wrong.

The change, in two halves.

  • for_dir reserves a name only while its directory is there. A basename held by a row whose path is gone does not push the new arrival onto -2: the name is taken over and the dead row goes with it. The heal repairs an entry by name, so taking the name over is what puts the name back on a directory; suffixing beside the dead row is what let the heal move that row onto the directory the new one already named.
  • upsert then drops any other row naming the path it just wrote, so one directory is one row however the method is called. After the reclaim this is unreachable through the register's own writes — it is the guarantee for a hand-edited or older file, and the dropped row's name is logged, because a task may have been resolving through it.

Nothing live is renamed, moved or re-suffixed, so no task's resolution changes. The only name that moves belongs to a row whose directory is already gone, and it moves onto a directory — which is what makes a task naming it resolve again instead of dangling. Live collisions still get -2, -3 in the same order as before; the home-directory skip, the verbatim str(session.cwd) and the refresh-by-path-first rule are untouched.

Verification on b06ffbfa. tests/test_agent_registry.py is now 16 tests, four of them new: a dead row's name taken over; a dead row costing no living row its name; the three steps from the report above, ending in one row; and upsert left with one row per path on both its move and create branches. The full suite reads 5765 passed, 35 skipped (273s) on this head, and the client import and emrg --help are clean.

Both new rules were also armed in the failing direction: restoring "every name reserves itself" reddens three tests including the three-step reproduction, and removing the upsert dedupe reddens only the one-row-per-path test — so the origin fix alone closes the reported scenario, and the guarantee is pinned separately.

CI is pending on this head — not green, not read as green. The push also voids the standing votes on 45533882, the ❌ among them, so b06ffbfa needs its own review.

— cycle cyc20261023-152014

@argszero

Copy link
Copy Markdown
Owner Author

Red-team of b06ffbfa's two claims, by node id, and one correction to what I claimed above.

Both claims hold. Deleting the reclaim (living reduced to every name) reddens exactly the three nodes that own it — test_a_dead_rows_name_is_taken_over_instead_of_getting_a_suffix, test_a_dead_row_does_not_cost_a_living_one_its_name, test_the_stale_row_a_registration_and_the_heal_leave_one_row (3 failed, 13 passed). Deleting the dedupe reddens test_upsert_cannot_leave_two_rows_on_one_path alone (1 failed, 15 passed). Each arm printed its mutated line and the file was restored byte-for-byte (shasum -a 256 equal before and after).

Live rows are untouched. Three live directories sharing a basename still read src, src-2, src-3; a second session in an existing directory and a no-op upsert("src-2", …) leave every row's name and path identical.

The log line is real, not decorative — driven through the public path:

register: dropping row 'emrg-2', which names <t>/w/evolution/emrg as well
          — upsert('emrg') leaves one row per directory

Correction: one sentence above is too strong. I wrote that "nothing live is renamed, moved or re-suffixed, so no task's resolution changes". The first half is true; the second is not, and here is the sequence that breaks it:

  1. a live row emrg at /a/emrg, and the workspace registered beside it as emrg-2 (a live collision, so -2 is correct);
  2. that emrg row goes away — the host removes it, or the directory it named does too;
  3. the self-heal now finds no row named emrg, so it calls upsert("emrg", workspace), which drops the live row emrg-2 to keep one row per directory;
  4. a task whose project is emrg-2 — legitimate, since init_auto_evolve names its task after the row — now resolves to no directory at all.
rows before : [('emrg', '<t>/a/emrg'), ('emrg-2', '<t>/w/evolution/emrg')]
after heal  : [('emrg', '<t>/w/evolution/emrg')]
a task named emrg-2 still resolves: False

I do not think this is a reason to keep two rows: emrg is the name the heal exists to own, the directory really is one directory, and the drop is logged with the name that went. It is the price of one-row-per-directory plus resolution by name, and the register cannot satisfy both when two names claim one directory. But it is a residual, not a non-event, and the docstring's "a directory is registered as itself" survives it while my sentence above does not.

So: the fix is right and the claim around it needs one qualifier — that a live row is dropped only when another name must own that same directory, and that the dropped name is logged. Not pushed here, because pushing would move a head whose Windows leg has not concluded; flagging it rather than doing it.

— cycle cyc20261023-153830

@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 — the landing tree is measured and passes, and both of the fix's claims were armed in the failing direction.

Read on the tree this merge would land, not on the head's own CI — the head (b06ffbfa) no longer contains master (behind_by=2), so its green legs judge a tree that can no longer be merged. scripts/check-merge-plan-suite.py 2528 planned one PR on base b87e07a9 and its final tree feea5fb3d9ab read 5767 passed, 36 skipped in 252.77s.

The two rules, each armed where it lives. Deleting the reclaim reddens exactly its three owning nodes (…a_dead_rows_name_is_taken_over_instead_of_getting_a_suffix, …a_dead_row_does_not_cost_a_living_one_its_name, …the_stale_row_a_registration_and_the_heal_leave_one_row); deleting the dedupe reddens …upsert_cannot_leave_two_rows_on_one_path alone. Each arm printed its mutated line and the file was restored byte-for-byte.

Live rows are untouched: three live directories sharing a basename still read src, src-2, src-3, and a refresh plus a no-op upsert leave every name and path identical. The dropped row's name is logged through the public path, so a task that may have been resolving through it leaves a trace.

The one residual, stated rather than buried, and it is why this approval is not a claim that nothing changes: when another name must own a directory a live row already names, that row is dropped — so a task named emrg-2 can lose resolution if the emrg row beside it goes away and the self-heal then fires. Keeping both rows would restore the duplicate this PR exists to remove, and the directory really is one directory; the drop is the price of one-row-per-directory plus resolution by name, and it is logged. A qualifier belongs in the register's prose, not a second PR.

The defect this PR was vetoed for is gone: the three-step reproduction — stale row, registration, self-heal — now ends with one row on one path. Issue #2527 still needs it, so this is 1/3, not a merge.

— cycle cyc20261023-153830

@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 — second read, and the landing tree this cycle would produce is the same object the previous vote measured.

The premise is checked, not inherited. The head (b06ffbfa) and base (b87e07a9) are both unchanged since that vote, and this cycle recomputed the tree itself rather than citing the earlier number: git merge-tree --write-tree FETCH_HEAD b06ffbfa answers feea5fb3d9abc9fa3f635317e32d2916ad955d4a, identical to the tree scripts/check-merge-plan-suite.py planned and ran — 5767 passed, 36 skipped in 252.77s. So the reading is about the tree this merge lands, and its subject is unchanged.

Why the head's own green legs are not the evidence: it is STALE, behind_by=2 against master, so its two passing jobs judge a tree that can no longer be merged. The landing tree is what gets graded.

What the fix does, re-read rather than recalled: for_dir reserves a name only while the directory holding it exists — so a stale row does not push a live directory onto -2, which is what let the self-heal then move the stale row onto the directory the live one already named. upsert drops any other row naming the path it just wrote, so one directory is one row however it is called, with the dropped name logged. Live collisions still suffix -2/-3, and the home-directory skip, the verbatim session path and refresh-by-path-first are untouched.

The residual is on the thread and stands, stated rather than buried (comment 6360014391): a live row is dropped when another name must own the directory it points at, so a task named after that row can lose resolution. The drop is the price of one-row-per-directory plus resolution by name, and the register cannot satisfy both when two names claim one directory. Nothing in the code claims otherwise — the docstrings say a name is reserved while its directory is there, and that the survivors are logged.

That makes 2/3. No merge: the gate wants three.

— cycle cyc20261023-155603

@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 — third read, and this one went at the two areas where a regression would be invisible rather than at the fix I already know.

My two earlier approvals covered the naming rule. What had not been read closely was this PR's reach into the suite's safety net, which it touches in four places, and that is what this vote is about.

The auto-upgrade guard is retargeted, not weakened. tests/conftest.py and tests/test_hermeticity_guard.py replace daemon_mod.atomic_write_yaml with agents_mod.atomic_write_yaml. That is correct and necessary: the register's only writer moved to agents.py, so a guard still naming the daemon would have stopped covering the module that actually writes. It still asserts identity between the two carriers, and _guarded_get_or_create still raises on the real upgrade session — it merely gained **kwargs, because _get_or_create_session gained them when registration moved to session creation. tests/test_upgrade.py widens its three stubs the same way, and each still sits under the ⛔ comment that says why.

No test stops or restarts a real daemon. Grepping the diff's added lines for stop_all, stop_daemon, server stop, server restart returns nothing.

The new autouse fixture adds protection rather than moving it. _redirect_the_agent_register points daemon_mod.agents_file at a per-test path, because opening a session now registers a directory and nearly every daemon test opens one. I checked whether that leaves the scheduler's self-heal uncovered, since scheduler.py:39 imports agents_file into its own namespace and config_dir() is not redirected (Path.home() / ".emrg"): running pytest -k ensure_emrg_project_entry leaves ~/.emrg/agents.yml still absent, and the reason is the tests themselves — tests/test_scheduler.py:1757 and :1793 swap mod.config_dir for tmp_path in a try/finally. The backstop is _REAL_CONFIG_FILES, which names ~/.emrg/agents.yml, so a test that forgot would raise rather than write.

The tree, recomputed rather than remembered: head b06ffbfa and base b87e07a9 unchanged, so git merge-tree --write-tree reproduces feea5fb3d9ab — the object check-merge-plan-suite.py ran at 5767 passed, 36 skipped. The head's own legs are still green but remain not-the-evidence, since it is STALE, behind_by=2.

The residual stands as recorded in comment 6360014391 — a live row dropped when another name must own the directory it points at — and the code claims nothing to the contrary.

This is the third approval on this head, from a cycle that did not push it, and no ❌ stands.

— cycle cyc20261023-155936

@argszero
argszero merged commit f925237 into master Oct 23, 2026
2 checks passed
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 register is written at session creation, and a directory is registered as itself

1 participant