Repository navigation
Conversation
|
Thanks again, @meutex! Publishing the memory footer status once per session bind, instead of repeatedly, sounds like a nice cleanup. We've queued a review and will post the findings here. |
|
Thanks, @meutex. The review found two things to fix before merge, plus two small ones: 1. A second session loses its footer. 2. Fragment name: the fragment is Smaller:
|
The bind-time footer dedupe cache is shared by every session and keyed only on the fingerprint, so a fresh bind's syncActive must still drop the cache (without emitting a restore line) or a later session that binds to the same git state keeps no footer. The bind-time publish also moves out of the completion-drain try so a non-ENOENT readdir error no longer leaves the session without a footer. Fragment renamed to 12270.
|
Thanks, @meutex! The re-review at
omo's |
|
Thanks @code-yeongyu for the re-review! Glad the second-session footer issue and the drain-throw path are fixed. I appreciate you catching the two additional edge cases as well. I'll take a closer look at the deduplication behavior across concurrent sessions |
|
Non-binding review, read the full diff. The ownership change is clear and well pinned by tests: moving One behavior change worth confirming as intentional: previously a throw from |
|
Thanks for the thorough review! I traced the refresh path and confirmed that Git read failures are already caught inside Wrapping Appreciate you checking this! |
Summary
setStatus("memory", …)key: the initial-status pass started frombind()and the fingerprint-gated footer-live refresh after the completion drain. Whichever git read returned first decided whether one or two requests reached the engine UI stream, so the same input produced different extension UI request streams across runs.syncActivealso no longer emits a restore line for a segment that was never animating. The published text is byte-identical to the previousrefreshMemoryStatusoutput, so the footer looks the same; only the duplicate publish is gone.Changes
packages/omo-senpi/src/components/memory/wiring-reflection-live.ts:refreshInitialStatusnow callsrefreshMemoryStatuswithshowFooter: falseand reuses the injected git repo, so a bind produces exactly onesetStatus("memory", …)call regardless of git read timing.nowandcreateGitRepoare forwarded to the footer-live wiring so both paths can be driven through one repo.packages/omo-senpi/src/components/memory/status-live.ts:syncActivereturns early when nothing was animating (activeIdentity === undefined), so a fresh bind no longer publishes a spurious restore line. The restore path uses the captured identity/ui instead of falling back to the incoming values.packages/omo-senpi/src/components/memory/wiring-types.ts: adds the optionalcreateGitReposeam toMemoryWiringOptions.packages/omo-senpi/src/components/memory/wiring-reflection-live.test.ts: new describe block "bind-time memory footer publish ownership (Memory footer publishes its status a varying number of times at session bind (same input, different engine event stream) #12253)" with three co-located given/when/then cases: initial-status reads finish after the footer refresh, initial-status reads finish first, and unreadable head / unresolvable commit age. All assert the exact recordedsetStatuscalls.changes/unreleased/12253.md:### Fixedfragment describing the behaviour change and linking the pull request.QA & Evidence
wiring-reflection-live.test.tsdrive a bind through a gatedcreateGitRepo: one run holds the initial-statushead()read behind a promise while the footer-live refresh completes, the other lets it finish first. Both observe the recordedui.setStatuscalls for thememorykey. Thesenpi-qaskill was run with the evidence dir resolved viaresolve-evidence-dir.mjsto.omo/evidence/omo-senpi-adapter/20261103-memory-footer-single-publish(gitignored, not committed).Observed result: RED (before the change): the gated case failed because the
memorykey was published twice —expect(calls).toEqual([{ key: MEMORY_STATUS_KEY, text: expectedText }])received a second identical entry once the heldhead()read resolved. GREEN (after the change): the three #given/#when/#then cases under "bind-time memory footer publish ownership (Memory footer publishes its status a varying number of times at session bind (same input, different engine event stream) #12253)" pass with 0 failures, and the existing cases in the file still pass; the unreadable-head and unresolvable-age cases publish nothing.Artifact:
drive-self-test.txt,drive-live.json,README.mdunder the evidence dir above (sanitized: no tokens, auth headers or credentials).Why sufficient: The cases pin the exact UI request stream rather than the rendered text, which is what diverged between runs, and they cover both interleavings of the two git reads plus the no-data paths. The live artifact records the real engine run alongside the self-test.
Digests are recorded in the sanitized
README.mdof the evidence dir; the dir is gitignored so the artifacts are not attached to the branch.Risks & Residuals
lint:ratchetbaseline left unchanged: the changed files add no new findings and this change does not remove tracked debt.Screenshots
Not applicable: no visible UI change. The footer text is byte-identical to the previous output; only the duplicate publish is removed.
Automated Checks
bun run typecheck tsgo --noEmit -p packages/omo-senpi/tsconfig.json bun run test:senpi bun test bun run lint:ratchetAll pass locally on Bun 1.4.4 / Node 24.
Related Issues
Summary by cubic
Fixes the session-bind memory footer so each bind publishes exactly one footer status, and a later session binding to the same git state still gets its own footer.
try, so a drain failure no longer drops the footer.syncActiveclears the shared dedupe cache on a fresh bind without emitting a restore line, so a second session bound to identical git state still publishes.Written for commit e6e6135. Summary will update on new commits.