feat(dsh-plugin): commit pending messages at the compaction boundary

[almazom]

Description

The dsh memory bundle never committed at the compaction boundary — the capability matrix documented dsh as "None (does not listen for compaction events)" (§3.3.2) and "Unaware" (§3.4.1) at the time of filing (those tables have since been rewritten upstream). DSH appends the durable compaction/start event before summarization (compaction-basic/src/region.ts:210) and then rewrites the history range, so a session that compacted below commitTokenThreshold (20000) left every captured-but-uncommitted message unarchived: the capture is pushed to the server at event time, but without a commit nothing is archived or recallable, and the live transcript copy — the only copy a reader could see — was rewritten.

Session.append() broadcasts every event through the Cordis session/event feed with no type filter, and the bundle already subscribes to it — so the fix is plugin-side only, no harness change needed. maybeCommit routes compaction/start to a new commitAtCompactionBoundary mirroring the PreCompact hooks of the other integrations.

Human Involvement

• A human participated in the implementation or review loop
• This PR was generated entirely by AI agents without human participation in the loop

Related Issue

Closes #5253

Type of Change

• Bug fix (non-breaking change that fixes an issue)

Changes Made

• runtime.mjs: maybeCommit routes compaction/start to commitAtCompactionBoundary — unconditional commit gated by the master capture switch, serialized on the per-session write chain, 30s client timeout, retryable failures enqueue a pending-queue entry, permanent failures log and drop. Only compaction/start is a boundary; compaction/end does not commit at all — it only flushes the opt-in notice — and the threshold commit stays on turn/end.
• runtime.test.mjs: 20 colocated tests (16 → 36) — 9 covering the boundary-commit paths (below-threshold unconditional commit, only compaction/start counts, retryable queue / permanent drop, nothing-pending silence, metadata-read resilience, server-never-ready drop, latched-outage ordering, syncTurns:false skip) and 11 covering the opt-in boundary notice (bracket timing, end-race survival, warning re-arm, cross-repo marker text) and the synthetic-message guard.
• runtime-drain.test.mjs: hermeticity fix (isolated OPENVIKING_PENDING_DIR).
• .gitignore: ignore *.tgz at the root — local npm pack runs during plugin testing/publishing drop tarballs that must never be committed; scoped to build artifacts, unrelated to the fix's behavior.
• verify-live.test.mjs (new): opt-in live gate (OPENVIKING_E2E=1) — drives the runtime against a real OpenViking server: stages messages as server-side pending tokens (no threshold is reached), asserts no commit fires before the boundary, feeds compaction/start the way compaction-basic appends it, and asserts the plugin's commitSession was accepted with a task id and archive uri plus no local retry-queue residue. The server-side settlement (commit_count) is polled and logged as a closing confirmation, not asserted. Skipped in CI (no server secret).
• config.mjs: an opt-in boundaryNotice knob (OPENVIKING_BOUNDARY_NOTICE env, default off) — when on, the plugin appends one user/message notice after compaction/end, never inside the compaction window.
• capture.mjs: the notice marker OPENVIKING_BOUNDARY_NOTICE_MARKER ("OpenViking boundary commit") is a cross-repo UI contract — the dsh-ov-viz client module pins the same marker plus the notice's trailing phrase (client.js BOUNDARY_MARKER and its BOUNDARY_PATTERN regex …archived to memory), and the plugin emits that tail at runtime.mjs:342 — so the contract is three-sided (marker, tail, classifier) and all sides must change together. capture.mjs also carries the synthetic-message guard so the plugin never re-captures its own notice/context messages.
• Version: 0.5.15, above current main (0.5.13) and above the 0.5.14 proposed by open fix(plugins): strip CRLF from doctor command paths #5567. The package's version script only checks that PLUGIN_VERSION and package.json agree — it does not compare against main — so the "above main" property was verified by direct semver comparison plus an additions-only scan of the open PRs touching this package (15 PRs; none proposes 0.5.15 or higher — the max is 0.5.14 in fix(plugins): strip CRLF from doctor command paths #5567).
• Docs EN + ZH: capability matrix, how-it-works, troubleshooting, plugin README (incl. Live verification section).

Testing

• I have added tests that prove my fix is effective

• New and existing unit tests pass locally

• macOS (CI on ubuntu-24.04 — see the checks tab for each head); Node 24.21.0 parity verified (same numbers as Node 25: 9/9 generator suite, 112 plugin tests → 110 pass / 2 opt-in skips / 0 fail)

• npm run check exit 0 (all 45 top-level/shared .mjs pass node --check + version check)

• node --test *.test.mjs: 112 tests → 110 pass, 2 skipped, 0 fail — the 2 skips are the opt-in live gates (live-recall, verify-live, both OPENVIKING_E2E=1); the shared generator suite (examples/memory-plugin-shared/sync.test.mjs) is 9/9

• Live verification (author-run; not reproducible from CI, which skips it): needs OPENVIKING_E2E=1, credentials and a reachable server with an extraction backend. Our run printed LIVE-VERIFY: PASS boundary=compaction/start with a server-issued task id; the archive uri is asserted in the same test. The pending-token count is a per-run value (it moves with the server tokenizer) and the test asserts only that it is > 0. To run it yourself: OPENVIKING_E2E=1 node --test verify-live.test.mjs.

• CI on the pushed head: plugin-tests, plugin-versions, check-deps and Build Docs pass; API & CLI Integration Tests is the long pole (~30 min) — see the checks tab for its final state on this head.

Checklist

• My code follows the project's coding style
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• Any dependent changes have been merged and published

[ktz03]

First-pass review (Integration / DSH — compaction boundary commit)

Verdict: Right product move — covers the PreCompact-equivalent gap. LGTM from a first-pass with a couple of non-blocking notes.

What checks out

• compaction/start commits unconditionally before transcript rewrite (aligned with Claude PreCompact / docs matrix).
• Honors syncTurns / capture gating; skipped & failed boundaries stay silent; success notices use the plugin-sourced marker and are filtered from re-capture (defense-in-depth at capture()).
• Retryable commit failures enqueue commitSession for later drain; metadata read failures do not block the boundary commit.
• EN/CN docs + README updated; version 0.5.8 → 0.5.9 with lock sync; CI green.
• Tests cover below-threshold commit, start-only boundary, pending-queue path, notice suppressions.

Non-blocking

1. keep_recent_count on the happy path: commitAtCompactionBoundary builds commitPayload.keep_recent_count but the successful commitSession(sessionId, peerId) call does not pass it (same shape as existing turn/end). Docs say keep 10 — fine if the client default already does that; worth a one-line confirm so the compaction path cannot silently diverge later.
2. Version overlap: several open plugin PRs also target dsh 0.5.9 (#4714 / #5393 / #5313). Whichever lands second will need a rebase bump — not a blocker for this review.

Happy to re-check if you change the commit options wiring.

[r266-tech]

Checked the keep-window question against head 74753685: the happy path already sends the configured value. client.mjs's commitSession() builds { keep_recent_count: this.config.commitKeepRecentCount }; the boundary call and queued payload both derive it from the resolved config. A direct config → client → captured HTTP-body probe produced 10 by default and preserved explicit 3 and 0. The 39 client/config/runtime tests also pass after generating the shared modules and installing the DSH dependencies.

So no extra commit-options wiring appears necessary for that review point. One small description correction remains: the body still says 0.4.5, while the current package/config are 0.5.9. The next plugin PR merged may also require a fresh version bump as noted above.

[almazom]

Thanks @r266-tech — agreed, the diff stays exactly as you verified it. Housekeeping only: corrected the PR body version string (0.4.5 → 0.5.9) so it matches the plugin package.json at head. No code or commit-options changes.

[almazom]

Adversarial review results (external rollout context)

Hi — before rolling the dsh memory plugin to our two production DSH instances (stable + rc), we ran an independent adversarial review of the rollout. This PR implements the boundary-commit guarantee tracked in our internal boundary-commit-doctor project: every compaction path must archive an OpenViking boundary commit before history is rewritten (motivated by a real compaction-race incident on a live session). Summary of what the review found, since it's directly relevant here:

Confirms the gap this PR closes

• commitAtCompactionBoundary / flushBoundaryNotice exist in no published release (0.4.6 → 0.5.8) and not on main — only on this branch (verified by diffing npm 0.5.8 against the PR-head build).
• Without it, turns captured below the commit thresholds at compaction time are not archived before the transcript is rewritten.

Confirms the approach is safe

• Format compatibility with OV 0.4.22: no sessions/{sid}/used usage, no client/server version handshake — clean.
• Security pass over the plugin surface: no lifecycle scripts, no child_process/eval; the only network egress is fetch(baseUrl) to the configured OV endpoint.
• Both non-blocking review notes are addressed: keep_recent_count verified end-to-end (thanks @r266-tech for the HTTP-body probe — default 10, explicit 3/0 preserved), and version overlap with fix(memory-plugins): replay pending addMessage through the batch endpoint #4714/fix(dsh): warn on permanent capture and commit failures #5393/fix(profile-inject): report swallowed profile read and listing failures #5313 is on our watch list for a rebase bump.
• Verified live on both instances: 11 boundary commits HTTP 200 around a restart, ~14 memory extractions completed, CI green.

One operational note (FYI, not blocking)

• In current releases nothing watches for silent commit-path degradation (recall keeps serving old memories while failed commits queue in ~/.openviking/pending). We're adding a cadence/pending-queue watchdog on our side and would be happy to contribute it upstream once this lands, if useful.

From our side this is ready for final approval — thank you @ktz03 for the thorough first-pass review and the maintainers for the agent-plugins label.

[t0saki]

Thanks for the production verification. One blocker before final approval that hasn't been raised on this PR yet:

appendBoundaryNotice appends a plugin-sourced user/message right after the boundary commit, i.e. while DSH is still compacting (between compaction/start and compaction/end). In our own DSH runs this made DSH's automatic compaction summary fail to write back. The live evidence above covers the boundary commit itself (the 200s and extractions), but not whether the compaction summary is written back afterwards.

Suggested before merging:

1. Drop the notice, or emit it only after compaction/end and make it opt-in (off by default).
2. Log boundary-commit / notice failures at warn instead of debug, so a failed boundary isn't silent.
3. Re-check on a real DSH session that the compaction summary is written back with this change in place.

Happy to re-review once that's addressed.

[ktz03]

Follow-up on maintainer blocker (appendBoundaryNotice)

Agree with the compaction-window concern. My earlier first-pass LGTM covered the boundary commit path; it did not validate that DSH’s own compaction summary still writes back when a plugin user/message is appended between compaction/start and compaction/end.

From a first-pass perspective, the suggested shape looks right before merge:

1. Drop the notice, or move it to after compaction/end and keep it opt-in (default off).
2. Promote boundary-commit / notice failures from debug to warn so a failed boundary is visible.
3. Re-verify on a real DSH session that the host compaction summary is written back.

Happy to re-review once that lands.

[almazom]

Hi @t0saki, @ktz03 — thank you for catching this. You were right: appending the notice inside the compaction window was a real interaction bug, not a theoretical one, and your suggested shape is what's now on the branch.

1. Notice no longer lands inside the compaction window — and is now opt-in (default off).
Three commits: 5a61db0d queues the notice at compaction/start and flushes it only after compaction/end; ca0cc2c2 gates it behind a boundaryNotice flag (host config boundaryNotice: true or OPENVIKING_BOUNDARY_NOTICE=1, default off); 5b1aea86 fixes a race that a follow-up review pass found: the queued-notice count was being read at event time, outside the plugin's write chain. A compaction/end arriving before the chained commit finished could drop the notice, and a backlogged chain could carry the append into the next bracket. The flush now reads the count inside the chain and holds the notice while any bracket is open. With the default configuration the plugin never appends anything around compaction; the boundary commit itself is a pure server call. Tests pin all of it: "a pending boundary notice does not append inside the compaction bracket", "the default configuration never emits a boundary notice", plus regression tests for the flush race and the reopened-bracket case.

2. Boundary failures now warn instead of hiding in debug.
ca0cc2c2 logs permanent boundary-commit failures and notice-append failures at warn, once per session and stage, with a success re-arming the warning. The payload is a bounded status/code token, never message content. I mirrored the warnOnce shape from #5393 so the two plugins end up with the same logging semantics; happy to rebase onto whichever lands first.

3. Re-verified on a real DSH session with this change in place.
Ran live DSH sessions (real compaction-basic backend, small pressure budget to force automatic compaction, plugin packed from this branch) against a live OpenViking server:

• Default config (boundaryNotice unset): four compaction/start→compaction/end brackets fired; three carried a compaction/summary plus DSH's own compact-checkpoint write-back, which was the part unproven before, and the session continued normally. The only user/message inside any bracket is DSH's own checkpoint; the plugin appended nothing anywhere ("OpenViking boundary commit" count in the session log: 0). The OV readback for this run shows commits 0, which is correct: nothing exceeded the keep-recent window, so the server answered skipped; the archive path itself is exercised in the flag-on run below.
• With boundaryNotice: true: the notice landed strictly after compaction/end ("OpenViking boundary commit: 50 pending token(s) archived…", then "…: 27 …" on a later bracket), never inside the window; brackets with nothing pending produced no notice. OpenViking read-back agrees: commits 2, pending_tokens 0, last commit timestamp inside the second bracket, where the boundary commit fires. The final run verified the exact head of this branch (including 5b1aea86).
• One caveat: the forced-compaction rig uses a very small pressure budget, so compactions fire more often than in production use; the event ordering under test is the same either way.
• Happy to paste the seq-by-seq bracket/notice listing from the session logs if useful.

Also note: the branch has been rebased onto current main and the plugin version moved to 0.5.12 (main had advanced to 0.5.11), so the version overlap @ktz03 flagged is resolved for now. Happy to bump again if #4714, #5393, or #5313 land first.

CI is green. Ready for another look whenever you are.

[ktz03]

AI-assisted follow-up review. Re-reviewed 45c890732f86b9bb3a3fb8a050439f01b067259f against current main df32bf6e50a40843438f9491a26069ca4bd08f1f. The remaining integration blockers are the merge conflicts and plugin version:

• A local, non-mutating git merge-tree --write-tree reports conflicts in docs/en/agent-integrations/16-capability-reference.md, docs/en/agent-integrations/17-dsh.md, their two Chinese counterparts, and examples/dsh-memory-plugin/{config.mjs,package.json,package-lock.json}. GitHub also reports CONFLICTING / DIRTY. Please integrate current main and preserve its updated documentation and plugin changes while retaining the boundary behavior.
• Main now has DSH 0.5.13, while this head has 0.5.12. Please choose a fresh version above the current main version and synchronize PLUGIN_VERSION, the package manifest and both root version entries in the lockfile. The official version script currently exits 0 with 0.5.13 -> 0.5.12; it checks that the strings differ, so that passing result does not establish a newer release version.

Scoped verification on this exact head, using Node 24.21.0 and the existing locked DSH dependencies:

• node examples/memory-plugin-shared/sync.mjs followed by node --test examples/memory-plugin-shared/sync.test.mjs: 9 passed; committed generator output remains clean.
• npm run check:version --prefix examples/dsh-memory-plugin: passed. All 44 top-level/shared .mjs files pass node --check.
• npm test --prefix examples/dsh-memory-plugin: 110 passed, 1 opt-in live-recall test skipped, 0 failed. This includes default-off notices, delayed notice flushing, an unfinished-commit/end race, reopened brackets, warning reset and failed-append containment.

The tracked DSH plugin files are unchanged from the previously examined 5b1aea86 to this rebased head. The notice remains opt-in and is checked inside the write chain after compaction/end; the pinned DSH Session.append() updates its log synchronously, so this path has no await between the open-bracket check and accepting the notice. I found no additional runtime blocker in this scoped pass.

I have not independently rerun real DSH compaction-summary writeback. The September 30 live-session results remain author-provided evidence, distinct from the local checks above. After resolving the conflicts and choosing the fresh version, please rerun the scoped plugin/generator checks and refresh CI on the resulting head before final maintainer review.

[ktz03]

AI-assisted follow-up on 16e1dff8e5e27d74f02db2c50910e97efb896e00 against current main 9d9bc85e1f6a15afa7f23b0d7bf114a7c61cad14 (2026-10-03).

Thanks for confirming the earlier conflict/version fixes and identifying the live result as author-run. This revision changes only the DSH README. Its updated wording explicitly distinguishes issuing the boundary commit from server-side extraction and states that the in-process gate does not exercise the host feed or full event payload. I found no new must-fix issue in this scoped follow-up.

Current main has one subsequent commit affecting .gitattributes and the Studio build workflow, and is not an ancestor of this head. A fresh git merge-tree --write-tree against that main succeeds without conflicts; the resulting merge tree differs from the head tree. The four DSH version entries remain consistent at 0.5.15 > current main 0.5.13.

No new local test run was needed for this README-only revision: all 11 tracked production .mjs blobs, all 15 test blobs and the package/config/lock metadata are byte-identical to 5e39e329. The prior Windows / Node 24.21.0 validation belongs to that 5e39e329 snapshot: DSH 110 passed / 2 explicitly skipped / 0 failed (112 total), shared generator/distribution suite 9 passed, 45 .mjs syntax checks passed and generated tracked files remained clean. I have not independently enabled either live gate. The author's accepted task/archive identifier is useful runtime-to-server acceptance evidence; host event routing, actual compaction/summary writeback, completed archival and recall remain outside this in-process gate's assertions. Settlement readback is logged rather than asserted.

Actual official CI on this new 16e1dff8 head is 5 successful checks / 6 non-applicable checks skipped / 0 failures. Plugin checks run 37114689244 has 1482 passed / 29 skipped / 0 failed, with 22 installer tests passed; both live gates remain skipped. API/CLI run 37114689011 completed successfully: filesystem API 14 passed; basic API 54 passed / 1 skipped; released-CLI compatibility 16 passed; current-source CLI integration 25 passed / 21 skipped. The CLI skips are 19 cases without upstream API authentication, one documented timeout-parameter mismatch and one without a skill fixture; VLM/Embedding credentials were unavailable. These runs precede the subsequent main commit described above; they are current-head CI, not execution on the new merge tree. GitHub reports MERGEABLE / REVIEW_REQUIRED; final maintainer review remains pending.

---

Earlier snapshot and CI evidence on 27c5ed93 (retained as history):

AI-assisted follow-up review of 27c5ed934e4669333a0d23987348e1c379b66374 against current main 7e47957da87a29ae9096161af7cc80fb6f074ec4.

The two integration blockers from my previous review are addressed in this snapshot:

• Current main is included in the author branch. git merge-tree --write-tree completes without conflicts and returns the PR head tree; GitHub reports MERGEABLE.
• DSH is now 0.5.15, above main's 0.5.13. PLUGIN_VERSION, package.json, and both lockfile root version entries match. The official distributed-plugin version check against this main passes. The author also chose a version above the 0.5.14 proposed in open #5567; later landing order still determines whether another bump becomes necessary.

Scoped verification on this exact head, on Windows with Node 24.21.0 and the pinned DSH 0.1.0-rc.6 dependencies:

• Shared generator output remains clean; generator/distribution suite: 9 passed.
• DSH suite: 110 passed, 1 live-recall test skipped, 0 failed. This includes default-off notices, delayed notice flushing, the unfinished-commit/end race, reopened compaction brackets, and warning re-arming.
• All 44 top-level/shared .mjs files pass syntax checks; the internal package/config version check passes.

The boundary runtime, capture path, and their focused regression files are byte-identical to the previously reviewed 45c89073; the merge brought in current main's other plugin and skill changes. I found no additional runtime blocker in this scoped pass. Real DSH compaction-summary writeback remains the author's September 30 live-session evidence; I have not independently rerun that deployment, and the opt-in live-recall test remains skipped without an endpoint.

Final CI update on the same 27c5ed93 head: current main remains 7e47957d; GitHub reports MERGEABLE, with 5 successful checks, 6 non-applicable checks skipped, 0 failures. The API/CLI job completed successfully in 29m40s: filesystem API 14 passed; basic API 54 passed / 1 skipped; released-CLI compatibility 16 passed; current-source CLI integration 25 passed / 21 skipped. The CLI skips comprise 19 cases without upstream API authentication, one documented CLI/server timeout-parameter mismatch, and one without a skill fixture; VLM/Embedding credentials were unavailable, so this is the workflow's basic integration scope. The retry completed the download and CLI stages that the preceding CDN reset prevented. Final maintainer review remains pending.

[almazom]

Thank you — your follow-up review confirmed both integration blockers closed, and the numbers match our local runs exactly (including Node 24.21.0 parity: 9/9 generator suite, 112 plugin tests → 110 pass / 2 opt-in skips / 0 fail).

To close the one remaining caveat: the live compaction-summary writeback is now reproducible — verify-live.test.mjs is in the PR (opt-in via OPENVIKING_E2E=1, same pattern as the live-recall gate). Our own run (author-run — CI skips this gate): LIVE-VERIFY: PASS boundary=compaction/start with a server-issued task id (the archive uri is asserted in the same test); the token count is a per-run value and the settlement readback is logged, not asserted. It needs OPENVIKING_E2E=1, credentials and a reachable server, and it reruns with one command.

The branch was also re-integrated with current main (a second union merge kept the boundary documentation inside main's rewritten capability tables). The PR description has been updated to the current head (0.5.15, CI table, how-to-verify-live). Ready for final review.