Skip to content

Publish the memory footer once per session bind - #12270

Open
meutex wants to merge 3 commits into
code-yeongyu:devfrom
meutex:meutex/1793702897-publish-the-memory-footer-status-once-per-session-bind
Open

meutex wants to merge 3 commits into
code-yeongyu:devfrom
meutex:meutex/1793702897-publish-the-memory-footer-status-once-per-session-bind

Conversation

@meutex

@meutex meutex commented Nov 3, 2026 •

Copy link
Copy Markdown

Summary

  • At session bind the memory footer had two independent publishers of the same setStatus("memory", …) key: the initial-status pass started from bind() 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.
  • This change gives the bind-time publish a single owner. The footer-live segment path publishes; the initial-status pass keeps only its advisory duties (pressure dream and notify) and never writes the footer. A fresh bind's syncActive also no longer emits a restore line for a segment that was never animating. The published text is byte-identical to the previous refreshMemoryStatus output, so the footer looks the same; only the duplicate publish is gone.

Changes

  • packages/omo-senpi/src/components/memory/wiring-reflection-live.ts: refreshInitialStatus now calls refreshMemoryStatus with showFooter: false and reuses the injected git repo, so a bind produces exactly one setStatus("memory", …) call regardless of git read timing. now and createGitRepo are forwarded to the footer-live wiring so both paths can be driven through one repo.
  • packages/omo-senpi/src/components/memory/status-live.ts: syncActive returns 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 optional createGitRepo seam to MemoryWiringOptions.
  • 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 recorded setStatus calls.
  • changes/unreleased/12253.md: ### Fixed fragment describing the behaviour change and linking the pull request.
  • No changes to the committed senpi plugin bundle or goldens.

QA & Evidence

  • What was tested: The new regression cases in wiring-reflection-live.test.ts drive a bind through a gated createGitRepo: one run holds the initial-status head() read behind a promise while the footer-live refresh completes, the other lets it finish first. Both observe the recorded ui.setStatus calls for the memory key. The senpi-qa skill was run with the evidence dir resolved via resolve-evidence-dir.mjs to .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 memory key was published twice — expect(calls).toEqual([{ key: MEMORY_STATUS_KEY, text: expectedText }]) received a second identical entry once the held head() 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.md under 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.
sha256sum .omo/evidence/omo-senpi-adapter/20261103-memory-footer-single-publish/drive-self-test.txt
sha256sum .omo/evidence/omo-senpi-adapter/20261103-memory-footer-single-publish/drive-live.json
sha256sum .omo/evidence/omo-senpi-adapter/20261103-memory-footer-single-publish/README.md

Digests are recorded in the sanitized README.md of the evidence dir; the dir is gitignored so the artifacts are not attached to the branch.

Risks & Residuals

  • The initial-status pass no longer publishes the footer, so if the footer-live path cannot resolve a head or commit age, the bind publishes no footer line at all. That is the intended deterministic behaviour and is covered by the third test.
  • The desktop equivalence cell E17 golden is not re-recorded here; this change only removes the nondeterminism that blocked it.
  • lint:ratchet baseline left unchanged: the changed files add no new findings and this change does not remove tracked debt.
  • CLA not signed by a human; it must be signed before the contribution is acceptable, and nothing in this pull request implies otherwise.
  • Fork-target PR: no golden or bundle regeneration, no dependency changes, no reformatting outside the touched files.

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:ratchet

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

  • The footer-live segment path owns the publish; the initial-status pass keeps only pressure dream and notify.
  • The publish runs after the completion-drain try, so a drain failure no longer drops the footer.
  • syncActive clears the shared dedupe cache on a fresh bind without emitting a restore line, so a second session bound to identical git state still publishes.
  • Adds regression tests for both git read interleavings, the unreadable-head / unresolvable-age paths, and the second-session case; published footer text stays byte-identical.

Written for commit e6e6135. Summary will update on new commits.

View guided diff

@code-yeongyu

Copy link
Copy Markdown
Owner

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.

@code-yeongyu

Copy link
Copy Markdown
Owner

Thanks, @meutex. The review found two things to fix before merge, plus two small ones:

1. A second session loses its footer. packages/omo-senpi/src/components/memory/status-live.ts:110 now returns early from setActive(false) when nothing was animating. Before, that path cleared lastFingerprint and republished on every bind, and it was the only reset besides clearStatus. One footerLive instance is shared by every session in the process, and the dedupe key at status-live.ts:85 doesn't include the session. So when a second session binds through the before_agent_start path (index.ts:137, which has no clearStatus, unlike session_start at index.ts:162), refresh at wiring-reflection-live.ts:156 sees the same fingerprint and publishes nothing. That can happen when a multi-session host restarts and resumes several sessions, and the session then has no footer until memory state changes. Suggested fix: invalidate the fingerprint on bind, or key the dedupe on the session as well as the fingerprint, and add a two-session test.

2. Fragment name: the fragment is changes/unreleased/12253.md, but this PR is #12270, and the changelog gate (script/changelog-gate-rules.ts:190) requires the fragment to be named for its PR. Please rename it to 12270.md.

Smaller:

  • In wiring-reflection-live.ts:155-158, the only bind-time footer publish now sits inside the completion-drain try, so a throw from consumePendingReflectionCompletions (for example a non-ENOENT readdir error) leaves the session with no footer. Moving the syncActive/refresh pair after the try/catch would restore the old behavior.
  • The fragment cites the fork PR meutex/oh-my-openagent#3 instead of Publish the memory footer once per session bind #12270, so the release changelog would link to the fork.

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

Copy link
Copy Markdown
Owner

Thanks, @meutex! The re-review at e6e6135495 is clear: the second-session footer is fixed, the fragment is renamed, and the drain-throw path still publishes. Two non-blocking notes for a follow-up:

  • status-live.ts:89 still keys the dedupe only on the git-state fingerprint, with no session or identity in it. So two live sessions on one identity can still go stale at settle time: when state changes, the first session's onSettled refresh (wiring-reflection-live.ts:172) publishes, and the second session's refresh sees an unchanged fingerprint and keeps its old line. That predates this PR and is outside its bind-time scope.
  • The drain-throw fix at wiring-reflection-live.ts:158-162 has no test that makes the completion drain throw and checks that the footer still publishes.

omo's dev is in a merge freeze for a release, so this merges in its slot after that.

@meutex

meutex commented Nov 3, 2026

Copy link
Copy Markdown
Author

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
Thanks again for the thorough review and for helping make this more robust!

@surlymochan

Copy link
Copy Markdown
Contributor

Non-binding review, read the full diff. The ownership change is clear and well pinned by tests: moving syncActive/refresh out of the drain try/catch means a bind no longer loses its footer to a drain failure, and the two ordering tests (gated git reads vs fast reads) plus the unreadable-head case cover exactly-one-publish well. The cross-session test for the shared lastFingerprint reset is the part I most wanted to see, and it is there.

One behavior change worth confirming as intentional: previously a throw from footerLive.refresh itself was caught by the same try and logged as a drain failure; now it propagates out of bind. If a refresh-time git read fails mid-bind, does bind now fail instead of degrading to no-footer? If that is intended, this is good to go; if not, wrapping just the refresh in its own try would preserve the old degradation while keeping the drain-failure fix.

@meutex

meutex commented Nov 4, 2026 •

Copy link
Copy Markdown
Author

Thanks for the thorough review! I traced the refresh path and confirmed that Git read failures are already caught inside recompute() in status-live.ts, unchanged from the default branch. The bind continues without a footer, and failed probes aren't cached, allowing subsequent retries.

Wrapping footerLive.refresh() in another try/catch would be redundant. No code change is needed for this case.

Appreciate you checking this!

This branch has not been deployed

No deployments
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.

3 participants