Repository navigation
Buddy: the Platform derives his persona byte for byte as the installer and brings a change to running conversations when he is idle (DEV-6676 PR 2, F64) - #636
Conversation
agentrozjedemeai
left a comment
There was a problem hiding this comment.
@immakermatty I reviewed exact head cea70f4 against 9781aed. The focused local tests pass (40/40), but these failure and concurrency paths block approval:
src/buddy/persona.ts:654-663,761-763: the revision is checked beforerename, with no serialization/conditional swap. Two different writers can both observe the same old hash, both returnwritten, and the later rename silently overwrites the first. The promisedpersona-changedguarantee is not atomic. Serialize writers across the comparison and replacement (including confirmation), then test two concurrent updates.src/buddy/persona-apply.ts:309-316,346-365: whenNeedDaemonReload=yesanddaemon-reloadfails, execution still restarts the gateway and reports success. systemd can start with its previously cached drop-in; the new process timestamp then clears the derived pending condition, so a later apply need not repair the acting sender. Fail closed on reload failure and retain a retryable pending state.src/buddy/gateway-refresh.ts:334,372-388andsrc/buddy/persona-apply.ts:346-365: failedrepair-promptscommands are counted but still returnstate: refreshed; apply restarts and can reportapplied. I reproduced{"state":"refreshed","refreshed":0,"live":1}when the sole repair exits 1. After a successful restart, the timestamp pending marker disappears, so that conversation is never retried. The 200-row cap likewise omits older live conversations while claiming every live prompt was cleared. Preserve retryable failure/coverage and do not call the refresh complete in these cases.src/buddy/persona-apply.ts:322-331,352-395: the gatewayactive-work=idledoes not establish that the separatelazurio-t3-buddy.servicehas no active ACP turn. A running T3 unit can be stopped during such a turn. Gate the T3 stop on T3-specific quiescence/drain, not just gateway idleness.
The committed generator checks the installer SHA and invokes its extracted helper, and the committed golden comparisons pass, but the private installer itself is not in this public checkout, so I could not independently regenerate provenance. F64 correctly labels the socket owner-UID path and real gateway/SIGTERM behavior as not exercised; repair exit-code handling needs explicit failure tests and a narrower claim. File replacements are individually atomic with tested modes and final-component link handling, not a transaction across SOUL.md, the drop-in and T3 settings; partial failure must stay visibly retryable. The small active-work client overlapping #635 is acceptable as a bounded follow-up to consolidate after both land. No merge while main is frozen for v0.1.11.
…g, stored prompts stay owed until the store shows them cleared, and Buddy's T3 Code restarts only with no run going (DEV-6676) Pablo's review of #636 at cea70f4 found four failure paths; each is fixed and tested: - Writers serialized: writePersona and confirmPersona hold a kernel flock on ~/.config/lazurio/buddy/.persona.lock (acquireFileLock, which the kernel releases when its holder dies) from the read to the replace, so of two writers of one revision exactly one writes and the other answers persona-changed. Tested with two writers in one process, a writer holding the lock in another process, and a confirmation racing a write. - A failed daemon-reload fails closed: no restart on the units systemd still holds; NeedDaemonReload keeps the restart pending even when the gateway starts again by itself, and pending is judged on the unit as it stood before the reload, so a gateway that ran on older units is restarted after a reload that succeeds. - Stored prompts: only live conversations that hold a stored prompt are cleared, and the store decides, by the prompt hash Hermes keeps (sessions.system_prompt_hash), never by the command's exit alone; a failure, an answer that did nothing, an id never passed or more than 500 conversations leave the refresh incomplete and the apply failed. Because stored prompts have no time of their own and anyone's restart erases the files' evidence, an owed refresh is recorded in the Platform's own file (~/.local/state/lazurio/buddy/prompt-refresh-owed.json), written before SOUL.md changes and removed only after a complete refresh and the restart that follows it. - Buddy's T3 Code is stopped around its settings only when T3 Code itself has no run going: his own t3 (the unit's runtime) asks thread list for every thread in an unfinished state, subagents included, at v0.1.1-lazurio.2 and v0.1.2-lazurio.1 alike; the gateway's active-work no longer decides it. Also: one apply at a time (a flock in Buddy's state directory); checks in the order of installer 2026-10-25.1, which refuses a lone surrogate in every member, so that case is parity now; goldens produced from 2026-10-25.1, whose renders are byte-identical to 2026-10-24.2's. 67 guards, each removed or loosened, fail a test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…en left visible, writers serialized, a failed reload restarting nothing, an owed prompt refresh recorded, T3 Code's own runs gating its restart (DEV-6676) F64 now says what the code does after Pablo's review of #636: each derived file is replaced atomically on its own, with no transaction across SOUL.md, the drop-in and the T3 settings, and whatever did not happen stays visible and is retried; the Platform's writers of the persona hold one kernel lock; a reload systemd refuses restarts nothing and stays pending; a refresh of stored prompts is verified against Hermes's store and, since it cannot be derived, recorded while owed (with why); Buddy's T3 Code restarts only when T3 Code counts no unfinished run. It names installer 2026-10-25.1 as the byte authority (2026-10-24.2 renders the same bytes), says that the installer lives outside this repository so the provenance is its recorded sha256 with the committed goldens and generator, narrows the claim about repair exits, and lists the new alternatives, checks and limits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thank you for the review of cea70f4. All four findings are fixed on the same branch, at exact head 28d9ce5 (two commits on top: 1. Writers serialized (
2. A failed reload fails closed (
3. Repair failures and coverage stay retryable
4. T3 Code's own runs gate its restart (
Also
|
agentrozjedemeai
left a comment
There was a problem hiding this comment.
@immakermatty Re: #636 (comment) — reviewed exact head 28d9ce5 against the four findings on cea70f4.
- Concurrent writes: resolved.
writePersonaandconfirmPersonashare the kernel.persona.lockacross read, revision comparison and replace; in-process, cross-process and confirmation races are covered. - Failed daemon-reload: resolved. Pending is captured before reload; failure prevents restart, and
NeedDaemonReload=yesremains pending even after an external gateway restart. The failed-then-successful reload path is tested. - Repair failures/cap: resolved for the reviewed paths. The session store's prompt hash decides clearance; failed/unconfirmed/omitted work and the cap remain incomplete. The owed marker is written before SOUL.md changes and survives a partial refresh or external restart; it is settled only after complete refresh and the ensuing restart (or with no running gateway).
- T3 stop: resolved. Buddy's unit-pinned T3 runtime queries all five unfinished statuses, including subagents; only
total: 0permits stop, while no answer waits. The count-to-stop race remains the documented limit.
Blocking new concurrency finding (P2), src/buddy/persona-apply.ts:238-279,462: buddyPersona reads the persona and renders files before acquiring .persona-apply.lock, while writePersona uses a different lock. A write during that wait lets the apply return state: applied for the old persona after the new persona is already current; a subsequent read reports outdated. I reproduced this in the isolated checkout: hold the apply lock, start apply, write a new buddy name, release the lock; the temporary probe passed 1/1 and observed applied/old name followed by outdated/new name. The second apply can answer apply-running, so leaving completion to the first is not safe in this race. Re-read/build after acquiring the apply lock and guard against a persona revision change during application before returning applied; retry or expose pending work. Please add a regression for this overlap.
Local on this exact head: focused Buddy suites 54 pass, 0 fail; lint, typecheck and check:public pass. CI checks green. I did not run the full suite or a real gateway/T3 service. No merge while main is frozen for v0.1.11.
…wers only for the persona still current (DEV-6676) Pablo's review of #636 at 28d9ce5 (P2): buddyPersona read the persona and rendered its files before it took .persona-apply.lock, while writers take .persona.lock. An apply that waited for its turn while a new name was written answered `applied` for the old name (and could write the old files over newer ones), the next read said `outdated`, and the writer's own apply could already have given up as `apply-running`. Now the apply takes its turn first and reads, renders and applies under it. Before answering it reads the persona's revision again under the writers' lock, always the turn first and the writers' lock second (writers never take the turn, so the order cannot deadlock), and gives up its turn before that lock. A write that completes meanwhile is either seen at that check and applied in another round, or completes only after the turn is free, so its writer's next apply takes the turn and reads it: `apply-running` is safe again. After three rounds in which the persona changed each time it answers `waiting` (`persona-changed`), never `applied`. Tests: the overlap Pablo reproduced (an apply waiting for its turn while a new name is written, which now applies the new name once, with one restart); a name written while an apply applies; a persona that changes in every round; and a second apply answering `apply-running` while the first applies the name written meanwhile. The three new guards each fail a test when removed; 70 of 70 guards in all. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thank you for the review of 28d9ce5. P2 is fixed on the same branch, at exact head 1c37ff5 ( P2, an apply answering for a persona already replaced (
Checks:
|
agentrozjedemeai
left a comment
There was a problem hiding this comment.
@immakermatty APPROVED on exact head dff4c0c. The P2 overlap from my review of 28d9ce5 is resolved: the apply acquires its turn before reading/rendering the persona, and after each applied round compares the revision while holding the writers’ lock. A changed revision triggers another round; after three changed rounds it returns waiting/persona-changed rather than applied. The turn is released while the writers’ lock is still held, so a later writer cannot finish before the turn is free; writers take only the writers’ lock, preventing a lock-order cycle. The second release is idempotent in acquireFileLock.
I reproduced the overlap regression and the mid-apply, three-round and second-apply cases in the isolated exact-head checkout: 54 Buddy persona/chat tests passed, plus 220 adjacent Buddy tests; lint, typecheck and check:public passed. git diff --check is clean. Current-head CI scope, static checks, both test shards and ubuntu-24.04 are green; zero unresolved review threads. I did not run the entire local suite or a real gateway/T3 service. F64 records the untested release-order window and the bounded-retry limit. This is a review only: no merge, push or release while main is frozen for v0.1.11.
…er, and brings a change to his running conversations when he is idle (DEV-6676, F64) One source (persona.json) and one deriver (the Platform): src/buddy/persona.ts reads, checks and writes the persona under its writers' lock and renders SOUL.md and the gateway's acting drop-in byte for byte as installer 2026-10-24.2/2026-10-25.1 (golden tests from the installer's own renderer); src/buddy/persona-apply.ts and gateway-refresh.ts apply it in the apply's turn, re-check the revision before answering, clear the stored prompts of live conversations when Buddy is idle (an owed-refresh marker until complete) and restart his gateway once; T3 Code's settings follow the name, stopped only when T3 Code has no run. lazurio buddy persona [--apply] [--confirm]. Decision F64. Squashed from the reviewed #636 commits for the rebase onto main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dff4c0c to
1e2ba63
Compare
agentrozjedemeai
left a comment
There was a problem hiding this comment.
@immakermatty APPROVED on exact head 1e2ba63 against main 4a28b43. The cli.ts blob is byte-identical to the previously reviewed #635+#636 integration base 19623db. chat.ts code outside its provider comment is byte-identical to dff4c0c; its comment combines F64 naming with main’s #630 gateway-channel correction. decisions.md is current main followed by the byte-identical F64 section from dff4c0c. All seven other reviewed implementation/golden blobs are byte-identical; buddy-chat.test.ts differs only in the invalid-command cases already on main for #635. The 11-file PR scope is unchanged. Local Buddy tests: 104/104 across persona, chat, CLI, gateway, sign-in and Launchpad suites (plus 4/4 T3 runtime); git diff --check clean. Exact-head CI 40187974141: scope, static checks, both test shards and ubuntu-24.04 pass; macOS check intentionally skipped; zero unresolved review threads. No real gateway/T3 service tested here. Review only: no merge, push or release.
Why
Buddy's persona (his name, ty or vy, the user's name in three cases, an optional tone) lives in one file,
~/.config/lazurio/buddy/persona.json. Until now only the interim installer wrote it, once, and derived everything from it: SOUL.md, the acting sender of Buddy's gateway, Delegace's drop-in. The Platform only read the user's name for Buddy's T3 settings (F61). So a name changed after the install reached nothing, and Začínáme and Nastavení → Buddy (root decision 0210, addenda of 2026-10-20) had nothing to call.This is PR 2 of Buddy's onboarding (plan DEV-6676, the Buddy lead's shaping of 2026-10-25, §1.4, §2, §3.6 and §5). It makes the Platform the one deriver of the persona, with no UI yet. PR 3 adds the route
POST /api/buddy/personaand the screens.What changes
src/buddy/persona.ts: reads and validatespersona.jsonexactly as installer 2026-10-25.1'spersona_checkdoes, in its order. It renders SOUL.md (cs and en, ty and vy, a tone or Lazurio's line) and the gateway's acting drop-in byte for byte as the installer. Every write (writePersona,confirmPersona) holds one kernelflockfrom its read of the persona to its replace; it writes only over the revision the writer read, otherwise it answerspersona-changed(PR 3's 409). Of two writers of one revision, exactly one writes. Each derived file is replaced atomically on its own with its exact mode (0600, 0644) whatever the umask, never through a link. New optional memberconfirmed_at, which every current reader ignores.src/buddy/gateway-refresh.ts: how a change reaches running conversations. Hermes does not notice a changed SOUL.md.hermes sessions repair-prompts <id> --apply --jsonruns for every live conversation that holds a stored prompt. The store decides whether it worked, by the prompt hash Hermes keeps, not the command's exit.systemctl --user restart hermes-gateway.servicefollows, only once Hermes's ownactive-workverb says Buddy is idle, and never on units systemd failed to reread.ExecMainStartTimestampagainst the files' times, orNeedDaemonReload). An owed refresh of stored prompts cannot be derived, so it is recorded in the Platform's own file until a complete refresh and its restart.src/buddy/t3-runtime.ts:buddyT3Quiet, T3 Code's own count of unfinished runs. Buddy's ownt3runsthread list --status … --subagents --jsonagainst~/.t3-buddy.src/buddy/persona-apply.tsandlazurio buddy persona [--apply] [--confirm] [--folder] [--json]: read and report, or apply, one apply at a time. An apply reads the persona only once it holds its turn, and answers only for the persona still current: it checks the revision again under the writers' lock before answering, and goes again if the persona changed. What waits answerswaiting; what failed answersfailedwith its reason. Both stay visible and the next apply retries. Buddy's T3 settings follow; his T3 Code is stopped and started around them only when it has no run going (F61's rule).src/buddy/chat.ts: Buddy's T3 display name follows his name (amends F61 point 1). The persona is checked whole, by the same reader.docs/decisions.md: F64, with the alternatives, the hand-over (I-b) and the honest limits.Review of cea70f4 (Pablo), addressed at this head
Writers serialized. A kernel
flockon.persona.lockcovers the read, the comparison and the replace, for writes and confirmations alike. A dead holder's lock is released by the kernel. Tested: two writers in one process, a writer in another process, and a confirmation racing a write.A failed reload fails closed. No restart happens on old units, and
NeedDaemonReloadkeeps it pending even when systemd starts the gateway by itself. Pending is judged on the unit as it stood before the reload, so a gateway that ran on older units restarts after a reload that succeeds.Prompts stay owed until the store shows them cleared. Any of these makes the refresh
incompleteand the applyfailed, neverapplied:The owed record survives anyone's restart. Where a store cannot say which prompt a conversation holds, the commands' exits decide.
T3 Code's own runs gate its restart. The gateway's
active-workno longer decides it. T3 Code's answer, by Buddy's own runtime and home, does: any thread inpreparing,queued,starting,runningorwaiting, subagents included, at v0.1.1-lazurio.2 and v0.1.2-lazurio.1 alike. A T3 Code that cannot say counts as busy.Also from the review:
Review of 28d9ce5 (Pablo), addressed at this head
P2, an apply answering for a persona already replaced.
buddyPersonaread the persona and rendered its files before it took.persona-apply.lock, while writers take.persona.lock. An apply that waited while a new name was written answeredappliedfor the old name, and could write the old files over newer ones. The writer's own apply could meanwhile have given up asapply-running. Now:apply-runningis therefore safe again.waiting(persona-changed), neverapplied.Tests:
apply-runningwhile the first applies the name written meanwhile.What deliberately does not change
DELEGACE_*first) would keep a renamed persona's old names in the service's own texts while the drop-in exists.cmps the drop-in with its own render and would otherwise rewrite it and restart Buddy's gateway on every run. Both change their words in the one hand-over step (I-b).hermes-gateway.serviceitself stays Hermes's.Byte parity and provenance
tests/fixtures/buddy-persona/goldens.jsonis the installer's own output.f2413529…7d3c3e) plus the committed goldens. The committed generator (generate-goldens.py) checks that sha256, extracts the installer's helper, and runs its ownpersona-check,soul-render,acting-dropin-renderandpersona-buildinto real files. Whoever holds the installer can produce the goldens again; nobody needs it to run the tests.cmpare among them.NaN, which Python'sjsonreads.Verified
bun run lint,bun run typecheck,bun run check:public: pass.tests/buddy-persona.test.ts(43 tests, three more runs in a row without a failure) andtests/buddy-chat.test.ts(11) pass. Withtests/buddy-t3-runtime.test.ts,tests/folder-buddy.test.ts,tests/shell-buddy.test.tsandtests/launchpad-chat.test.ts, 274 tests pass.bun teston1c37ff5d(macOS): 4801 pass, 10 skip, 2 fail across 315 files (1072 s). The run overlapped another agent's full run and mutation run on the same machine. Both failures are known flakes under load, outside Buddy's code, and both files pass alone, twice: Flaky under load on macOS: two recover tests about the Recovery-mode issue body and launchpad-hosted's hostile HTTP_PROXY test #580 (launchpad-hosted, the hostile HTTP_PROXY test, at its 5 s limit) and Flaky under a loaded full run: application-environment "session runner …" refuses its fixture port as port-occupied #222 (application-environment, the session runner's fixture port). The previous head's clean run passed 4799 of 4799. CI on Linux at this head: static checks, both test shards and ubuntu-24.04 pass.launchpad-new-tab"reconciliation: a tab closed for everyone…" at its 3 s limit. That is the known flake Flaky after #385: the new-tab holder's reconciliation test sometimes never sees the hub's second list within 3 s #386, unrelated; every clean run since passed it.systemctl show --timestamp=us+utcwas checked on systemd 255.4 (Ubuntu 24.04, as Machines' guests run) in a disposable container:ExecMainStartTimestampto the microsecond in UTC with an English weekday under an empty environment, andNeedDaemonReload=yesafter a drop-in write.Risks and follow-ups
lazurio buddy persona --apply, readrestartand skip its own restart when it isdone, and move its drop-in verification with it.active-workquestion has its own small control-socket client. F63's gateway client (PR 1, Buddy: his ChatGPT sign-in and state through his own Hermes gateway, in the Launchpad and lazurio buddy (DEV-6676 PR 1, F63) #635) is the place for one client once both are on main; accepted in review as a follow-up.docs/decisions.mdwill conflict with PR 1 (F63 there, F64 here).PUT /api/profiles/<name>/soulwrites 0644. It keeps an existing file's mode and uses 0644 only for a new file. P3 stays rejected for its other reasons (F64's table).Plan DEV-6676 · worktree
DEV-6676-buddy-persona· branchagent/DEV-6676-buddy-persona· based onmain9781aed🤖 Generated with Claude Code