Skip to content

fix(dsh): normalize session IDs for portable storage - #5522

Open
ZaynJarvis wants to merge 1 commit into
mainfrom
fix/session-id-portable
Open

ZaynJarvis wants to merge 1 commit into
mainfrom
fix/session-id-portable

Conversation

@ZaynJarvis

Copy link
Copy Markdown
Collaborator

Summary

  • normalize Windows-invalid DSH session ID characters and trailing dots/spaces before using the ID as an OpenViking storage path component
  • append a deterministic 12-hex SHA-256 suffix only when normalization is needed, preventing collisions such as im:a and im?a
  • keep existing mappings unchanged for already-portable session IDs
  • document that previously created non-portable DSH sessions start a new normalized OpenViking session after upgrade

This is intentionally scoped to the DSH adapter. Changing the shared helper or server storage mapping would alter session identities for every integration and would require a broader migration plan.

The plugin version is bumped to 0.5.13 because open PRs #5254 and #5459 already use 0.5.12.

Fixes #5493

Tests

  • node --test session-id.test.mjs runtime.test.mjs (21 passed)
  • npm run check
  • .github/scripts/check-plugin-version-bumps.sh origin/main
  • git diff --check

npm test currently has one unrelated failure in index.test.mjs under the local Node 22.16.0 runtime; the same assertion fails on a clean origin/main worktree. The package declares Node 22.19.0 or newer.

@ZaynJarvis ZaynJarvis added the agent-plugins Agent harness and plugin integrations label Oct 1, 2026

@ktz03 ktz03 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-assisted first-pass review of 50b825ca, checked locally on Windows with Node 24.21.0 and the lockfile-pinned DSH rc.6 dependencies.

The runtime mapping looks consistent: stateFor() retains the native harness ID and sends the derived OV ID through session ensure, capture, recall, commit, and pending-queue matching. Already-portable IDs keep their previous mapping. The generated shared closure and dry-run npm package both include the new helper and its session-model.mjs import.

Local validation: the existing DSH suite passed 94 tests with one live-server recall test skipped; the relevant shared sync/recall/pending contracts passed 21 tests; syntax/version consistency checks and the generator clean-tree check passed. A separate bounded Windows filesystem probe created and read back directories/files for 11 synthetic IDs, including ordinary Unicode IDs, an IM-like colon-delimited ID, and trailing dot/space cases. A fake-client runtime probe also confirmed ensure/add/get/commit/dispose use the same mapped ID. These probes did not exercise a live DSH IM login or OpenViking server.

One release follow-up before merge: current main is now df32bf6e and already has DSH version 0.5.13, matching this PR. Running the repository's version check against that current base fails:

bash .github/scripts/check-plugin-version-bumps.sh df32bf6e50a40843438f9491a26069ca4bd08f1f
::error file=examples/dsh-memory-plugin/package.json::examples/dsh-memory-plugin changed but its version is still 0.5.13.

The same check passes against the older eadf2c7d base (0.5.11 -> 0.5.13); that earlier-baseline result does not cover current main. Could you refresh against current main, move package/config/lock together to a new version above the then-current main, and rerun the version check against that base? I did not find another runtime blocker in this pass.

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

agent-plugins Agent harness and plugin integrations

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

[Bug]: Windows 下 IM 会话 id 含 ":" 导致建会话失败(503 UNAVAILABLE / os error 123),并静默丢数据

2 participants