Repository navigation
Conversation
An agent exists exactly when its identity is committed (by-id/<nodeId>/identity.json, in SYM_IDENTITY_DIR when set). The name index and the 0.13 compat links are caches of which agent a name is: written after the commit, re-derived from the committed identities' names when missing or unreadable, never read as "no such agent". Names are compared case-insensitively (NFC, lower case): the entry is keyed by the folded name and holds the exact one. A 0.13 directory is moved in by ONE rename (node dir first, identity last); the compat link is made before the commit and renamed onto the 0.13 path right after it. A 0.13 directory behind a link, on a mount point, on another volume, or holding a relative link that leads out of it is linked to from by-id, never copied; across volumes only the identity files are copied (staged, synced, read back, committed by one rename) after a free-space check. A husk is set aside whole under a short per-nodeId lock and reported; a mint holds the name's short mint lock from staging to claim. Keys never left loose: dirs 0700 and keys 0600 narrowed on every load, asides narrowed at once and on load, dead staging, temp links, temp entries and temp lock files cleared on every load, a stray key copy in the node dir removed. Export: intent, tombstone (beside the key and in the node dir), the bundle's hash, the bundle, complete, the scrub of every copy of the key. Resume needs the same out (while a complete bundle may exist) and the same recipient; abort only while no complete bundle was produced, also on the CLI (sym node abort-export). Import: the identity first, each install staged and marked with its bundle; an interrupted import is finished by running it again. lib/node.js (identity and lock only): once the identity lock is held, at construction and at a restart after a stop, an identity exported or being exported meanwhile is refused and the lock released.
…identity-v2), and the spec tests moved to v2 tests/identity-v2.test.js: 36 tests, each a scenario of the re-review (rr-eN) or of R1 that fails on 9f6847c or on 0.14.2 (fe285a3), plus one per guard of the design (each caught by its mutant). identity-commit and identity-migration-by-id: the expectations the copy design wrote are changed to the move design, each with its reason in the test (moved not copied, linked not copied, the index a cache, the tombstone before the bundle, a husk set aside not a halt).
…ed versions, rollback and timing prepare.js git-archives v0.14.2 and v0.13.17 (rollback trees); kill.js kills one operation at every mutating fs call (before and after it) and checks the next start, a pinned start, a 0.14.2 and a 0.13 rollback (migration, mint, husks, SYM_IDENTITY_DIR, across volumes with SYM_FUZZ_FAR, export, import); race.js starts one agent from N processes at once (plain, pinned, husk, split, new, SIGKILLed at random, mixed with 0.14.2); rollback.js runs a 0.14.2 host through this tree twice and back, and times the first start of a 5,000-file agent and of 12 agents in one process.
… as built, upgrade notes, known limits, numbers The section describes what ships (one source of truth, moved not copied, linked across volumes, the export order), with the upgrade notes (upgrade every embedder on a host before the first start; the behaviour changes), the limits left (the one-call window, mixed versions, an export bundle moved away before its complete record) and the measured numbers (tests, mutation, kill -9 at every step, races, rollback, rehearsal, timing, disk).
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd12d835d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!resuming) install(idDest, split ? (staging) => write(staging, idFile) : storeFiles); // THE COMMIT | ||
| if (split && !ours(dest)) install(dest, storeFiles); |
There was a problem hiding this comment.
When SYM_IDENTITY_DIR is separate, this installs idDest first, making the identity discoverable and startable before dest contains the bundled store. If the process dies between these calls, an automatic start by name can claim the identity and run against an empty or stale node directory; while it holds the lock, retrying the import fails, and after it stops the retry archives any newly written state as a husk rather than merging it into the imported history. Keep the identity unstartable until the store installation is complete, for example by installing the store first and treating the identity installation as the commit.
Useful? React with 👍 / 👎.
| if (srcs.nodes) { | ||
| const lock = readLockFile(path.join(srcs.nodes.path, 'lock.pid')); | ||
| if (lock && !lock.migrating && lock.pid !== process.pid && lockIsHeldByLiveProcess(lock)) return { status: 'skipped-live', nodeId, holderPid: lock.pid }; |
There was a problem hiding this comment.
In the mixed-version migration path, this one-time lock check leaves a race with a 0.13 process: it can load nodes/<name>/identity.json after this check, the migration can rename that directory to by-id, and the old process can then recreate nodes/<name> when acquiring its lock. The temporary compatibility link cannot replace that nonempty directory, while the new process acquires the unrelated lock in by-id, so both processes run the same key concurrently. Acquire and retain the legacy directory lock across the rename/link transition rather than merely sampling it.
Useful? React with 👍 / 👎.
| for (const d of new Set([config.identityDirById(id), dir])) { | ||
| try { fs.unlinkSync(path.join(d, config.EXPORTING)); fsyncDir(d); } catch { /* */ } | ||
| try { fs.unlinkSync(path.join(d, config.TOMBSTONE)); fsyncDir(d); } catch { /* */ } | ||
| } | ||
| return { nodeId: id, aborted: true, bundle: intent.out || null, bundleRemoved }; | ||
| removeIntent(id); |
There was a problem hiding this comment.
During abortExport, every tombstone unlink error is swallowed and the export intent is removed unconditionally. In split-directory mode, if deletion succeeds beside the key but transiently fails in the node directory, the call reports success yet readTombstone() still finds the remaining copy; because the intent is gone, future aborts classify the export as completed and the node remains permanently refused despite no bundle being produced. Only remove the intent after all existing tombstone copies were removed successfully.
Useful? React with 👍 / 👎.
Replaces the copy-and-commit identity layer of the 0.14.3 candidate (9f6847c) with design v2: one source of truth, and a 0.13 agent moved (or linked to), never copied. Into
release/0.14.3; not merged, tagged or published.The design as built
nodes/by-id/<nodeId>/identity.json; withSYM_IDENTITY_DIR,SYM_IDENTITY_DIR/by-id/<nodeId>/identity.json).by-nameentries and thenodes/<name>compat links are caches: written after the commit; when missing or unreadable, re-derived from the committed identities'name(the compat link a second witness); never read as "no such agent". An entry naming an identity not in place halts; two committed identities of one name halt naming both.link()claim).by-id/<id>becomes an absolute link to it. Across volumes onlyidentity.json/e2e-keypair.jsonare copied (staged, synced, read back, one rename) after a free-space check.by-id/.<id>.husk-…, short per-nodeId lock), the node moved in, differing files kept and reported (SYM_HUSK_SET_ASIDE).complete→ scrub of every key copy. Resume: same out (while a complete bundle may exist), same recipient.abortExportonly while no complete bundle was produced; CLIsym node abort-export. Import: identity first, each install marked with its bundle, a killed import finished by running it again.SymNodere-checks after taking the identity lock.lib/node.jsis touched only where the identity is loaded/locked (_assertStillHereafteracquireIdentityLock, at construction and in_takeOwnership).Deviation from the brief
The mint takes a short per-name lock instead of racing lock-free with "the loser removes its own identity": a loser killed between its commit and its cleanup leaves a second committed identity of the name, which no later start can tell from a real second agent. With the lock there are no losers; a killed minter's identity is adopted by the next start.
Results
Every R1 and re-review scenario ends correct on this branch (the review scripts pointed at this tree, 9f6847c and fe285a3, all from
git archivecopies):kill -9 at every step (
scripts/identity-fuzz/kill.js, SIGKILL before and after each mutating fs call): 492 crash points (489 reached) over plain / pinned / husk (stale lock) / husk with files / SYM_IDENTITY_DIR / by-id on another volume / SYM_IDENTITY_DIR/by-id on another volume (HFS+ image) / mint / mint split. The next 0.14.3 start: the same agent, whole store, keys 0600, at every point. 0.14.2 / 0.13 rollback: a second identity only at the one point between the commit rename and the compat-link rename (2 points per scenario = one instant), and where 0.14.2's own bugs act on untouched state (a husk: 0.14.2 destroys the key itself; EXDEV). Export 102 + 130 (split), import 104 + 132: never a startable source beside a complete bundle; abort or resume always completes; one early split run had 2 of 130 points report a key file left after resume, not reproduced in 4 reruns (516 points) + 96 targeted repeats.Multi-process: rr-e2 (6 starters, legacy/new/husk/lock, 5 rounds each) all GOOD; rr-e10 chaos 20/20 GOOD; rr-e11 6/6; rr-e3 lock race 6/6 one holder;
race.jsplain, pinned-mix, husk-content, split, new, chaos 5/5 each. Mixed 0.14.2/0.14.3 (rr-e21, 20 rounds of 6): 0.14.3 only 20/20; 0.14.2 only 9/20 bad (0.14.2's own TOCTOU: skipped-duplicate → mint); mixed 12/20 bad — the original key kept, and 0.14.3 then halts naming both identities. Mixed lock race: 1 of 6 rounds two holders (0.14.2's non-atomic reclaim). Hence the upgrade note: upgrade every embedder before the first start.Rollback / rehearsal:
rollback.js(0.14.2 host: migrated, minted, renamed, re-pointed, mixed case, split) through this tree twice and back: every agent same in every phase. Rehearsal: APFS clone of a sandboxed 15-agent 0.13.17 + 0.14.2 host (real stores, links, husk, rename, pinned start under a second name): 15/15 unchanged by name, pinned and as SymNode through two 0.14.3 starts and a 0.14.2 rollback.Mutation: 42 mutants (one per guard), 42 caught.
Suite: full suite
--test-concurrency=21688 tests, 1684 pass, 0 fail, 4 skipped; integration 6/6;gate-two-stock-nodespassed;npm audit0. GitHub CI (Linux, Node 20 and 22) green. Linux docker (node:22-slim, node:20-slim, overlayfs, no cloning) for the identity test files (identity-v2, identity-commit, identity-migration-by-id, core-secure-identity, core-secure-final-review, config, identity-halt): 200 tests, 192 pass, 4 skipped, 4 fail — the lock tests that read process start times withps, which the slim images do not ship (the same 3 config.test.js tests fail on 9f6847c there; all pass in CI and on macOS; the lock code is unchanged). Before-check in docker: the two case-sensitive tests (exact-name entry, names that fold together) fail on both 9f6847c and 0.14.2 on ext4/overlay.Left open (in the CHANGELOG's known limits)
outbetween its rename and thecompleterecord cannot tell it was complete.