Repository navigation
Promote staging → main (WALM-617) - #916
Merged
Merged
Conversation
The style-guide audit flags "request Security review when it is required" as passive voice and blocks merge on it. The earlier style-nit commit changed the H1s and em dashes but missed this line.
…COMG-718) The completion artifact is the durable operator record for a security migration, so the target package it names has to be the one finalize-tx actually used. manifestSha256 was already validated, but packageId accepted any non-empty string: "not-a-package-id-at-all" was written without complaint, and a valid-but-unnormalized id (uppercase hex, or no 0x prefix) was recorded in a form that no longer string-matches what build-finalize-tx.ts used, since assertObjectId() normalizes via normalizeSuiAddress(). Mirror isValidSuiObjectId() + normalizeSuiAddress() instead. The rules are reimplemented rather than imported because the CI job runs this script under bare node with no install step; verified equivalent to @mysten/sui 2.20.3 across prefix, case, length and non-hex cases. Writing also used a plain writeFileSync, so re-running against the same --out silently destroyed the previous ceremony's record. Open with "wx" and exit 1 on EEXIST; --force is the deliberate replace, flag-only with no env equivalent so a stray exported variable cannot enable it. Document both, and state plainly that the approver field is a procedure the ceremony follows rather than a control the tooling enforces. Self-test covers the rejected ids, the normalization, the refused overwrite leaving the file byte-identical, and --force replacing it.
…(COMG-718) A misspelled value flag was ignored and the field fell back to the env var of the same name, so `--aprover user:alice` with APPROVER=ci-bot wrote ci-bot and exited 0. An artifact that silently disagrees with the command the operator ran is not evidence of anything, so reject any flag outside the known set. Also reject a directory `--out` up front: "wx" reports EEXIST for one, so it hit the overwrite branch and told the operator to pass --force, which then fails with EISDIR. Drop the unused third element of REQUIRED, and the docs pointer to a PR template this repo does not have. Self-test covers the unknown flag and the directory --out.
…n-completion-artifact docs(ops): migration completion artifact schema (COMG-718)
…WALM-611) (#886) * fix(chatbot): stop guest-auth redirect loop on Railway bind address (WALM-611) * refactor(chatbot): slim public request URL helper (WALM-611)
…-719) (#845) * fix(server): report restore decrypt failures instead of skipped (COMG-719) Permanent decrypt/UTF-8 failures were chained into the restore skip set, so they inflated skipped and were invisible to callers. Count them as failed, keep skipped as the local success index only, and expose the additive field on RestoreResponse and both SDKs. * docs: onchain spelling and might in restore failed copy (COMG-719) * fix(server): bound restore failed to the page and signal retry on embed-down (WALM-480) Intersect failed with the on-chain page so it cannot exceed total. When an inspected page yields only transients (download/decrypt/embed), set truncated so the caller retries. Do not negative-cache embed failures. * fix(mcp): print restore failed and retry transients instead of raising limit (WALM-480) Show failed in memwal_restore output. When truncated is a download/embed blip, hint retry at the same limit; keep raise-limit for WALM-431 page/cap. * refactor(restore): slim fail-class helpers and MCP restore copy (WALM-480) Keep classification on decrypt/UTF-8 only. Tool descriptions mention retry vs raise-limit; the exact transient-page predicate stays in formatRestoreResult. SKILL RestoreResult.failed matches the required TS field. * chore: dump sdk 0.1.7 / python 0.1.10 / mcp 0.0.13 for restore failed (WALM-480)
…442) (#827) * fix(sdk): use typed tx.pure helpers in account and manual PTBs (WALM-442) * chore(sdk): dump 0.1.7 for typed tx.pure PTBs (WALM-442)
…d (WALM-612) (#889) * fix(relayer): alert and write_ready when Postgres storage is exhausted (WALM-612) * fix(relayer): alert on remember job insert and probe Neon cluster size (WALM-612) * refactor(relayer): drop unused WALM-612 size fields and probe helpers * fix(relayer): alert insert_vector storage exhaustion and warn when neon probe no-ops (WALM-612) * fix(relayer): fire postgres storage alert from insert_vector (WALM-612) * refactor(relayer): slim WALM-612 postgres storage alert paths Keep insert_vector Slack, public.pg_cluster_size warn-fail-open, OnceCell cap cache, alerts.rs helper, and sqlx SQLSTATE 53100. Drop the quota-reservation duplicate, leftover remember_jobs match rewrites, the sqlx alert wrapper, and bench insert_vector_plaintext hook. * fix(relayer): address WALM-612 review round 2 (probe fallback, timeout warn, enqueue alert) Fall back to sum(pg_database_size) when public.pg_cluster_size is missing, log probe timeouts at warn with a 1s budget, restore remember_jobs INSERT and enqueue Slack hooks, and send insert_vector alerts after the tx drops. * refactor(relayer): slim WALM-612 round-2 extra helpers Keep probe fallback, 1s timeout warn, remember_jobs/enqueue Slack, and insert_vector drop(tx). Route enqueue through the sqlx 53100 helper; drop the string wrapper, wrapped-message tests, and message-only 42883 fallback.
…895) Closes WALM-136 (GH #322). Also settles WALM-599. sha256hex fell back to `await import("crypto")` when WebCrypto was missing. Vite replaces that with a stub: the app builds clean, then throws in the browser the first time the path runs. It sat on the signed-request path, so every remember and recall reached it, and it was the last bare Node-builtin import in the SDK. The fallback could never have helped a browser — `crypto.subtle` is absent precisely when the page is not a secure context, where `node:crypto` is absent too — so it only served Node <19 while being the sole source of the exposure. Hashes with @noble/hashes instead, already a dependency here and in @mysten/sui. Reproduced under Vite 5.4.21 end to end: before, the build succeeds, emits a __vite-browser-external stub, and throws TypeError: crypto.createHash is not a function; after, no stub and a correct digest. Adds a dist scan guarding the whole class, and the SDK's missing engines.node >= 20.0.0, which is what left WALM-599 ambiguous.
…layer in health (WALM-390) (#876) * fix(mcp): warn on unrecognised flags, document env presets, report relayer in health (WALM-390) Unknown CLI flags fell through parseArgs' default branch and vanished, so a typo'd --namesapce silently wrote to the default namespace. parseArgs now collects what it did not match and main() names each one on stderr — warning rather than exiting, so a flag from a newer config cannot brick the server. An unknown flag consumes a following non-flag token as its presumed value, so --namesapce work warns once about the flag rather than twice, the second naming the user's data; that also keeps a mistyped secret out of the logs. --help gained a "Network presets" section rendered from ENV_PRESETS rather than retyped, so a new preset cannot ship undocumented the way --prod did. Also corrects the --label default, which help gave as "Walrus Memory MCP" against "MCP Client" in code. memwal_health now names the relayer that answered, threaded onto MemWalSession from the URL resolveAuth already receives. MemWal.serverUrl is private and the server imports the published SDK, so no SDK change was needed. * fix(mcp): report a relayer that names a network, and stop unknown flags eating commands Review follow-ups on WALM-390. memwal_health printed `session.relayerUrl`, which is the address the sidecar DIALS, not a network identity. The Rust parent fills it with `http://127.0.0.1:$PORT` whenever an operator did not override it, so on any self-hosted or local deployment health reported a loopback address as the network — the exact "healthy on the wrong network" failure the field was added to catch. Hosted OAuth deployments happened to be correct only because their issuer must already be public. Split the two meanings apart. `relayerUrl` stays the dial address; a new `publicRelayerUrl` carries an origin only when an operator supplied one, and health reports nothing when it is absent. The stdio package fills the gap for everyone else: the bridge always knows the URL it connected to — it is what `--prod` / `--relayer` / MEMWAL_SERVER_URL selected — so it annotates the health reply on the way through. The new sidecar test builds its session through `resolveAuth` rather than injecting one, so the loopback case is actually covered. Also from review: - An unknown flag no longer swallows `login`. `memwal-mcp --typo login` used to consume the command as the typo's value and never log in. - `--tokenn=hunter2` now records `--tokenn` only. The whole token used to reach the stderr warning, which is the one place a mistyped secret must not land. - Help said an explicit --relayer "overrides the preset it follows". Preset application is `??=`, so it wins from either side; the wording promised an order dependence the parser does not have. - Dropped comments that narrated the ticket rather than a constraint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HsS2mBzMfpzy3QE8EMiKvS * docs(reference): drop the em dash from the relayer-origin note Style-guide audit: no em dashes in prose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HsS2mBzMfpzy3QE8EMiKvS * docs(mcp): changelog entries for the WALM-390 fixes The PR changed packages/mcp/src but shipped no changelog entry, so the unrecognised-flag warning, the documented network presets and the relayer in memwal_health would have landed in 0.0.13 undocumented. Entries go in the existing unreleased 0.0.13 section — no version bump is involved — and are mirrored into docs/mcp/changelog.mdx along with that release's summary line and the frontmatter answer, as every other packages/mcp change does. --------- Co-authored-by: Le Tien Phat <91601109+Niko1444@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
… on-chain read (WALM-618) Every authenticated path re-read the account object from Sui on every request. `auth.rs::resolve_account` verified on each signed API call -- the Postgres `delegate_key_cache` only saves the registry *scan*, never the `GetObject` -- and `mcp_proxy.rs` verified on every MCP request: the SSE handshake and each JSON-RPC envelope alike. One `memwal_remember` through MCP is therefore ~10 fullnode reads: SSE open, initialize, tools/list, tools/call, `POST /api/remember`, and one per status poll. Against the public fullnode that load throttles into `RpcError`, which is correctly a 503 -- and a 503 makes clients reconnect and poll again, issuing more verifies. Measured on prod: ~200 uncached verify calls/min from reconnecting bridges, degrading every other user on the instance. Cache the positive result for 30s, keyed by (account object id, public key), shared by both paths. A burst of requests carrying the same credentials now costs one `GetObject` instead of one each. Only successes are cached, so adding a delegate key (the tail of `login`) takes effect immediately rather than after a TTL. A definitive rejection also evicts the pair, so an observed revoke cannot be outlived by a positive still inside its window; an unavailable RPC proves nothing about the key and leaves the entry alone. The trade this makes explicit: 30s is now the upper bound on revocation latency at the relayer. That is the same staleness the `/agents` listing already accepts (`DELEGATE_KEYS_CACHE_TTL`). The map is swept on the existing 5-minute `delegate_keys_cache` task so it stays bounded to recently-active pairs.
#809) * fix(mcp): confirm a completed sign-in, and keep serving stdin after it The sign-in flow reported failure twice — a notification and a notice on the next tool call — but reported success nowhere except the log file. A user who approved in the browser had no way to tell whether credentials landed, whether the bridge adopted them, or whether a retry would work. Writing a test for that confirmation surfaced a worse bug behind it. The auth-required stub hands off to the bridge by detaching its listeners and calling process.stdin.pause(), and a stream paused that way stays paused however many `data` listeners attach afterwards. The bridge therefore served only the request replayed from the hand-off and then read nothing further, so every call after the first hung unanswered. The sign-in had worked; the connection was deaf from the second call on. - resume stdin in the bridge's reader, with a regression test that signs in mid-session and then makes two calls rather than one - queue a one-shot banner on credential adoption, consumed by the first tools/call result so it states the account, delegate, and resolved credentials path exactly once and never repeats - send the notifications/message twin of the existing failure warning - move the sign-in copy into messages.ts so the signed-out stub and the signed-in bridge stop drifting apart, and state the resolved credentials path instead of assuming the global one * fix(mcp): confirm a completed sign-in, and keep serving stdin after it * fix(mcp): read `signedIn` from disk, and cover the re-login confirmation Review follow-ups on WALM-394. `handleLocalLogin` hardcoded `signedIn: true` on the assumption that the bridge only runs while credentials exist. `memwal_logout` deletes them, and login is intercepted before the signed-out guard, so a login after a logout in the same session told the user they were already signed in and that a stored delegate key would be replaced — twice wrong, and it reads as though the logout did not take. `handleLoginToolCall` had the opposite hardcode: a second `memwal_login` after a completed callback but before the hand-off skipped the replacement warning even though `saveCreds` would overwrite the file. Both now pass `signedIn: loadCreds() !== null`. The flag's contract is that credentials already exist, which is a fact about the disk, not about which mode answered the call. The success tests only ever spawned signed out, so the re-login pair (`handleLocalLogin` + `adoptCredentials`) was unexercised — dropping either its notification or its banner queue would have passed the whole suite. The logout-then-login test now pins the post-logout prompt, and a new test starts already signed in and asserts both surfaces plus the one-shot. Also from review: - `loginFailureNotice` moved into `messages.ts` beside the success copy, as a pure function of the reason. The `{@link}` pointing at it did not resolve while it was private to `auth-required.ts`, and one module for both voices is what that module says it is for. - Reattached the `runBridge` JSDoc. Inserting the `pendingLoginSuccess` docblock after it left two consecutive blocks, so the first documented nothing and `runBridge` lost its docs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HsS2mBzMfpzy3QE8EMiKvS * docs(mcp): drop em dashes from the changelog entries Style-guide audit: no em dashes in prose. Comma where the clause continues, parentheses for the aside. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HsS2mBzMfpzy3QE8EMiKvS * docs(mcp): move the WALM-394 entries to the unreleased 0.0.13 The four #633 bullets sat under ## 0.0.12, which is published on npm, so merging would have rewritten a released section. dev is unreleased 0.0.13, which is where they belong. Mirrored into docs/mcp/changelog.mdx, which had none of them, along with that release's intro line and the frontmatter answer. package.json stays at 0.0.13 — nothing here warrants a bump. --------- Co-authored-by: Le Tien Phat <91601109+Niko1444@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The sweep carried a second, longer threshold (600s) copied from the `/agents` cache next door. An entry past the 30s TTL can never be served again -- `is_fresh` is the same predicate the lookup uses -- so the only effect of the extra constant was to hold dead entries in memory for 20x longer than they were useful. Sweep on the TTL and drop the constant. Also states in the type's own docs why the map cannot be grown by a caller: only successes are recorded, so every entry corresponds to a delegate key really registered on an account. That matters because the MCP proxy takes `x-memwal-account-id` from an unauthenticated header, checks only that it is non-empty, and `/api/mcp/*` has no rate limit ahead of the verify -- recording rejections would have let an anonymous caller mint one entry per made-up account id. A test pins it.
`/api/mcp/sse` on production has served 784,627 401s against 50,958 200s. Every one of them exits `legacy_delegate_registered` through the same arm, whose only trace was `debug!` — dropped by the default `memwal_server=info` filter. The route also records nothing in `memwal_errors_total`, so 74% of the endpoint's failures (401 as a share of all non-200 SSE responses) are invisible in both logs and metrics. Raise that arm to `warn!` and attach `account_id`, so a rejected key can be traced to the account that presented it. The sibling `Unavailable` arm already logs at `warn!`; this makes the two failure modes symmetric. Nothing else changes: the response is still 401 with no detail to the caller, and the key itself is never logged.
The 30s verify TTL only helps a caller who repeats inside 30s. MCP clients do not: a tool call every few minutes misses the cache every time, so while the public fullnode throttles, a key that verified cleanly minutes ago collects a 503 per attempt. Production shows 155,874 × 503 against 50,958 × 200 on `/api/mcp/sse`, and six consecutive handshakes with a valid registered key all failing. `VerifyCacheMissAction::Keep` already survived that case, but nothing could ever read the entry it kept — the lookup filters on `is_fresh()`, so by the time `Keep` ran the entry was unservable, and the sweeper deleted it at the TTL besides. Keeping it was a no-op. Give it a reader. On an unavailable error only, serve a cached success within TTL + `DELEGATE_VERIFY_STALE_GRACE` (10 min) and log it at warn. The sweeper now retains on the same bound so the entry still exists. Revocation latency is unchanged at 30s whenever the chain answers. It stretches to 10 minutes only while the chain cannot be read — a window in which the relayer could not have observed a revoke anyway. A definitive rejection still returns and still evicts.
Caching only successes left the larger half of the traffic uncached. Production `/api/mcp/sse` answers 784,627 × 401 against 50,958 × 200, and `GetObject` volume (1,147,414) tracks total requests (1,119,744) almost 1:1 — so ~70% of the load throttling the public fullnode comes from requests that were never going to be accepted. That throttle is what produces the 503s everyone else sees, including callers whose credentials are perfectly valid. Those are not 784,627 people. A bridge holding a key the relayer will never accept treats the 401 as a transient connect failure and retries on a backoff capped at 15s, forever. Remember a definitive rejection for 10s and answer from it. The client half of the loop is #894; this half needs no client change, so it also covers the versions already installed on users' machines. Bounded deliberately, because unlike the positive cache this map is keyed by what callers send rather than by what exists on chain: - 10s, an order of magnitude under the 30s success TTL. A stale positive authenticates a revoked key; a stale negative only delays one that just became valid. - The positive lookup runs first, so a key registered after being refused works on its next successful verify rather than waiting out the TTL. - A successful verify drops any rejection for the pair. - Hard cap of 4,096 entries. At the cap a new pair is not recorded and falls through to the live read — today's behaviour — rather than trading a throttle for unbounded memory. The sweeper expires entries on the same TTL so ordinary churn never reaches the cap. An ordinary `memwal_login` is unaffected: it registers a freshly generated key, so the pair has never been rejected and has no entry.
`openSseStream` read the relayer's `retry-after` header and then threw it away: it was only interpolated into the Error *message string*, so nothing machine-readable reached the retry loops. Both `connectInBackground` and `reconnect` fell back to the generic geometric backoff, meaning the first retry after a 429 landed ~500ms later — well inside the window the relayer had just asked for. For `ip_active_cap`, which sends no `retry-after` at all because it is a CONCURRENT cap that only clears when another session closes, a sub-second retry is pure noise against a condition it cannot affect. What changed: - The 429 branch now throws a typed `RelayerThrottledError` carrying a resolved `retryAfterMs`. `Retry-After` is parsed defensively (delta-seconds or HTTP-date; unparseable values yield null rather than a NaN that would poison the backoff arithmetic), clamped to `MAX_THROTTLE_WAIT_MS` so a misconfigured or hostile header cannot park the bridge for hours, and falls back to `DEFAULT_THROTTLE_FLOOR_MS` (5s, overridable via `MEMWAL_MCP_THROTTLE_FLOOR_MS`) when the header is absent. - Both retry loops floor their backoff at the recorded throttle deadline. The deadline is held in `throttledUntilMs` so it survives across the separate `reconnect()` calls that would otherwise each restart from 500ms, and is cleared once a session actually opens. The `immediate` reconnect path (a login credential swap) still bypasses everything, so re-login does not gain a multi-second stall. - One `note()` per throttle episode says this is a rate limit rather than a bad config or bad credentials, with the next attempt's ETA. Previously the 429 went only to a JSON stderr log line, so a throttled bridge was indistinguishable from a broken one. - The non-OK handshake exits keep draining the body and still do NOT abort the socket. 45b0ad8 removed those aborts deliberately because that is the path that fired the Windows libuv assertion in this same ticket; the comment above them now records why, so they are not reintroduced as "hardening" a third time. The only behavioural change on those paths is `.catch()` around the drain so a truncated error body cannot mask the real status with a read failure. Test: packages/mcp/test/sse-handshake-429.test.mjs spawns the bridge against a mock relayer that 429s the SSE GET and timestamps every attempt. It pins both the header-honoured case and the header-less floor, plus no fatal exit, one local `initialize` reply, recovery of a call buffered during the throttle, and the throttle-vs-misconfiguration wording. Verified failing on the pre-fix code (attempt gaps of 504ms/1002ms) and passing after. Changelog and the environment-variable reference are updated for the new behaviour and for `MEMWAL_MCP_THROTTLE_FLOOR_MS`. Not addressed here (server side, needs its own change): what the cap is actually keyed on and the release of phantom connections. The limiter keys solely on IP (`acquire(ip)` is its only signature), so the reporter's inference that it tracks the account or delegate key is wrong — but a deployment whose `TRUSTED_PROXY_HOPS` is left at 0 behind an ingress collapses every user onto the ingress IP, which would explain a second public IP hitting the same cap.
… (WALM-618)
A tool call that arrives before the relayer session exists waits in
`pendingForward`. The orphan sweeper did bound that wait, but it explained
every expiry the same way:
"Walrus Memory did not answer this call. The connection to the relayer
dropped before the result came back. Please retry."
For a buffered call that is wrong twice over. Nothing was sent, so no
connection dropped and no reply was lost; and "before the result came
back" implies a write that may or may not have landed, when in fact none
was attempted. The one thing the bridge did know -- that the MCP
handshake had been failing, and with what error -- it kept to its own
log. That is the user-visible half of WALM-618: 15 session opens for 6
calls that ever reached the relayer, and from the user's side a
`memwal_remember` that was simply slow for minutes.
Track the last handshake failure and how long the run of failures has
lasted, cleared on every successful connect. An expiry now distinguishes
the two cases and, for a call that never left this process, answers with
what it was actually stuck behind:
"Walrus Memory could not reach the relayer - the MCP connection has
been failing for 251s, so this call never ran and nothing was stored.
Last handshake error: ... Check the relayer, or run `memwal-mcp login`
if the delegate key was revoked, then retry."
Answering a buffered request also means removing it from the buffer.
Left in, the flush after the next successful connect would run the call
we just reported as never having run -- with the agent, having been told
nothing was stored, likely to have retried by then. `initialize` is
exempt, as everywhere else: it is answered locally and buffered only so
the session can still negotiate capabilities.
This stays a deadline, not a per-attempt failure. A call the next attempt
could serve is still not failed early, which is what `coldstart-timeout`
locks down and what keeps the auth-required hot-handoff request alive.
The new test drives the real shape: a relayer that 503s every handshake
the way the relayer does when the on-chain delegate verify cannot reach a
throttled fullnode, then recovers -- proving the expired call is answered
once, never reaches the relayer afterwards, and that a call sent after
recovery does.
…view) Two review findings on the previous commit. **The wait itself was unchanged.** Rewording the expiry left the deadline at `callTimeoutMs` (240s by default), so a user whose handshake is failing still sat through ~4 minutes of silence -- which, plus one agent-level retry, is exactly the "3-8 min" the ticket reports. The wording was better; the symptom was not fixed. A request that is still buffered while no working connection has existed for `stalledHandshakeMs` (90s, `MEMWAL_MCP_STALLED_HANDSHAKE_MS`) is now answered on that shorter deadline. The two cases carry different risk, which is the whole reason they can have different deadlines: a request that was SENT might have executed, so failing it early invites the agent to retry a `remember` that already landed. A request that was never sent cannot have executed -- answering it is provably a no-op and the retry costs one round trip. 90s is past a relayer cold start and past six reconnect attempts at the capped 15s backoff, so it does not fire on a slow-but-recovering relayer. The login flow is unaffected: the browser wait happens in `runAuthRequiredServer`, before `runBridge` is called with the handed-off lines, so no request is buffered across it. **The failing-connection wording could describe an outage that was not happening.** Post-connect, `handleClientLine` also buffers behind an in-progress flush -- on a healthy session, with `handshakeFailingSince` cleared. Such a request took the same branch and was told the connection "has been failing for over 240s". There are three cases, not two, and the third now says what is actually true: the call was still queued when it timed out, and nothing was stored. That case also keeps the full deadline, since nothing is failing. The test now pins the short deadline specifically -- a 60s call timeout against a 3s stalled-handshake bound, so an answer near 3s proves the new bound fired and not the ordinary timeout.
The 429 backoff half of this branch shipped a changelog entry and an environment-variables row; the stalled-handshake half shipped neither, so `MEMWAL_MCP_STALLED_HANDSHAKE_MS` existed only in the source. Both changes are user-visible, so both are now written down.
WALM-618 took two days to attribute because the relayer's own telemetry
could not show it. There is no slow request: a 3-8 minute wait is N
handshakes of 10-30ms each, separated by client-side backoff the server
never observes. Three specific gaps made it unfindable.
**Nothing tied the attempts together.** The per-request id is minted
fresh on each request, so forty retries were forty unrelated log lines.
The bridge now generates one id per connect episode and sends it as
`x-memwal-connect-id` on every attempt including retries, so one grep
returns the whole episode and the span between its first and last line
is the wait the user felt. The relayer tracks the episode and records
`memwal_mcp_time_to_session_seconds` on the attempt that succeeds — the
number no per-request histogram can produce.
**Four of the five ways to be refused logged nothing.** Only the
on-chain rejection said anything, and none of them touched a metric, so
401 — 70% of this route's traffic — was absent from logs and dashboards
alike. Every refusal now goes through one path that logs the account,
the connect id, the client, and a stable reason code, and increments
`memwal_mcp_handshake_total{outcome,reason}`.
**`/api/mcp/*` never reached `memwal_errors_total`.** `record_app_error`
fires from `AppError::into_response`, and the proxy returns raw status
tuples, so 784,627 errors incremented no error counter at all. The
refusal and unavailable paths now record.
Also sends `x-memwal-bridge-version` on the handshake. `x-memwal-client`
comes from `initialize`, which a failed handshake never reaches — and a
first connect happens before stdin is wired, so on the attempt that
matters the relayer had no idea who was calling. Which bridge build is
looping is the actionable half, and that the bridge always knows.
Bounded like the caches next to it: episodes are keyed by a value the
caller supplies, so 4,096 max and swept at 15 minutes. At the cap new
episodes simply are not timed.
**Sample the refusal line; keep the counter always-on.** `/api/mcp/*` has no rate limit ahead of it and a stuck client retries forever, so one line per refusal is one line per retry — 784,627 warn lines over the measured window, from an unauthenticated header. The counter (`memwal_mcp_handshake_total`) and `memwal_errors_total` now record every refusal unconditionally; the line carrying account, client and reason is written at most once per account per 60s. Bounded at 4,096 accounts, with expiry on insert, for the same reason the caches are. **Do not let a completed read resurrect an evicted pair.** Cold misses are deliberately not single-flighted, so request A could be reading while B observed a revoke and evicted — and A's older success then re-opened a full 30s window on a key already known to be gone. The cache now carries an eviction generation: an insert is skipped when it moved under the read. The caller still gets its answer; it just is not cached. **Stamp the entry from before the read, not after.** A slow `GetObject` was extending the stated 30s revocation bound by its own duration. **`Keep` is load-bearing in a second way the docs missed.** Beyond the stale-grace path, it stops an unavailable RPC from evicting a *fresh* entry another request wrote between this thread's miss and its failed read. Documented, since the existing test only pins the stale case. **Strategy 1's comment described pre-cache behaviour.** It still promised a live on-chain re-verify on every request; a fresh verify-cache hit now answers without touching Sui, including during an outage. Rewritten to say what the code does. **Trimmed the incident write-up out of the module comments**, per the same note #882 got. The invariants stay on the const and the type. Deferred: the borrowed-key lookup nit. It needs `raw_entry` or a `dyn` key to avoid the probe allocation, which is a wider change than this PR should carry; worth doing alongside #882's version rather than twice.
The existing test seeds a stale entry, which does not affect serving — ducnmm's point. The case that needs pinning is the other one: a fresh entry written by another request between this thread's miss and its own unavailable read must survive. Keep is unconditional, so the policy is the guarantee and no interleaving is required to assert it.
Every call built `(account_object_id.to_string(), public_key_bytes.to_vec())` just to look up, and this now runs on the signed-request path and on every MCP envelope — so the allocation landed on exactly the traffic the caches exist to make cheap. Key both maps through a `dyn DelegateAccountKey` borrow, the same shape #882 uses for its `(String, String)` pair. A hit allocates nothing; the owned key is built only where the map is written. The rejection cache shares the key, so one trait covers both. A test pins that the borrowed probe finds, and removes, what an owned insert wrote. If the two ever hashed differently the failure would be silent — no error, just a cache that never hits and a `GetObject` per request, which is the condition this PR was opened to remove.
…906) * fix(relayer): reconnect Redis and stop 503-ing sponsor without ConnectInfo (WALM-626) * fix(relayer): bound Redis reconnect and slim WALM-626 comments
…M-386) `parseRetryAfterMs` returned 0 rather than null for `Retry-After: 0` and for any HTTP-date already in the past, and the caller falls back to the floor only on nullish — so `Math.max(0, 0 ?? floor)` was 0, the throttle wait collapsed to the ~500ms geometric backoff this feature exists to remove, and `serverAdvised` stayed true, which also suppressed the concurrent-cap hint. A correct server reaches the second case simply by being a second behind the client's clock. Both now return null, which is what the function's own docstring already claimed it did for anything unusable. Only reachable in production since the relayer started forwarding `retry-after` at all, one commit ago — before that the header never arrived and this branch was dead code.
…asing one Two defects in `verify_delegate_key_cached`, plus a tightening of the guard added in bef0487. **The cap counted entries nobody could be served from.** The rejection TTL is 10s and the sweeper runs every 300s, so the raw length that `should_record_rejection` consults held up to thirty dead generations. A few thousand distinct made-up pairs — `/api/mcp/*` has no rate limit and `x-memwal-account-id` is unauthenticated — took every slot for five minutes, during which no genuine rejection was recorded at all and every looping client went back to one fullnode read per retry, which is the amplifier this cache exists to remove. Entries are now expired before the cap is consulted, the same policy `observability::should_log_refusal_at` already uses. **A success erased the rejection that contradicted it.** The Ok path ran `reject_cache.remove` unconditionally, before the generation check. So a read whose result the code then declined to trust still deleted another request's freshly recorded revoke: no positive entry (correct) and no rejection either (wrong), sending the next N requests back to the chain. The removal is deleted rather than made conditional — any entry present at that point was either stamped before this read, and so already stepped past at the lookup above and expired, or stamped during it, which is exactly the one that must survive. **The eviction guard now spans both maps.** bef0487 put the removal and the generation bump under one guard; the rejection write still happened after it dropped. Holding it across all three makes `entries` the single order between an eviction and a concurrent store. Deliberately NOT included, having been tried and reverted: bounding the chain read with `tokio::time::timeout`. It converts our own deadline into `OnchainVerifyError::RpcError`, which `is_unavailable()` reports as "the chain could not be read" — so a fullnode answering `KeyNotFound` at 6s was cut off at 5s and the revoked key kept serving from the stale-grace path for up to 630s, where before it evicted. Adversarial review caught it. The underlying issue — `verified_at` is stamped before an unbounded read, so an entry's real life is the TTL minus that latency — is real and still open; the fix has to let a definitive answer land rather than outrank it.
…d-session case Both from ducnmm's review of bc531e5. `messages_proxy` still dropped `retry-after`. The sidecar rate limits that route as well, so a 429 on a JSON-RPC POST left the bridge unable to tell a cap that clears on a timer from one that clears when another session closes — the same gap the SSE and streamable allowlists just closed. It builds its response from a fixed header array rather than the allowlist loop the other two use, so the header is captured before `bytes()` consumes the response and added when present. The bridge tests only ever failed the handshake from cold start, so nothing pinned the case the `sent` flag was actually added for: a request issued after `firstConnectDone`, while `sse` is null, expiring on the 90s stalled deadline instead of the full call timeout. That is the shape Dio and Teo reported — the bridge worked, then the relayer stopped answering — and it is exactly the one `pendingForward` membership could not see. The new test serves one healthy session, kills it, refuses every reconnect, then issues the call, and asserts it comes back naming the failing connection well inside the call timeout. It needs its own mock: the existing one fails from the first handshake, which lands the request in `pendingForward` and so passes even with the bug present.
…unded maps Three notes from the cloud review, all the same shape: state or logging that this PR added, bounded everywhere except one place. **The stale-grace warn was unsampled.** It runs on every signed request and every MCP envelope, and it fires hardest precisely during a Sui outage, when many requests fall past the TTL at once — the 155,874 x 503 shape measured here. That is the same unbounded repetition the refusal line was sampled for one commit earlier, so this PR had learned the lesson and applied it to only one of the two. It now goes through the same per-account 60s policy. Its own map, though, not the refusal one. The two report unrelated facts — "this key was refused" versus "this key is being authenticated from a verification we could not refresh" — and an account in trouble tends to produce both, so one map would let whichever fired first silence the other for the whole window. Pinned by a test. **Neither sampler was swept.** Both expire on insert, but only once they hit the cap, so an account that went quiet an hour ago held its slot until an unrelated overflow reclaimed it. They were the only per-account maps this PR adds that the periodic sweep did not touch; they are in it now. **The connect-episode cap did not expire first.** `note_connect_attempt` runs before authentication, on a route with no rate limit, keyed on a header the caller chooses. At the cap it simply returned, so a few thousand made-up connect ids held every slot until the 300s sweep — and while they did, no legitimate client was timed at all, blinding `memwal_mcp_time_to_session_seconds` during exactly the incident it exists to measure. It now expires before consulting the cap, as the rejection cache and the samplers do. That last one self-heals once the spam stops; it does not stop a caller who keeps it up, and the comment says so rather than implying the hole is closed. Bounding that properly needs an authenticated key, or the measurement moved to the bridge, which already knows its own elapsed time.
…he call The mid-session test raced the bridge and lost about half the time. Sending the `tools/call` the instant the mock kills the session leaves `sse` still set when `handleClientLine` runs, so the call takes the POST path, `postIfCurrent` marks it `sent`, and it then correctly keeps the full call timeout — which is the opposite of what the test exists to pin. CI caught it: `bridge.tool_call` and `server-pump-eof` are logged in the same millisecond. The product behaviour is right and deliberate: once a POST has been issued we can no longer prove the call did not run, so it must keep the full deadline even if the socket then fails. Only the test needed a synchronisation point. It now waits for a second SSE GET to reach the mock, which means the reconnect ran and was refused — so `sse` is null and the handshake is on record as failing before the call is sent.
Two CI runs, two different failures, and I do not have a confirmed explanation for the second — so this comes out rather than staying red or landing flaky. Run 1 was a genuine test bug: the call was sent in the same millisecond the session died, so `sse` was still set, the call took the POST path, `postIfCurrent` marked it `sent`, and it correctly kept the full call timeout. Waiting for a refused reconnect before sending fixed that ordering. Run 2 still timed out with the call unanswered, and stderr carries no `bridge.call_orphaned` at all, so the sweeper never decided it had expired — even though both the elapsed time and the handshake-failing time exceed the 3s deadline by the first 5s tick. That does not match my reading of the sweeper, which means my reading is wrong somewhere I have not found. The mock is a suspect (it answers POSTs 202 regardless of session state, where the real relayer 404s a dead session), but that is a hypothesis, not a diagnosis. The product change it was meant to cover stays: `InFlightEntry.sent` is what distinguishes "never left the process" from "might have run", and `pendingForward` membership never could. What is missing is the proof, and the honest state is that the mid-session path is exercised by no test. Pinning it likely needs a seam the mock can drive rather than a timing race against an internal state transition it cannot observe.
…bridge's own log Third attempt, built so that a failure explains itself. The previous two raced an internal state transition the mock cannot observe and then failed in ways the output could not account for. Every precondition is now confirmed from the bridge's stderr before the next step runs — `bridge.connected` before the session is killed, `bridge.reconnect_failed` before the call is issued — and each wait names what it was waiting for and dumps both stderr and the received envelopes on timeout. The mock also 404s a POST against the dead session, as the relayer does, rather than answering 202 and letting a stray post quietly flip the request to "sent"; the test asserts the POST count did not move, which is the thing that actually earns the short deadline. Covers what `pendingForward` membership structurally could not: a request issued after `firstConnectDone`, while `sse` is null, expiring on the 90s stalled deadline instead of the full call timeout.
The mid-session test earned its keep on the third attempt: it proved the `sent` flag alone does not fix the case it was added for. `sse` is not cleared when the server pump hits EOF — it keeps pointing at the dead session until a reconnect succeeds. So a call arriving during a mid-session outage does not take the buffering path at all: it takes the POST path, posts to the stale session URL, and is 404'd. `postIfCurrent` had already marked it `sent`, so it kept the full call timeout and came back blaming a dropped connection — exactly the symptom the stalled deadline exists to remove, and exactly what I claimed the previous commit had fixed. A 404 here is the relayer saying that session does not exist, so the message was discarded rather than routed: it provably did not run. The request therefore goes back to being never-sent and can take the short deadline. The conservative default is unchanged everywhere else — marking still happens before the await, because a network error mid-flight is genuinely ambiguous and must keep the full timeout.
) * fix(mcp): tell the client its credentials were rejected (WALM-602) A 401 on the SSE handshake was caught by the background connect's generic `catch`, so it backed off and retried a delegate key the relayer will never accept. The queued `memwal_recall` stayed parked in `pendingForward` until the orphan sweeper's deadline — 240s by default — and was then answered with "the connection to the relayer dropped, please retry", advice that cannot work when the key itself is the problem. GH #365 reported the symptom as an expired session returning empty results. The empty half closed server-side in 0.0.11 (45b0ad8 made the MCP proxy require a registered delegate, so an unregistered key no longer opens a session that then honestly reports zero rows). This is the client half: the rejection now names itself and points at `memwal_login`. Carry the 401 as `RelayerUnauthorizedError` so the connect loop can tell it apart from a retryable failure, fail everything queued the moment it lands, and refuse later requests immediately while the key stays rejected — the same shape as the existing signed-out refusal, and placed just after it so `memwal_login` still returns locally and leaves a way back in. The loop keeps running: a successful connect clears the flag, covering both a re-registered key and a transient WAF or rate-limit 401. Credentials are still never wiped automatically. Reconnect-time revocation still takes the old path; #365 is a fresh process, so the first connect is the reported case. * fix(mcp): keep the bridge alive and recoverable after a rejected key Review follow-up on the WALM-602 fix. The fail-fast 401 answered the first call correctly but left the bridge unable to come back, and the recovery the changelog described was never reachable. `signalFirstConnect()` on the handshake 401 let the server pump run with `sse` still null, so it took its "stdin closed before we ever connected" break, won the shutdown race in `runBridge`, and `markStdinClosed()` disabled the very `reconnect()` the error text points at. `failPendingForward` writes to stdout directly and never needed the pump, so drop the signal and leave the pump parked until a session actually exists. `credentialsRejected` was cleared only where the background connect publishes. `memwal_login` republishes through `reconnect()`, so every later memory call stayed refused. Clear it wherever a handshake is accepted, and signal `firstConnect` there too — otherwise the first session to exist at all is one nothing is draining until the connect backoff, up to 15s, happens to expire. A handshake that 401s after a login already replaced the key says nothing about the new one; latching the flag on it would refuse requests against a live session. Guard the set with the same staleness test the publish path uses. Mid-session revocation now takes the same path: `reconnect()` recognises the 401, answers the replay set instead of leaving it to the orphan sweeper's "connection dropped, please retry", and the pump keeps driving reconnects so a transient WAF or rate-limit 401 still recovers with no client intervention. `initialize` no longer arms a suppression it will not forward while the key is rejected, mirroring the logout path. Tests cover the second fail-fast call, recovery through `memwal_login` (with the pump released promptly, not on the backoff), the in-flight call at revocation time, and unattended recovery from a transient 401. Each fails without its fix: 66/66, tsc clean. * docs(mcp): list the rejected-key fix in the 0.0.13 docs changelog (WALM-602) The package CHANGELOG carried the WALM-602 bullet, but the docs changelog's 0.0.13 section, its intro line and the `answer:` frontmatter did not mention it. Copy the bullet and name the fix in both summaries. No version bump: 0.0.13 is still unpublished. * fix(mcp): clear an unsent initialize's suppress arm when the key is rejected (WALM-602) At cold start the bridge answers `initialize` locally, arms a suppression for the upstream reply, and queues the request until a session exists. When the handshake then 401s, `failPendingForward` closes the queue out through `failRequest`, which keeps initialize arms on purpose for replies that can still arrive. This one never can: the request was never forwarded. Now that `memwal_login` restores service, the leftover arm is reachable. A client that reuses the initialize id has the genuine reply dropped, and the pump untracks the id as it drops it, so the orphan sweeper never answers either. The call hangs. Delete the arms of queued initializes before failing the queue. The new test holds the relayer's 401 until initialize is queued, signs in, reuses id 1, and fails by timeout without the fix. --------- Co-authored-by: Le Tien Phat <91601109+Niko1444@users.noreply.github.com>
Resolves three conflicts in `bridge.ts` and one in each changelog. All four are both-sides-added at the same point, not competing edits, so every resolution keeps both sides. `#894` (WALM-602) landed on dev and touches the same connect paths: - **reconnect() success** — it clears `credentialsRejected` and calls `signalFirstConnect()`; this branch resets the throttle deadline, the handshake-failure clock and the connect episode. Independent state, both kept. - **reconnect() catch** — it answers the replay set on a `RelayerUnauthorizedError`; this branch records the throttle deadline and the handshake-failure clock, and logs with the captured reason. Kept both, with one log line rather than the two the raw merge would have produced. Noted at the site that the 401 path takes precedence over the stalled-handshake deadline: a rejected key is terminal until the user logs in again, so waiting out even the short deadline buys nothing. - **connectInBackground() success** — same shape as the first. The two paths cannot double-answer a request: `failRequest` deletes the id from `inFlight` before writing, so whichever runs first removes the entry the other would have found, and `closedOut` still absorbs a late genuine reply. Both changelogs keep all eleven 0.0.13 bullets — dev's eight plus WALM-602's one plus the two here. Verified: `tsc --noEmit` clean; `cargo test --lib` 466 passed / 21 failed and `--bins` 690 passed / 47 failed, every failure a sandbox `PermissionDenied` in jobs/routes/walrus, matching the pre-merge baseline exactly.
…verify-cache fix(mcp,relayer): stop refusing valid MCP credentials during a Sui throttle, and stop hiding it (WALM-618, WALM-386)
…JSON-RPC CORS (WALM-604) (#891) Researcher's Google/Enoki sign-in failed in dev, staging and production: public Sui fullnodes no longer answer browser JSON-RPC preflights, so dapp-kit's ambient SuiJsonRpcClient died on getNormalizedMoveFunction with "Failed to fetch". Noter was moved to gRPC for this in #684; researcher was left behind. Ports noter's approach rather than the stale researcher half of hotfix/noter-mainnet-rpc-cors: - lib/sui/grpc-client.ts — a standalone memoized SuiGrpcClient, bypassing SuiClientProvider (still hard-typed to SuiJsonRpcClient) instead of casting a gRPC client through it. Base URLs are hardcoded as in noter, so no new NEXT_PUBLIC_SUI_GRPC_URL has to be wired into three deploy environments to avoid silently falling back to the broken path. - lib/sui/account-lookup.ts — registry/account reads over gRPC. These use include: { json: true } rather than hand-written BCS struct schemas; BCS schemas decode silently wrong once a struct grows a field, and account.move has already grown fields that noter's account-bcs.ts does not model. - enoki-login-card.tsx — pre-serialize the sponsored transaction with the gRPC client before handing it to dapp-kit's signTransaction, which is what actually short-circuits the failing ABI resolution. - sui-providers.tsx — hand registerEnokiWallets the gRPC client too; otherwise Enoki keeps a dead JSON-RPC client for the zkLogin flow. Verified against live testnet and mainnet: the json reads agree with BCS decoding on both registries, and six real owner addresses each resolve to a MemWalAccount whose on-chain owner matches the address looked up. Co-authored-by: Le Tien Phat <91601109+Niko1444@users.noreply.github.com>
Plugin .mcp.json launched unpinned npx, so a ^0.0.5 cache never picked up 0.0.9–0.0.13. Pin args to the package version and check it in the release verify script.
install_codex_hooks.mjs still wrote unpinned npx args when registering [mcp_servers.memwal]. Read the package version and pin the spec.
Drop the package.json walk-up; plugin.json is already version-synced.
…od-after-write (#806) * fix(mcp): write credentials through a fresh 0600 inode instead of chmod-after-write saveCreds wrote the delegate private key to the final path and only then called chmodSync(0600). writeFileSync's `mode` follows POSIX open(): the kernel applies it when it creates the inode and ignores it for one that already exists. A credentials.json left world-readable by anything outside this code — a manual chmod, a restored backup, another tool — therefore received the plaintext key under the old permission, with a second, non-atomic syscall to tighten it afterwards. Anyone reading the path in between gets the key (GH #520). Both writes now go through writeSecretFile(), which creates a randomly named temp file in the target directory with { mode: 0o600, flag: "wx" } and renames it into place. O_EXCL means the mode is always honored on creation and a temp path that already exists — an interrupted earlier run, or a file planted by someone else — fails hard rather than being written through. rename(2) repoints the name atomically, so a reader sees either the whole old file or the whole new one, never a permissive inode holding a fresh secret. The temp file is unlinked on any failure. The account-switch backup is fixed with it. copyFileSync + chmodSync is the same bug class and needs no precondition at all: every switch created a second plaintext copy of the same key under the process umask before tightening it. The regression test asserts the property that closes the window rather than trying to observe the race: a reader holding the pre-existing 0644 inode open across the save must never see the new key. It fails on the previous code with exactly that assertion. Note the issue's line references (auth.ts:59-70, CREDS_PATH) predate the project-local credential resolution, so its "~/.memwal is 0700 and limits exposure" caveat is weaker than stated — credsPath() can resolve to a .memwal inside a project directory. * docs(mcp): address style-guide audit on the credentials changelog entry Rewrite the account-backup sentence in active voice: the CLI is the actor that writes the backup and that previously created it under the process umask. * fix(mcp): keep a Windows credential save from failing on a locked destination Review follow-ups on WALM-312. `renameSync` is the POSIX-correct replace, but Windows implements it as `MoveFileEx(MOVEFILE_REPLACE_EXISTING)`, which refuses with EPERM / EACCES / EBUSY while another handle holds the destination — an antivirus scan or a backup agent on `credentials.json` is enough. The `writeFileSync` this PR replaced survived that, and `login.ts` turns a thrown `saveCreds` into an HTTP 500, so the change traded a permission bug for a failed sign-in. Windows now retries briefly and then writes in place. The fallback gives up the atomic swap but not what this code is protecting: Windows does not enforce POSIX mode bits at all, so `0600` was never doing the work there — NTFS ACLs are, and they are inherited from the directory either way. POSIX keeps the plain `renameSync` with no retry and no fallback, because there the mode IS the protection and `rename(2)` replaces a destination regardless of who holds it. Tests whose premise is the mode bit now skip on Windows rather than asserting something the platform does not implement, and the backup test keeps its content assertions everywhere. Also drops the unused `home` binding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HsS2mBzMfpzy3QE8EMiKvS * fix(mcp): unlink the temp file when a Windows save falls back to writing in place Review follow-up. The fallback returns successfully, so `writeSecretFile`'s catch never runs and nothing removed the temp — which still holds the plaintext delegate key. The comment claiming otherwise was wrong. Every locked save left another `.credentials.json.<pid>.<uuid>.tmp` beside the credentials file, which is the opposite of what this helper exists for, and a regression against the old in-place `writeFileSync` that never created a sibling at all. Unlinked best-effort after the destination write lands, so a temp that cannot be removed does not fail a save that already succeeded. `replaceWithTemp` now takes an injectable platform / rename / sleep and is exported for tests. CI has no Windows runner, and this is the one branch here that can leave a second copy of the key on disk, so it needed to be exercisable off Windows rather than reasoned about. Seven cases: each lock code lands and leaves nothing, repeated locked saves do not accumulate, a lock that clears retries instead of falling back, a non-lock error still propagates, and POSIX neither retries nor falls back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HsS2mBzMfpzy3QE8EMiKvS * docs(mcp): fold the #520 credentials note into the open 0.0.13 release Rebasing onto dev dropped this branch's `## Unreleased` heading and its duplicate #705 entry, which dev had already folded into 0.0.12, but it landed the #520 note at the end of that same 0.0.12 list. npm publishes 0.0.12 as `latest`, so a fix that is not in it cannot be listed under it. Move the note to 0.0.13, the section dev opened and has not published (npm carries only 0.0.13-dev.0), and say so in the mdx release line and the `answer:` summary. The 0.0.13 bump itself came with the rebase: the MCP manifest, the six plugin and marketplace JSON files, and verify-manual-sdk-release.mjs already carry it, and the script passes. * docs(mcp): match the #520 changelog bullet across both files d860fc4 applied the style-guide audit's active-voice wording to docs/mcp/changelog.mdx only, so the same sentence in packages/mcp/CHANGELOG.md still read "is written" and "was previously created". The two 0.0.13 Fixed lists are meant to stay identical. Published sections are left as they are. --------- Co-authored-by: Le Tien Phat <91601109+Niko1444@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hrough the public edge Measured on dev: every MCP tool call took ~61s. `memwal_health` — one unsigned GET the relayer serves in 10ms — answered in 61.9s, twice in a row, warm and cold. The relayer's own metrics locate it. Polling `/metrics` every 2s across a single health call, the `/health` counter does not move for 60s, then increments by exactly one, and the SSE reply lands 0.1s later. One request, no retries: the sidecar spends a minute reaching a relayer that then answers instantly. Neither side logs anything in between. It is reaching it the long way round, and so is every other environment. Read from Railway: prod dials https://relayer.memory.walrus.xyz (railway edge) 0.3s staging dials https://relayer.staging.memwal.ai (cloudflare) 0.3s dev dials https://relayer.dev.memwal.ai (cloudflare) 61s None dials loopback, and none sets MEMWAL_PUBLIC_RELAYER_URL — because `MEMWAL_RELAYER_URL` is both the address the sidecar dials and the only way to make `memwal_health` name the network a session is bound to, and the env reference told operators to set it to the public origin for exactly that reason. So every `memwal_*` call in every environment leaves the container and comes back to the process it started from. Two of those round trips are cheap and one costs a minute. Why dev's is the expensive one is not settled here: Cloudflare fronts staging too and staging is fast, and dev's other egress is healthy in the same metrics scrape. What is certain is that a sidecar dialling loopback never takes that path, and that no deployment should pay a public round trip for an in-process call as the price of naming its network. Split the two: - `MEMWAL_PUBLIC_RELAYER_URL` becomes an operator input of its own, so naming the network no longer moves the dial off loopback. An operator-supplied `MEMWAL_RELAYER_URL` still doubles as the public origin when no explicit one is set, so all three environments keep reporting what they report today. - A non-loopback dial address is warned about at startup, naming the cost and the variable to move the value to. - The sidecar's own default drops `localhost` for `127.0.0.1`: it only covers a standalone run, and there a dual-stack `localhost` can resolve to an address nothing answers on. And stop it being invisible. The only reason this was findable was diffing a Prometheus counter against a wall clock: the relayer's latency histogram starts when the request lands, so a minute spent reaching it reads as healthy there, and the sidecar logged `tool.call` going in and nothing coming out. Every tool call now reports its duration — `tool.done` at info, `tool.slow` at warn past `MCP_TOOL_SLOW_WARN_MS` (5s), `tool.failed` on the error path — each naming the address dialled, which is the thing an operator changes. Tests: `cargo test -p memwal-server` 1,162 passed (6 new; 68 failures are sandbox `PermissionDenied`, all in tests that bind local sockets). Sidecar suite 225 passed including 3 new; its 33 failures are the same sandbox socket restriction in `integration.test.ts` and `instructions.test.ts`. Follow-up to #900, which fixed the handshake half of the same user experience. Nothing here is on that PR's path: it changed `bridge.ts` and six Rust files, none of them the sidecar or the SDK.
…loyment's own public edge
Amends the approach in the previous commit, which was unsafe. It told
operators to unset `MEMWAL_RELAYER_URL` to stop the round trip — but that
variable is also the MCP OAuth issuer:
// oauth.rs:161
let issuer = std::env::var("MEMWAL_RELAYER_URL").ok()?;
The `?` makes `McpOAuthConfig::from_env` return `None`, which leaves
`config.mcp_oauth` empty and every OAuth handshake refused with
`OauthNotConfigured` (mcp_proxy.rs:397). All three deployed environments
have `MCP_OAUTH_DELEGATE_ENCRYPTION_KEY` set, so following that advice
would have taken every Claude custom connector offline.
So the dial address is the half that moves, not the public identity.
`MEMWAL_RELAYER_URL` keeps its meaning — OAuth issuer, and the network
`memwal_health` names. What the sidecar dials is now loopback unless
`MEMWAL_SIDECAR_RELAYER_URL` explicitly says otherwise, which almost
nobody should set: the managed sidecar is a child of this process and the
relayer it needs is this one.
The result is that no deployment changes an environment variable and none
pays a public round trip per tool call. Measured on dev, that round trip
costs ~61s on every `memwal_*`; prod and staging pay a cheaper version of
the same thing.
Also from review of the previous commit:
- The timing instrumentation could not observe the failure it was written
for. It logged on settle, and a hang never settles — the 61s incident
would have produced silence for the whole minute. It now fires from a
timer while the call is still outstanding (`settled: false`), and the
settle-time line marks `alreadyWarned` so one call is never counted
twice. The timer is `unref`d so it cannot hold the sidecar open.
- `MCP_TOOL_SLOW_WARN_MS` was `parseInt`'d with no validation: "" and
"abc" gave NaN, every NaN comparison is false, and the warning silently
turned off. Now validated, falling back to the default and saying so.
- `tool.failed` named no error. It now carries `errName`, `errMessage`
and `causeCode`, so the structured log is enough on its own.
- `is_loopback_relayer_url` missed IPv4-mapped IPv6 (`[::ffff:127.0.0.1]`,
the form a dual-stack listener reports) and a trailing-dot `localhost.`.
- The dial URL is echoed in startup warnings and per-call logs, so any
`user:password@` is stripped before it reaches either.
- The new test's 40ms threshold was tight enough to flake in CI; raised.
Tests: `cargo test -p memwal-server --bin memwal-server` 697 passed
(7 sidecar-URL tests, up from 6; the 47 failures are sandbox
`PermissionDenied` in tests that bind local sockets, none in main.rs).
Sidecar suite 75 passed across every file that does not bind a socket,
including 4 in tool-duration-log.test.ts; `tsc --noEmit` clean.
… the launcher
A `memwal_remember_bulk` whose reply never came back was reported as:
❌ Walrus Memory did not answer this call. The connection to the
relayer dropped before the result came back. Please retry.
Every clause after the first is wrong for a write, and the last one costs
money. The relayer answers `/api/remember/bulk` with HTTP 202 and finishes
the work in a durable Postgres/Apalis queue, so a client-side deadline
cancels nothing: items can still be landing long after the bridge gave up.
`/api/remember/bulk` also carries no idempotency key — unlike the single
path, whose `RememberRequest` has one precisely "so an ambiguous timeout
can't produce a duplicate paid on-chain blob" — and there is no content
dedupe anywhere on the write path. So "please retry" tells the user to
mint a second paid copy of memories that may already be stored, which
`recall` will then hide behind the first.
#900 rewrote this message for the two cases where the call never left the
bridge. It copied the already-sent branch across byte for byte, and that
is the branch a bulk POSTed on a healthy session lands in: `postIfCurrent`
sets `sent = true` before awaiting the POST, deliberately, because once
the request is issued we can no longer prove it did not run.
So distinguish by what the call does, not only by whether it was sent:
- A sent `memwal_remember` / `memwal_remember_bulk` / `memwal_analyze` now
says the write may have completed, that a timeout does not undo it, and
to check with `memwal_recall` before re-saving anything.
- A sent read says plainly that retrying is safe, which is true and is
what the old text meant to say.
- The two never-sent branches #900 added are untouched.
Also pin the launcher configs to `@mysten-incubation/memwal-mcp@latest`.
Unpinned, npx recorded `^0.0.5` in its cache, and a caret on a `0.0.x`
version locks the exact patch — so a reporting machine kept launching
0.0.5 for weeks after 0.0.9 through 0.0.12 shipped, and updating the
plugin did not move it. `.mcp.json`, `.cursor-mcp.json` and
`.codex-mcp.json` all carried the same unpinned spec.
Changelog entries go under 0.0.13, which is the version in package.json
and has no stable release on npm yet.
The hermetic suite proves the bridge's intent against a mock relayer. It
cannot tell you whether a deployment answers, and the 2026-09-14 report is
entirely about one that does not — so "is it fixed" kept coming down to
opinion. This runs the reported sequence against a real relayer and prints
a scorecard with thresholds taken from the report's own expectations.
Opt-in like the other live scripts: the `.live.mjs` suffix keeps it out of
the `npm test` glob and it exits early without `MEMWAL_LIVE_HOME` /
`MEMWAL_LIVE_RELAYER`. Writes cost a real Walrus blob, so T4-T6 need
`MEMWAL_LIVE_WRITE=1` and a testnet deployment; the read cases are safe to
run anywhere, production included.
T2 and T3 are the pair that matters. `memwal_health` is an unsigned GET
with no SEAL preamble, so it still answers on a deployment whose signed
path is broken — health passing while recall 504s is precisely that shape,
and it is what dev does today:
[PASS] T1 handshake reaches a session 0.8s
[FAIL] T2 memwal_health answers promptly 60.9s
[FAIL] T3 memwal_recall over the signed path 60.5s Unexpected status code: 504
T6 answers the question no log could: when a bulk reports failure, recall
is the only honest way to find out whether anything was stored — and that
answer decides whether a retry would have duplicated paid blobs.
… bit, green CI Review fixes plus the two CI failures. Review (@ducnmm): - [bug] `outcomeFields()` copied `session.relayerUrl` into every `tool.slow` / `tool.done` / `tool.failed` line with no redaction. That field is the sidecar dial URL, and `redact_url_userinfo` in main.rs only covers the Rust startup lines — so `https://ops:hunter2@host` reached JSON stderr on every call. Added `redactUrlUserinfo` in the sidecar, mirroring the Rust helper, with two tests that exercise it through `wrapTool`. An unparseable value is now dropped rather than echoed: it cannot be redacted, so it cannot be shown. `fetch` still gets the real URL. - `alreadyWarned` read `!watchdog.hasRef()` after `unref()`. `hasRef()` is false from the moment `unref()` is called, while the timer is still pending, so the bit was always true on settle and the comment explaining it was wrong. It is now set inside the timer callback. - The notes bullet still described the revision-1 meaning of `MEMWAL_RELAYER_URL` ("only needed when the sidecar should call a different relayer"), which reads as permission to unset it — the exact footgun that takes `McpOAuthConfig::from_env` to `None`. Rewritten to point dial overrides at `MEMWAL_SIDECAR_RELAYER_URL` only. - `MCP_TOOL_SLOW_WARN_MS` was documented as changing how a *completed* call is logged. The load-bearing behaviour is the in-flight warn, since a hang never settles. Table cell now leads with that. CI: - `Compile & CLI Smoke` — my own test was flaky. It asserted `durationMs >= threshold` on the FIRST `tool.slow` line, which is now the in-flight one emitted by the timer at the threshold itself; its duration sits within a millisecond of the threshold and landed at 149 against 150 on the runner. It now asserts on the settled line, which is the one whose duration must exceed the threshold. - `MCP / Integration (login handoff)` — `orphaned-call.test.mjs` asserted that an orphaned `memwal_remember` tells the caller it is "safe to retry". That is the assumption this PR reverses on purpose: the call was POSTed, the relayer answered 202 and finished in a durable queue, and `/api/remember/bulk` has no idempotency key, so a blind repeat buys a second paid blob. The test now asserts the write-case contract — says it may have completed, points at `memwal_recall`, never says "please retry" — and is renamed to match. Verified each assertion against the emitted string; the negative is scoped to "please retry" because the message deliberately says it "does not mean nothing was stored". Sidecar suite 77 passed (6 in tool-duration-log, 2 new), `tsc --noEmit` clean. The `packages/mcp` suite still cannot run in this sandbox (`listen` denied), so `orphaned-call.test.mjs` is verified by CI rather than locally.
Two items from the re-review of 27e48aa that the previous commit did not cover. The rest of that review ("still unfixed": wrapTool userinfo, the `hasRef` pairing bit, the stale notes bullet, the two CI failures) landed in 195e9b6, which was pushed 36 seconds after the review was submitted. - `docs/mcp/changelog.mdx` carried none of the new 0.0.13 entry. The Mintlify page is the user-facing changelog, and its `answer:` block is what AI search cites, so a fix that only exists in the package CHANGELOG is invisible where people actually look. Copied the bullet into `### Fixed`, and led both the intro and the `answer:` with it — "a lost reply does not mean the write did not land" is the sentence a user needs before they retry something that costs money. No version bump: dev is already 0.0.13 and it is unpublished. - Dropped the launcher JSON from this PR. `@latest` is a moving dist-tag, not a pin, and #913 (WALM-627) already does this properly: it pins the exact `@0.0.13` that `packages/mcp/package.json` declares, and covers the Codex fallback installer too, which this PR did not touch. Two PRs editing the same three files with different values is a conflict for no benefit. Removed the matching CHANGELOG bullet with it. That leaves this PR to one subject again: the sidecar dialling loopback, the in-flight slow-call signal, and the sent-write message.
fix(mcp): make dev usable again — dial loopback instead of the deployment own public edge
…-npx-so-users-stay-on-stale-memwal Conflicts came from #912 touching the same launch configs and the same 0.0.13 changelog section. - `.cursor-mcp.json` / `.codex-mcp.json`: keep this branch's exact `@0.0.13` pin over dev's `@latest`. `@latest` still lets npx serve a cached build, which is the failure WALM-627 is about, and dev's 4f2bad7 left the launcher pin to this PR on purpose. - `packages/mcp/CHANGELOG.md` and `docs/mcp/changelog.mdx`: both sides added an entry under the same unpublished 0.0.13 heading, so keep both, and fold the pin clause back into dev's rewritten `answer:` summary and release blurb rather than dropping either side's prose. `node scripts/verify-manual-sdk-release.mjs` passes, including the new launch-config drift check.
…aunches-unpinned-npx-so-users-stay-on-stale-memwal fix(mcp): pin memwal-mcp version in plugin npx (WALM-627)
Promote dev → staging (WALM-617)
ducnmm
approved these changes
Sep 15, 2026
This was referenced Sep 22, 2026
Merged
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hop 2 of the WALM-617 promotion (week of 10 Sep 2026). Takes current
stagingontomain.Heads at open
stagingcbbc9c62main01aa559b61 commits, 98 files, +9476 / −482. This is hop 1 (#909) unchanged —
staginghas taken nothing else since that merge.Gate evidence
CI on
cbbc9c62: 27 success, 1 skipped, 0 failures. The skip isE2E / dev relayer, which only runs ondev.Staging smoke was run against the published MCP artifact (
@mysten-incubation/memwal-mcp@0.0.13-rc.0, what hop 1 published), not a local build, and repeated againstdevfor comparison:memwal_healthmemwal_recallmemwal_remembermemwal_remember_bulk/healthreportswrite_ready=true,writes: ok, andrememberreturned a real blob id on both. No wallet-funding leak observed onremember.Staging is faster than
devon every measure, so this promotion does not regress the deployment it is being promoted onto.Write latency — measured, and accepted
A single-fact
memwal_remembertakes 24.2s on staging, 32.4s on dev;memwal_remember_bulk(5 facts) takes ~50s and stores 5/5. The GH field report expected "a few seconds", but that was the reporter's wording rather than an agreed target, and these figures are accepted as the current SLO for the testnet write path.Recorded here because it is the field report's ask #1 and the numbers should be on the record, not because it blocks this hop:
devandstagingalike, anddevis the slower of the two, so nothing here introduces it.POST /api/remember→ 202). The rest is the Walrus pipeline:registered~10.6s (a Sui transaction) anduploaded~10.4s (push to storage nodes) dominate. Neither is inside MCP's control.failed=0, on both environments.The remaining lever at the MCP layer is the one the report itself proposes: return the
job_idonce the relayer answers 202 instead of polling to completion, so the agent is not blocked for the duration. That is an API behaviour change and is deliberately not part of this promotion.Release ordering — read before merging
Merging this pushes
main, which triggersrelease-mcp.ymlto publish@mysten-incubation/memwal-mcp@0.0.13aslatest.0.0.13does not exist on npm yet — only0.0.13-dev.7and0.0.13-rc.0:The plugin launch configs on this branch pin
npx -y @mysten-incubation/memwal-mcp@0.0.13(WALM-627, #913). Those become live onmainthe instant this merges, while the publish job still has to install, typecheck, build, test and publish. For that window a fresh plugin install resolves a version that is not on npm yet and fails. #913 called this dependency out in its own Risks section.There are no pending changesets, so
changeset versionis a no-op onmainand the published version will be exactly0.0.13— the pin resolves once the job finishes. Nothing here is permanently broken; the exposure is the length of that one workflow run. WatchRelease MCP Packageafter merging and confirmnpm view @mysten-incubation/memwal-mcp@0.0.13 versionresolves before announcing the plugin release.Package versions
stagingvsmain: TS SDK0.1.7(main0.1.6), MCP0.0.13(main0.0.12).Supersedes
#907 (
fix(relayer): reconnect Redis so account setup is not 503) was cherry-picked ontomainso prod could take the relayer fix alone. Its content is already here —rate_limit.rs,Cargo.toml,Cargo.lock,walrus_seal.rsandsecurity_delete_auth.rsare byte-identical toe088697d, andtypes.rscarries the sameredis::aio::ConnectionManagerchange (mainstill hasMultiplexedConnection). Merging this makes #907 redundant; close it afterwards rather than merging both.Refs: https://linear.app/mysten-labs/issue/WALM-617/promote-dev-staging-main-week-of-10-sep-2026