Merge dev into staging - #922
Open
harrymove-ctrl wants to merge 40 commits into
Open
harrymove-ctrl wants to merge 40 commits into
harrymove-ctrl wants to merge 40 commits into
Conversation
A remember is a background job (pending -> running -> uploaded -> done) and the tool waited for `done` with a 90s deadline. Measured against production that costs 30-75s for a single short fact, while the job is durably accepted about a second in. Phase timing on one job: accepted 1.2s, pending->running 2.1s, running->uploaded 29.6s (84% of the call), uploaded->done 2.1s. Payload size does not drive it — a 1.2 KB fact finished faster than a 97-character one — so the agent spends half a minute waiting on a queue it cannot affect. memwal_remember now waits MEMWAL_MCP_REMEMBER_WAIT_MS (default 10s) for the write to land, so the fast path still returns a blob_id, and otherwise hands back the job_id and says plainly that the fact is not saved yet. Set 0 to always return at accept, or 90000 to restore the previous behaviour. Returning early cannot mean losing writes: a job can still fail after it is accepted — an outage on the SEAL encrypt sidecar lands it in `failed` with "Memory encryption backend is unavailable" long after the tool returned. So memwal_remember_status resolves an in-flight job by id, reporting a failed or missing job as an error envelope rather than a status line, and a genuinely failed job still reaches the agent as an error from memwal_remember itself. Only our own wait expiring is treated as a non-failure.
The fast-return path keyed "job is still running" off a `MemWalRememberJobTimeout` constructor name. No shipped SDK throws that: both the pinned 0.0.x and the current 0.1.x line reject with a plain Error carrying `status` — 504 when the wait ran out with the job still going, 500 when the job itself failed. `wrapTool`'s mapping for those names is dead code for the same reason. So every slow write came back as `isError: true` with "Tool error: remember job timed out after 30000ms", telling the agent the save had broken when it was simply still running — the exact failure mode the fast return exists to avoid. Caught by driving the tools against the production relayer; the unit tests missed it because the stubs threw a conveniently-named class instead of the shape the SDK actually produces. Match on `status` instead, and reproduce the real error shape in the stubs. memwal_remember_status now also names a failed job (500) plainly rather than letting it fall through to the generic "Tool error" prefix.
The 10s wait this branch shipped was the worst of both options. Against the measured 30-75s completion spread it lands in the pending branch on nearly every call, so the agent pays the full 10s and still gets no guarantee. Wait long enough to mean it (MEMWAL_MCP_REMEMBER_WAIT_MS=90000) or do not wait — so the default is now 0 and the tool returns once the relayer has durably accepted the job, ~1.1s. Returning at accept is safe from a disconnect: the job is a row in remember_jobs driven by the relayer, not work held in this process. It is not safe from a job that fails after acceptance, which is why the result never reads as saved and memwal_remember_status exists to settle it. Split the shared budget parsing and job-error mapping into remember-wait.ts rather than duplicating the 504-vs-500 discrimination in both tools. memwal_remember_status gains waitMs=0 for an immediate read. waitForRememberJob cannot express that — it sleeps before its first poll, so a 0ms deadline answers "still running" without ever asking the relayer. That needs getRememberStatus, added to the SDK in 0.0.4, hence the dependency bump. The wider 0.1.x upgrade stays with the sidecar SDK work. Also adds the Streamable HTTP transport behind MEMWAL_MCP_TRANSPORT (sse by default). It answers a call on the same request, so there is no idle watchdog and no replay-on-reconnect. Tests: 269 run, 236 pass, 33 fail — the same 33 sandbox port-binding failures present on dev. tsc --noEmit clean.
Round-robin `next_index()` hands out the next wallet in sequence whether or
not it is mid-upload. The sidecar allows one upload per wallet
(WALRUS_UPLOAD_PER_WALLET_CONCURRENCY defaults to 1), so landing on a busy
wallet costs the full upload ahead of it while other wallets sit idle.
Observed in production as a job waiting 6.2s for its assigned wallet with the
global semaphore reporting available:2, queued:0 — the contention was never
global, only per-wallet:
[walrus/upload] limiter_acquired {"keyIndex":6,"waitMs":6221,
"limits":{"global":{"capacity":8,"available":2,"queued":0},
"perWalletCapacity":1,"wallet":{"capacity":1,"available":0,"queued":1}}}
`least_loaded_index()` picks the wallet with the fewest in-flight attempts and
falls back to round-robin ordering among equals, so an idle pool still spreads
evenly and a fully busy one does not pile onto one signer.
Marking a wallet busy is a `WalletAttemptGuard` rather than paired
increment/decrement calls: the upload path has many early returns, and every
one of them has to release the slot. A guard cannot forget.
Tests: 11 new unit tests covering selection, guard release on early return,
nested attempts, saturating release, and an out-of-range index. Lib suite goes
466 → 477 passing with the same 21 pre-existing failures (no database in the
sandbox). cargo check clean, rustfmt clean on the touched files.
The sidecar installs @mysten-incubation/memwal from npm (Dockerfile runs
`npm ci` in scripts/), so packages/sdk in this repo is not what production
runs — and the pin had drifted far behind it. 0.0.4 predates every release
that matters here; most importantly it has no idempotency support at all,
so `rememberAsync` cannot send an idempotency_key and the relayer treats
every retry of a write as a brand-new one. A replayed remember therefore
mints a SECOND paid Walrus blob for a write already in flight. The SDK
grew keys in 0.1.2 ("collapse retries onto one paid remember job") and
production never received it.
0.1.7 is the current published version. The MCP layer only calls
analyzeAndWait / health / recall / rememberAndWait / rememberBulkAndWait /
restore, and deliberately excludes the manual-mode methods that carry the
one breaking change in the range (0.1.5 reshaped rememberManual), so the
used surface is unchanged. Verified by `npm ci` against the regenerated
lockfile followed by a clean `tsc --noEmit` over scripts/.
Also picks up the widened zod peer (^3.23.0 || ^4.0.0), which the
sidecar's own zod ^3.25.0 already satisfied.
memwal_remember no longer blocks to terminal, but memwal_remember_bulk and memwal_analyze still do, and both were left on the SDK's default cadence. That default backs off as min(10s, 1500ms * 1.5^attempt), so status checks land roughly 1.5, 3.75, 7.1, 12.2, 19.8 and 29.8s apart. Writes finish in the 15-35s band, which is exactly where those gaps are widest: a batch that truly completed at 20.5s is not reported until 29.8s. None of that is work, it is waiting to be told the work finished. Reuses REMEMBER_POLL_INTERVAL_MS rather than picking a second number — the rate-limit reasoning behind 400ms is the same one, and a bulk poll covers every pending job in one request (/api/remember/bulk/status takes all the ids), so the request budget matches a single remember's.
The sweeper applied one callTimeoutMs to every tracked request. That number is DEFAULT_CALL_TIMEOUT_MS, sized for memwal_analyze — the slowest tool there is — so a memwal_remember whose reply was lost (the relayer answered, the stream dropped before it arrived) kept the agent blocked for 240s even though that tool cannot still be working: it gives up on its own job long before. Users read a four-minute silence as a hang and reload the client, which is the reload-for-minutes symptom. Each entry now carries the deadline its own tool enforces plus 30s of transport headroom, fixed when the request is first tracked so a reconnect replay keeps the original budget. memwal_remember lands at 120s. The stalled-handshake shortcut still wins when it is tighter but can no longer extend a call past its tool's ceiling. Deliberately generous headroom: the sidecar answers at its deadline with a result or an error envelope rather than going quiet, so a reply is one hop behind it. Cutting a merely-late reply off early is the expensive mistake, because the agent then retries a write that actually landed. Behaviour is unchanged wherever MEMWAL_MCP_CALL_TIMEOUT_MS is set, which is every existing expiry test — resolveDeadlineMs returns the override as-is, and in the stalled path min(stalledHandshakeMs, callTimeoutMs) is already stalledHandshakeMs. Also drops the duplicate local toolNameOf in favour of the module-scope one this needs.
Three changes to what a caller waits for, none to what a write means. The backoff ceiling drops from 10s to 2s and the base default from 1500ms to 600ms. The ceiling is pure observation cost — a job that finished is not reported until the next poll lands — and at 10s the checks fell at ~1.5, 3.75, 7.1, 12.2, 19.8 and 29.8s, straddling the 15-35s band where writes actually complete. Polling is one indexed row read on remember_jobs, so the extra checks are cheap; callers on a long budget still pass a larger base because the relayer's per-delegate-key rate limit, not cost, is the real constraint. Both wait loops slept BEFORE their first check, so an idempotent replay of a write the relayer had already finished paid a full interval for a result that was ready on arrival. They now check first and sleep second. Generated idempotency keys are derived from the content over a 30-minute bucket instead of crypto.randomUUID(). pendingRememberKeys only dedupes retries that reuse one client instance, and the MCP sidecar builds a fresh MemWal per transport session — so a reconnect replay found an empty map and a random key read as a brand-new write, minting a second paid Walrus blob for one already in flight. The bucket bounds the collapse: remember_jobs rows are never pruned, so an unbucketed key would dedupe against a job from any point in history and re-saving a since-deleted fact would hand back the old blob id instead of storing it again. Callers passing an explicit idempotencyKey are unaffected, and distinct text or namespaces still derive distinct keys.
memwal_remember stopped blocking on the whole Walrus write; bulk did not, and bulk is the path an agent actually takes. The server instructions send it here whenever it learned more than one thing, so leaving this tool on a fixed 120s block meant the common multi-fact turn still stalled — the fast return only covered the case that mattered less. Blocking is also worse per fact here, not better. A batch is N separate Walrus writes contending for the same upload slots (WALRUS_UPLOAD_PER_WALLET_CONCURRENCY defaults to 1), so they land one after another. Against the measured 30-75s single-write spread a five-fact batch could exhaust the whole budget and come back as nothing but timeouts, having held the agent for two minutes first. Same shape as memwal_remember: rememberBulkAsync to accept, then hand back the job_ids. Safe on the same grounds — remember_bulk commits every remember_jobs row before it spawns preparation or answers (services/server/src/routes/remember.rs), so acceptance survives the client going away. Each job_id is returned paired with its fact, because with a batch "one of these failed" is only actionable if you can tell which. A non-zero MEMWAL_MCP_REMEMBER_WAIT_MS still waits, and that path now reports a mixed batch honestly: waitForRememberJobs does not throw on expiry, it marks stragglers `timeout`, so those are shown as still uploading with their job_ids rather than as failures. memwal_remember_status takes job_ids to settle a whole batch in one call. It deliberately does not throw on a failed job the way the single-id path does: a batch comes back mixed, and throwing on the first failure would discard the blob_ids of the writes that did land.
memwal_remember was observed still running past 120s by an MCP client, on a tool documented as capping at 90s. The cap is not a bound. The SDK's signedRequest aborts a request only when the caller hands it a signal, and of the memory methods only recall() does (15s). rememberAsync, rememberBulkAsync and every job-status read call it with no signal, so the underlying fetch has no deadline. waitForRememberJob then tests `Date.now() < deadline` at the TOP of its poll loop, which bounds when the next poll starts, not how long one takes — so a single stalled socket runs as long as it stays open and sails straight past timeoutMs. With nothing else in the way the stdio bridge's orphan sweeper is the first thing to fire, minutes later, which is what a user sees as the client hanging. Returning at accept does not fix this on its own: the accept POST is one of the unbounded calls, so memwal_remember could hang indefinitely even at MEMWAL_MCP_REMEMBER_WAIT_MS=0. Every entry point now runs under a deadline — accepts at 15s (the only deadline the SDK sets for itself; a healthy accept is ~1.1s), waits at their own budget plus grace, since the budget cannot bound its own last poll. The request is NOT cancelled: the SDK exposes no way to pass a signal, so fetch keeps running until it settles. What this bounds is how long the agent waits, which is the part a user experiences as a hang. An orphaned request costs one socket and resolves into a promise nobody reads. The timeout error is named MemWalRelayerUnresponsive rather than reusing a job-failure name, because "we do not know whether it was queued" is not "it failed" — and it says a retry is safe, since the SDK holds the same idempotency key until an accept succeeds, so a retry attaches to the existing job instead of queueing a second paid copy. Also documents MEMWAL_MCP_REMEMBER_WAIT_MS, which this branch introduced without an entry.
`fetch` has no timeout of its own, and this SDK passed an abort signal on exactly one method — `recall`, at 15s. The accept POST, every job-status read, and the `/version` and `/config` handshake calls that run before any of them could stay pending for as long as the socket stayed open. That is not merely untidy. A poll loop tests its budget at the TOP of each iteration, so it bounds when the next request starts, not how long one takes: a single stalled read ran straight past `timeoutMs`. A `memwal_remember` documented as capping at 90s was seen by an MCP client still running after 120s, with the stdio bridge's orphan sweeper the first thing to fire, minutes later. The MCP layer now wraps its own calls, but that only covers one consumer — the hole is here. Default 30s, matching the relayer's own outbound HTTP client: any call that needs the relayer to reach the sidecar, Walrus or OpenAI has already failed upstream by the time it fires. Settable via `requestTimeoutMs`; a non-positive or non-finite value falls back to the default rather than disabling the bound, since "no deadline" is the bug being fixed. Two endpoints legitimately outrun it and say so: `restore` (60s — the route bounds itself at 55s server-side and answers with an error rather than going quiet, so a tighter client deadline would abandon a reply already on its way) and `analyze` (60s — it runs the extractor LLM inline before it accepts). `recall` keeps 15s, now a named constant instead of a hand-rolled AbortController. Each poll inside a wait loop is bounded by the client deadline clamped to the remaining budget. Both directions carry weight: the remaining budget stops a poll outliving the wait it belongs to, and the client deadline stops ONE stalled poll swallowing the whole budget, which would leave the loop no room to retry. Expiry raises `MemWalRequestTimeout` with `status: 504` — already transient to `isTransientPollingStatus` — so a stalled poll is retried against what is left instead of failing the wait. An abort the caller asked for, and any other transport error, propagates untouched: an operator debugging DNS or TLS needs the original error, not "timed out".
`remember` commits the job row, then spawns preparation — summarize, embed, SEAL encrypt, enqueue — in a `tokio::spawn` inside the relayer process. If the process stops in that window the row is left at `pending` with nothing to resume it, and the sweeper never looked at `pending` at all. The state machine in migration 005 does not even name a `pending → failed` edge: the state had no exit. Nothing surfaced it either. `memwal_remember_status` read the row and reported "still uploading" indefinitely, so a write the user was told was on its way was simply gone. Returning at accept made that worse rather than causing it: the blocking call at least ended in a timeout the caller saw, where now nobody is waiting to notice. `pending` cannot be swept wholesale — a job that IS prepared waits at `pending` until a wallet worker takes it, and with WALRUS_UPLOAD_PER_WALLET_CONCURRENCY defaulting to 1 that queue is legitimately minutes deep, so failing those would abandon paid work about to run. `preparation_encrypted_b64 IS NULL` separates them: it is written by the statement immediately before `enqueue_wallet_job`, so its absence means the job never reached the queue. Failing is the only available outcome, not a preference. The row holds the SEAL ciphertext and never the plaintext, so a job that died before encrypting has nothing to retry from — the error says the fact was never stored and must be sent again, rather than implying a job merely died. Clearing `prepare_claim_token` is what makes this safe against a preparation that was slow rather than dead: that task's own UPDATE is fenced on the token, so it now matches zero rows, logs the lost claim and returns before `enqueue_wallet_job`. It cannot mint a paid blob for a job just declared dead. Quota needs nothing extra — `main` already runs `release_reservations_for_terminal_jobs` immediately after this on the same tick, ordered that way so rows this pass just failed are reconciled without waiting another minute. Composes with the existing idempotency recovery: a failed row with no blob_id is exactly what the remember route resets and re-prepares, so a client that does retry the same key gets a real second attempt instead of collapsing onto a corpse. Migration 005's comment still lists the old transitions. It is deliberately left alone — sqlx checksums migration files, so editing a shipped one breaks `migrate` on every deployed database; the transition is documented on the sweeper instead.
…M-470) (#911) * fix(server): stop scoring_weights reordering an explicit recall sort (WALM-470) A recall request carrying both sort and scoring_weights returned neither order: select_hits_for_sort ordered and truncated the hits, then the ranker reordered the survivors by composite score, so sort=recent stopped meaning newest-first. Per the decision on WALM-460, an explicit sort is the order and weights apply only when sort is omitted. RecallRequest.sort becomes Option<RecallSort> so omitted and "relevance" differ, and resolve_scoring_weights validates the weights, then suppresses them when sort is set. The SDK docs and both API references state the rule. * fix(server): narrow the embedding size check to the provider path (WALM-470) The WALM-423 check ran before the embedder looked for an API key, so deployments without one (local dev, CI, self-hosted) rejected remember and recall text over 16 KiB, though the key-less fallback hashes locally and has no context window. It now runs only when a provider key is set. The same change mapped every provider 400 to "input exceeds the model context limit". A 400 now becomes BadRequest only when its body names a context-length problem; anything else, such as an unknown model id, stays Internal so it is not blamed on the caller and still alerts. The embed call moves into embed_text, which takes only the key, base URL and text, so the tests can drive it against a local stand-in provider. * docs(sdk,server): add the 0.1.7 changelog entry and trim WALM-470 comments Review follow-up: record the recall sort precedence change under the unreleased 0.1.7 in both SDK changelogs (no version bump), and drop ticket ids, dates and design history from the new code comments. --------- Co-authored-by: Le Tien Phat <91601109+Niko1444@users.noreply.github.com>
…s (WALM-608) (#910) * fix(relayer): point the upload-queue saturation alert at real counters (WALM-608) The saturation monitor read queuedWalrusUploads, activeWalrusUploads and walrusUploadLimits.globalCapacity from the sidecar's /health. Since cef9728 (v1_new port, 31 Jul) /health is bare liveness and returns none of them, so every read fell through to unwrap_or(0) and the alert could never fire. /ready has the counters but waits on Sui, Walrus and an uncached archival GraphQL query, each with a 5s timeout, against the monitor's 2s budget. Backlogs arrive with Sui RPC pressure, so polling it would go blind exactly when the alert matters. - sidecar: add GET /metrics/uploads, serving the in-memory counters and limits with no auth, in both route modes - relayer: poll it; move parsing and the consecutive-check state into sidecar_saturation.rs; a missing or non-integer field, a non-JSON body or a non-2xx status logs an error instead of reading as an empty queue - docs: list the endpoint and the built-in alert in relayer observability * docs(relayer): trim WALM-608 comments to what the code guarantees Review follow-up: drop the incident history from the route, test and parser comments; the PR body keeps it. --------- Co-authored-by: Le Tien Phat <91601109+Niko1444@users.noreply.github.com>
…pted [WALM-332] (#793) * feat(server): add GET /api/whoami so a delegate key can resolve its own account A client that holds a delegate key but lost the surrounding metadata has no way to rebuild credentials: `account_id` is required locally, and nothing exposed it. `find_account_by_delegate_key` is internal to the auth middleware, `account_exists` takes an owner address and returns only `{exists}`, and `StatsResponse` carries `owner` rather than the account id. The middleware already resolves exactly what is needed while authenticating, so this hands back what it computed instead of doing new work: `account_id` and `owner` from the registry scan, `package_id` from config. Returning `account_id` is safe here precisely because the route is authenticated — the caller proved possession of a key registered against that account, so it only ever learns about itself. The public existence-check route deliberately withholds it, and that reasoning is unchanged. The field mapping is factored into `whoami_response` so it can be tested without a live AppState (this codebase has no axum-handler harness). `account_id` and `owner` are both 0x-prefixed 32-byte hex, so transposing them would compile and silently hand back the wrong identity — pinned by a test. Motivated by WALM-332. * fix(mcp): stop losing the delegate key when a login is interrupted The browser registers our delegate public key on-chain — a paid, irreversible action — and only afterwards POSTs the callback that makes us save the private half. Until now that private half existed solely in memory (`login.ts` created it, `saveCreds` persisted it only on success), and the callback listener lived in the same process. So any death in that window destroyed the only copy of a key that had already been paid for and committed on-chain, leaving an orphaned registration nobody could use. Nothing reported it: the browser's POST hit a closed port, and the process was gone so it logged nothing. `handleLocalLogin` had already returned the URL and told the client the call succeeded. Reproduced against the real binary over stdio: preflight succeeds while alive (200), the process is killed, and the callback POST gets ECONNREFUSED. Ports are never reused across restarts, so the stale tab cannot reach a new listener either — it fails at /preflight, never reaching the state check. Note this is NOT the state-nonce mismatch WALM-332 originally described. That path is unreachable: the callback handler gates `preflightVerified` before it compares state, so a bad-state 403 requires the same live process to have accepted a preflight carrying its own nonce. The fix is write-ahead. Persist the keypair before anything can hand the public key to a browser, and clear it once the key is safely in credentials.json. On the next start a stranded record is reclaimed via the new authenticated `GET /api/whoami`, which supplies the account metadata the lost callback would have carried. Two deliberate constraints: - A rejected key is never deleted. A 401 means "not registered" on mainnet, but on testnet the registry scan is disabled outright and a genuinely registered key is rejected for want of an x-account-id hint. Deleting would destroy a paid key in exactly that environment, so the record waits out its 24h TTL instead. - Recovery never rolls back a newer sign-in. If the user gave up and signed in again, adopting the older stranded key would silently downgrade them. Also fixes the swallowed failure in `startOrReuseLoginFlow`: a login that fails while the process is still alive (timeout, listener error) was eaten into a `warn` and never reached the client. It now logs at error, writes to stderr, and emits an MCP notification so the agent stops waiting on a dead flow. The canonical signature string is duplicated across Rust and TypeScript because they cannot share code. A silent drift there would fail only in production, as an opaque 401, so the exact literal is pinned by a test on each side with a comment pointing at the other. WALM-332. * docs(relayer): drop em dashes from the whoami section The Sui docs style guide disallows em dashes in prose. Split the first aside into its own sentence and parenthesized the second; wording is otherwise unchanged. * fix(mcp): make the WALM-332 recovery path actually able to reclaim a key Review follow-ups on WALM-332. The write-ahead record was in place, but nothing downstream of it worked. - `whoami` signed `String(Date.now())`. The relayer freshness-checks `x-timestamp` against `Utc::now().timestamp()` — seconds — so a millisecond value was always outside the drift window and every recovery attempt 401'd with ERR_TIMESTAMP_OUT_OF_BOUNDS. Recovery could not have succeeded once. - Every non-200 was `rejected`, which tells the user to sign in again and revoke the key. That is the wrong action for a 503 `AUTH_UPSTREAM_UNAVAILABLE`, a 429, or a 404 from a relayer too old to serve the route — the key is fine, and re-registering costs gas for nothing. `rejected` is now 401/403 without the upstream-unavailable marker; everything else is `unavailable` and retried. - Every `loginFlow` minted a keypair and overwrote `login-pending.json`. Recovery only runs at process start and is skipped for `--login`, so a timed-out login followed by `memwal_login` in the same process replaced the only copy of a key the browser may already have paid to register. A still valid record for the same relayer is now reused, `createdAt` included so the TTL keeps measuring from the attempt that may have registered it. - Logout cleared `credentials.json` and left the pending record, so the next start recovered from it and signed the user back in. Both logout paths now clear it. Deliberately not folded into `clearCreds`, which also runs on 401 session teardown where a newer stranded key is what recovery still needs. - `savePendingLogin` swallowed write errors, restoring the original loss with no log line. It now logs and throws: the invariant is that the key is durable before its public half can reach a browser, and a directory that cannot take this file cannot take `credentials.json` either — that login was going to fail at the callback anyway, one paid `add_delegate_key` later. On the reviewer's suggestion to exempt `GET /api/whoami` from the testnet `x-account-id` requirement: that gate is not policy. The registry scan behind it runs over Sui JSON-RPC, which testnet no longer serves (auth.rs "Strategy 3"), so exempting the route would only route it to a retired endpoint. Documented as mainnet-only in the API reference instead. Also reattached the orphaned `whoami` JSDoc and skipped the pending-record mode assertion on Windows, where the bits are not enforced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HsS2mBzMfpzy3QE8EMiKvS * docs(relayer): drop the em dash from the whoami network 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 * fix(mcp): stop pointing a failed sign-in at revoking the key it can reclaim Review follow-ups. The logout comment justified keeping `clearPendingLogin` out of `clearCreds` by saying `clearCreds` also runs on 401 session teardown. It does not: this tree deliberately refuses to wipe credentials on a relayer 401 (creds-wipe DoS, bridge.ts). The two logout paths are its only callers. Reworded to the reason that is actually true — discarding a reclaimable key is a decision only an explicit sign-out gets to make, and `clearCreds` is exported. Renamed the test that repeated the same false claim. The login-timeout message told the user to remove unused keys from the dashboard. Write-ahead exists precisely so that key survives, and revoking it destroys what the next start would reclaim. It now points at the two paths that work: run login again and the same key is reused, or restart and it is reclaimed. The `rejected` stranded notice had the same defect in the other order, telling the user to sign in again and then revoke. With keypair reuse that is self-defeating: the sign-in adopts that very key. Reworded. `superseded` keeps its revoke advice, which is correct there — that record is cleared, so the key really is dangling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HsS2mBzMfpzy3QE8EMiKvS * docs(mcp): move the WALM-332 note into the unreleased 0.0.13 section Review asked for a 0.0.12 -> 0.0.13 bump across package.json, the verify script and the six plugin/marketplace manifests. origin/dev has since done exactly that itself, for WALM-480: every manifest, the verify script and both changelogs are already on 0.0.13, and npm still has `latest` at 0.0.12 with only a `0.0.13-dev.0` prerelease published. So 0.0.13 is open, not released, and this change belongs in it rather than in a further bump. Merged dev and moved the #793 note out of the shipped 0.0.12 section into 0.0.13. docs/mcp/changelog.mdx gets the same entry, plus the release summary and the `answer` frontmatter the reviewer flagged as missing. Also capitalizes Testnet in the whoami note, the one style-guide violation still outstanding from the docs audit. * docs(mcp): apply the style-guide wording, and fix three stale comments Review nits. The audit re-ran on 71981f4 and still flagged the 0.0.13 bullet: `on-chain` -> `onchain` (docs run 155 to 19 that way), and two passive constructions. Applied to `packages/mcp/CHANGELOG.md` and `docs/mcp/changelog.mdx` together so the two stay byte-identical, and to the `answer` frontmatter, which carried the same hyphenation the audit does not scan. Three comments still described advice ab1636b replaced: - `recovery.ts` — the denied/unavailable split justified `rejected` by advice ("sign in again, then revoke the key") that the branch no longer gives. The reason still holds under the new copy, so it now states that one. - `recovery.ts` — `formatStrandedLoginNotice`'s JSDoc called revocation *the* actionable step. It is now the abandon path only; naming the key serves both. - `bridge.ts` — the logout comment pointed at "the relayer-401 handling below". It is above: the module doc, and the SSE 401 path. `superseded` keeps its revoke advice, which is correct there. No user-facing string changed, so the tests pinning the unavailable copy are untouched. * fix(mcp): send an approved stranded key to a restart, not a revoke or a retry loginFailureNotice and the troubleshooting page still told the user to remove the key from the dashboard and sign in again. That revokes the only copy a restart would reclaim. "Sign in again, the same key is reused" is not a fix for an approved key either. ConnectMcp.tsx always sends add_delegate_key, and the contract aborts on a key that is already registered. So the notice, the timeout reason, the bridge warning, the rejected-recovery notice and the troubleshooting page now split on whether the wallet step was approved: approved means restart to reclaim, not approved means sign in again, and the dashboard is only for abandoning the key. On Testnet, where reclaim cannot confirm the key, remove it first and then sign in. The notice and the rejected-recovery notice are pinned by tests. * fix(mcp,app): stop the failure surfaces retrying a key only a restart reclaims Two surfaces still contradicted the notice they sit next to. auth-required appended the generic LOGIN_INSTRUCTION after a failed sign-in, so the same blob said "restart the MCP client and it is reclaimed" and "no terminal command, no client restart", and led with memwal_login for a key that cannot be registered twice. A failure now gets a retry instruction scoped to the case it fixes: the wallet step was never approved. The concatenated blob is pinned so it cannot say both. The dashboard's callback-failed card told the user to sign in again and remove the unused key, which throws away the registration a restart would reclaim. It now points at the restart, and at the dashboard only for abandoning the key, matching the troubleshooting page it links to. --------- Co-authored-by: Le Tien Phat <91601109+Niko1444@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The session instructions are what actually steer an agent, and they still described only memwal_remember as returning ACCEPTED-but-not-saved. Now that memwal_remember_bulk behaves the same way, an agent reading this would take a pending batch for a stored one and tell the user their facts were saved. Also names the two things that only apply to a batch: its writes are stored one at a time rather than together, so it takes longer than a single fact, and memwal_remember_status accepts job_ids to settle the whole batch in one call rather than one id at a time.
…econd Apalis attaches no retry/backoff layer, so a retriable upload error is re-polled almost immediately. Production shows what that costs: one job took attempts 2, 3, 4 and 5 against Walrus `503 Too Many Requests` inside a single second, rotated through four wallets, and died as "exhausted retries" about a second after its first failure. The upstream limit is time-based, so rotating wallets cannot help — only waiting can, and nothing was waiting. Reuse the existing `backoff_duration` schedule (2s, 4s, 8s, 16s) between upload attempts, the way the lock-defer path already does. The wallet slot is held across the sleep on purpose: a backing-off job is still that wallet's turn, and releasing it would invite another job onto a wallet about to retry. `upload_retry_backoff` returns None for an aborting error and for the final attempt, so nothing sleeps when no retry is coming. 19 of the 24 hours of upload failures sampled were this one rate-limit, and roughly 23% of jobs reached a second attempt.
`memwal_remember` now returns as soon as the relayer accepts the job, which leaves a window where the write still dies — a SEAL encrypt outage, an exhausted upload budget — with nobody listening. `memwal_remember_status` answers for one job, but nothing obliges an agent to ask, and storing a memory is typically the last thing it does in a turn. An unasked question is the same as a silent loss, and for a product whose promise is durable memory, silent loss is worse than slow. Recall is the call an agent always makes, so the bad news rides along there. `/api/recall` now carries `failed_writes` — this owner's writes that reached `failed` in the last 24h, capped at 5 — and the MCP tool renders them as a warning naming the job, the namespace and the relayer's own error text, so the caller can judge whether re-sending will work. Scoped to `AuthInfo.owner`, never request input. Bounded by window and limit because recall is the hottest authed route; `remember_jobs (owner, status, updated_at DESC)` from migration 006 already covers the predicate and ordering. A lookup failure degrades to "nothing to report" rather than failing the recall it is attached to. Reports repeat until they age out. Suppressing after one sighting would put the burden back on the caller remembering to act, which is the failure mode this removes. `failed_writes` is skipped when empty, so an older client and an older relayer both see exactly today's response.
`memwal_remember_status` advertised `waitMs` up to 60000. The MCP SDK times a request out at `DEFAULT_REQUEST_TIMEOUT_MSEC` — also 60000 — unless the caller overrides it, so a caller using the documented maximum always lost the race: the client gave up first and the agent saw `MCP error -32001: Request timed out` with no way to tell a slow write from a broken tool. Found by driving the real tools against the production relayer rather than a mock: `waitMs: 60000` on a three-job batch reproduced it every time. The ceiling is now 45s, which leaves headroom for the round trip and still covers the median write, and the tool description interpolates the constants instead of restating them, so the advertised range cannot drift from the schema again. A job outliving the wait is not lost — the job_id stays valid and the caller asks again, which is why this tool is separate from the write. Also start the recall failure report concurrently instead of awaiting it after hydration. The published SDK aborts a recall after a hard 15s that no caller can raise, and a live recall was measured landing on exactly 15.0s, so the report has to cost the critical path nothing. Adds a regression test asserting the tool's advertised maximum keeps at least 10s of headroom under the SDK's request deadline, and a live end-to-end script that walks remember → status and bulk → job_ids → blob_ids against a real relayer, since the mocked tests cannot catch a client/tool deadline collision.
…anded The live check demanded a blob_id for every job in a 45s window. Measured write latency is p50 ~34s and p90 ~65s, so that window legitimately expires with writes still in flight — the check was asserting the relayer be fast rather than the tool be correct. What must hold is that every job comes back with a definite state, that the count of blob_ids matches the count reported saved, and that a report with nothing saved says so rather than reading as success.
…cepts The bridge carries its own copy of the tool list to answer tools/list before the relayer session is up. Two entries had drifted from the sidecar and both break a real flow during that window. memwal_remember_status advertised only `job_id`, marked it required, and set additionalProperties:false — while the sidecar takes `job_id` OR `job_ids`, and the pending body memwal_remember_bulk returns tells the agent to come back with `job_ids=[...]`. A schema-validating client refuses that call, so the instruction the tool itself gives is unfollowable and a batch cannot be settled at all until tools/list_changed lands. memwal_remember_bulk still carried its pre-fast-return description, with no hint that a result can come back ACCEPTED-but-not-saved. An agent reading it reports a queued batch to the user as stored. Neither id field is marked required: the sidecar rejects both-at-once and neither-at-all, which JSON Schema cannot express here without a `oneOf` some clients mishandle, so that stays a handler check. tool-definitions.test.mjs only ever compared the bridge against literals, never against the sidecar — which is why the drift was invisible. It now pins the parts an agent acts on: that a batch can be settled, and that both write tools admit a result may not be saved yet. Verified against the old definitions, where both new tests fail.
Observed against the live relayer while timing the flow: settling a batch printed 4. [still uploading] job_id=2868f48f… error=polling timed out after 45000ms `waitForRememberJobs` stamps that message on every row that had not landed when the budget ran out. It is our clock expiring, not the job failing — the write is still on its way — but rendering it as `error=` next to "still uploading" tells the agent the opposite, and the agent tells the user. Only a terminal row (failed / not_found) explains itself now. Also re-syncs the bridge's advertised waitMs ceiling, which drifted again in the other direction when the sidecar lowered MAX_STATUS_WAIT_MS to 45s. The bridge still advertised 60000, and a caller taking it at its word got MCP error -32602: Number must be less than or equal to 45000 That is the same failure shape as the job_ids drift, so it gets the same treatment: a test pinning the bound rather than a one-off correction.
…lyze writes The sweep I added keyed only on `preparation_encrypted_b64 IS NULL`, on the assumption that column marks "preparation never finished". It does not. It is written in exactly one place — spawn_prepare_remember_job, the SINGLE remember path. `/api/remember/bulk` and `/api/analyze` insert their rows directly and never write it, so for those two the column is ALWAYS NULL, healthy or not. The predicate therefore matched every bulk and analyze job that sat in `pending` past the 10 minute TTL — which is ordinary, not pathological, since WALRUS_UPLOAD_PER_WALLET_CONCURRENCY defaults to 1 and the queue is minutes deep under load. It would have marked live paid writes `failed` while their upload was still queued, and the recall failure report would then have told the user to send them again. Worse than the gap it was meant to close. `prepare_claimed_at IS NOT NULL` is the missing half: only claim_remember_preparation sets it, and only the single path calls it (analyze passes prepare_claim_token: None). Together the two columns mean "this row claimed a preparation slot and never redeemed it", which is the state that actually has nothing left to resume it. Orphaned bulk and analyze preparations stay unswept. That is the pre-existing behaviour, left alone deliberately rather than guessed at: neither path persists anything that separates stranded from queued, so sweeping them needs a durable marker they do not have yet. Two tests seed exactly the row shape those endpoints write — pending, no claim, no preparation, well past the TTL — and assert it survives. Also repairs the build: RecallResponse gained `failed_writes` but two test initializers in routes/recall.rs were not updated, so `cargo test` did not compile on this branch at all (3 errors, present before this commit).
…y is free Two defects with the same shape: a message written for the single-remember path was reused where its guarantees do not hold. SECURITY — the accepted-then-failed report attached to every recall read `remember_jobs.error_msg` straight out of the table. Every other client-facing view of that column (`GET /api/remember/:job_id`, `POST /api/remember/bulk/status`) runs it through `sanitize_job_error_for_client` first, which exists to do two things: replace an infrastructure-funding failure with INFRA_JOB_ERROR_MESSAGE, and redact long hex runs. Skipping it meant a WAL shortfall published the relayer's own hot wallet and its exact balance — "Insufficient balance of 0x356a26…::wal::WAL for owner 0x8d3c1f0a…c0d. Required: 64367730, Available: 10708877" — to every tenant whose write landed on that wallet, on every recall, for 24 hours. It also reads to an agent as "top that address up", which is the precise scam confusion INFRA_JOB_ERROR_MESSAGE was written to prevent. The repo already asserts this cannot happen (infra_wal_balance_failure_hides_relayer_wallet_address); that assertion was simply never extended to this path. Sanitized at the recall boundary rather than in storage, since `routes` is not reachable from the lib. CORRECTNESS — `withAcceptDeadline` emitted one message for every caller, ending "the SDK reuses the same idempotency key, so a retry attaches to the existing job instead of queueing a second paid copy". True for `POST /api/remember`, which carries a content-derived key. False for `POST /api/remember/bulk`, which carries none: the handler mints a fresh uuid per item and inserts with no conflict clause, so a retry is N more paid Walrus blobs for the same N facts. The deadline makes that near-certain rather than merely possible — `withDeadline` deliberately does not cancel the request, so when it fires the relayer has usually accepted already. The advice is now chosen per path, and bulk is told to check `memwal_recall` before re-sending anything. Tests pin both directions, because collapsing the two messages into one is how this happened: bulk must never claim idempotency it does not have, and the single path must keep saying a retry is safe.
`deadlineSignal` unref'd its timer, which reads as tidy and is exactly
wrong for this timer: the deadline is the one thing a caller IS waiting
on. Unref'd, it stops the clock the moment nothing else holds the event
loop open, and the stalled request it exists to bound then hangs forever
— the bug the deadline was added to prevent.
CI caught it as six cancelled tests in request-timeout.test.mjs
("Promise resolution is still pending but the event loop has already
resolved"): the stub fetch is a bare promise with no socket behind it,
so the loop drained and the deadline never fired. A real socket normally
keeps the loop alive, which is why this survived manual testing — but
"normally" is not a bound, and any caller whose transport does not ref
the loop inherits the unbounded hang.
Both call sites already run `dispose()` in a `finally`, so a ref'd timer
cannot outlive its request either.
`a_queued_bulk_job_is_never_swept` and `a_queued_analyze_job_is_never_swept` failed against a sweep that is correct. The helper was the problem: it stamped `prepare_claimed_at` on every seeded row, claim token or not, so a row meant to stand in for a queued bulk write carried the one column the orphan pass keys on and was duly failed. `/api/remember/bulk` and `/api/analyze` insert neither the token nor the timestamp; only the single-write path claims, and it writes both at once. Stamping the timestamp alone is a shape the database never holds, so the two tests were asserting against a fiction while the sweep they guard went unexercised. Seed the timestamp only alongside a token. Every row that must be swept already passes one, so the orphan-preparation tests are unaffected. Verified by `cargo check --tests`; the DB tests themselves need a pgvector Postgres, which CI has and this machine does not.
The cold-start tool list gained `memwal_remember_status` so a client that keeps the first `tools/list` can resolve the job ids `memwal_remember` and `memwal_remember_bulk` now return. The login-handoff test pins that list exactly, and was never updated, so it failed on the tool being present rather than on anything being wrong. Title and annotations are copied from the server definition (read-only, non-destructive), so the assertion keeps pinning the metadata clients receive rather than just the name.
Observed live against production while benchmarking: once the per-delegate-key
budget (60 weighted requests/minute) was spent, memwal_remember failed four
times in a row with
Tool error: Walrus Memory server error (429): {"error":"Rate limit exceeded",
"layer":"delegate_key","limit":"60 weighted-requests/min","retry_after_seconds":60}
and the facts were never written. Nothing retried, nothing honoured the
advertised cooldown, and nothing told the user a memory had been dropped. That
is the quietest failure this system has.
Fast-return makes it likelier rather than rarer: settling a batch adds requests
on top of the write, so an agent saving several facts in one turn spends the
budget faster than one that blocked.
A short cooldown is now absorbed (503 AUTH_UPSTREAM_UNAVAILABLE advises ~5s),
and a long one is reported. Sleeping out a 60s cooldown inside a tool call
would just be the hang this branch exists to remove, and the MCP client would
time out first — so the message names the wait, states plainly that the fact
was NOT saved, and points at the cheaper shape: one memwal_remember_bulk
instead of N memwal_remember calls, one memwal_remember_status(job_ids) instead
of N status calls.
Only rejections that provably never reached the handler are retried — 429 from
the limiter, AUTH_UPSTREAM_UNAVAILABLE from the delegate-key lookup. A 500 is
left alone: it could have been thrown after a write started, and
/api/remember/bulk has no idempotency key, so retrying it would store every
fact twice.
The absorb budget lives inside the accept deadline (8s of 15s) because
withAcceptDeadline wraps this; a test pins that ordering, since growing the
budget past the deadline would turn every absorbed retry into a spurious
"did not accept".
… claim TTL `memwal_remember` tells an agent to send a failed fact again. The derived idempotency key collapses that retry onto the failed row, and `claim_remember_preparation` refused the claim for 60s — while the handler answered 202 ACCEPTED regardless. The caller was told the write was durably queued while nothing at all was running, which is the one thing this branch's whole pending-vs-saved contract exists to prevent. The TTL protects a preparation that is still RUNNING. A job that reached `failed` has none: its preparation either errored on its own or the stale sweeper failed it and cleared its token. So `status = 'failed'` now bypasses the age check. That is safe because fencing is done by the token, not the clock. A claim rotates `prepare_claim_token`, and a straggler's own write is `WHERE ... prepare_claim_token = <old>`, so it matches zero rows and returns before `enqueue_wallet_job` — it cannot mint a paid blob for a job someone else has re-prepared. The test drives exactly that: re-claim a just-failed job, then watch the previous token's UPDATE affect nothing. Second half: stop answering "pending" when no claim was taken. Losing the race now means a concurrent retry won it, so the handler re-reads and reports what that winner actually left behind instead of asserting a state it never reached. A companion test pins that the TTL still does its real job — a `pending` row claimed a moment ago keeps its claim.
analyze was the last tool still blocking to terminal, which left it the slowest in the set by a wide margin: 37.0s measured against dev in the same session where memwal_remember had dropped to 0.2s there. Its wait has the same shape as bulk's — N Walrus writes, one upload per wallet — so there was no reason for the answer to be shaped differently. Extraction is still waited for. `analyze()` resolves once the LLM has run, so the facts it found lead the reply, which is the half an agent can act on straight away. Only the upload of those facts is handed back as job_ids, each paired with its fact so a later partial failure is actionable. Text that yields nothing says "Extracted 0 facts — nothing was saved" rather than handing back an empty batch and a status tool to call about it. 6 new tests. Suite: 302 run, 269 pass, 33 fail — the same 33 sandbox port-binding failures present on dev. tsc --noEmit clean.
…rk its wallet busy Two findings from the completeness pass over this branch. REPORT ONCE. The accepted-then-failed report attached to recall selected on owner + status + a 24h window with nothing recording that a row had been shown. So the same failures rode along on every recall for a full day while the message told the agent to send those facts again — a compliant agent re-sent, the original row stayed `failed` and in-window, and the next recall asked for the same thing. Every pass was another paid Walrus write, and `collapseDuplicates` on the read side hid the pile-up from the user. Rows are now stamped in the same statement that returns them, so the claim and the report cannot separate; two concurrent recalls cannot both surface the same failure (`FOR UPDATE SKIP LOCKED`). Migration 021 adds the column plus a partial index matching the query — failed and unreported only, so it stays small on a table that is never pruned. MARK THE WALLET BUSY. `least_loaded_index` answers "is this key signing right now", and only `UploadAndTransfer` was telling it. `SetMetadataAndTransfer` signs on the very same wallet and held no guard, so a key mid-transfer read as idle — and join-shortest-queue then steered new uploads onto it *because* it looked free, queueing them behind the transaction. That is precisely the failure the least-loaded change was written to remove, so the arm that regressed it is the arm that had to be fixed. It guards `enqueued_wallet_index`, not a fresh pick, because the operation must run on the key that already owns the blob object. `FinalizeUploadedBlob` deliberately stays unguarded: it only inserts the vector row, signs nothing, and holding a wallet slot through a DB write would make an idle key look busy. No new unit test for the guard: the pool's selection logic is already covered (a_busy_wallet_is_skipped_for_an_idle_one), and what broke was a missing call site, which only an integration test over the job arms could catch.
…egacy one
The backoff added earlier covered one of the upload job's three retryable
exits. Dev proved it was the wrong one: a burst of 12 concurrent writes
produced
14:33:47 job_id=423e8f6f… durable Walrus upload request failed:
error sending request for url (localhost:9000/walrus/upload-step-v3)
classification=transient retryable=true
14:33:47 selected wallet for attempt: attempt=2/5
— the retry starting in the same second, with no `backing off` line anywhere
in 24h of dev logs. The durable upload path returns through
`execute_upload_and_transfer_locked`, which has its own exit and so needed its
own spacing; only the legacy sidecar exit had been given one.
The contrast in the same trace is the point: the lock-defer path, which has
had a backoff all along, spaced its retry correctly —
14:33:54 deferring upload (attempt 1/5)
14:33:56 (retry) ← 2.0s, matching backoff_duration(1)
Still not covered: `insert_vector_and_mark_remember_done` takes no
`attempt_info`, so its exit cannot space its own retry without threading the
attempt through. Left alone rather than widened blindly — it fires after the
blob is already minted, which is a different failure shape.
a4a94e4 corrected this in the SDK; the sidecar's `withDeadline` had the same line and was missed. It matters more here, not less: the sidecar ships inside the relayer image, while the SDK copy sits behind a pinned published version, so this is the deadline actually running in production. The reasoning was wrong in the same way. `unref()` is right for a background sweeper nobody awaits — which is where it was copied from — but a request deadline is the one timer somebody IS waiting on. Unref'd it stops firing as soon as nothing else holds the loop open, so the stalled request it exists to bound hangs forever, which is precisely the failure it was added to prevent. `dispose()` still runs in the caller's `finally`, so the timer cannot outlive its request either way.
…s by hand Review items on this head. KEEP THE WAIT. `DEFAULT_REMEMBER_WAIT_MS` was 0, so `memwal_remember` returned at accept with no blob_id. D1 already decided the opposite: a result means the fact landed. Returning at accept is a product change on its own ticket, and the poll-cadence work it was bundled with belongs to #902. The default is the full 90s ceiling again; accept-and-continue stays reachable at `MEMWAL_MCP_REMEMBER_WAIT_MS=0` for an operator who chooses it, and the two suites that cover that path now opt in explicitly instead of inheriting it. A budget between zero and the real completion time stays the setting to avoid — against a 30-75s spread it pays the wait and still returns pending — so the default is the ceiling rather than something in between. MAKE THE IDEMPOTENCY CLAIM TRUE. The accept-timeout message told callers a retry was safe "because the write carries a content-derived idempotency key". That key exists in packages/sdk, which is NOT what runs here: the sidecar installs published 0.1.7, whose rememberAsync mints a crypto.randomUUID() and caches it per client instance — and a fresh client is built per transport session, so a retry after a reconnect stored the fact a second time at full cost. Rather than soften the wording, the tool now computes the key itself and passes it, which 0.1.7 accepts. The promise is true in the version actually deployed. Bulk still says the opposite, because /api/remember/bulk takes no key at all. DO NOT ASK FOR TEXT WE DO NOT HAVE. The recall failure report told the agent to send the fact again, but the relayer stores only SEAL ciphertext, so a failed write's wording is gone. It now says so and asks the user to restate it, rather than inviting the agent to invent it. (The repeat-every-recall half was fixed in 8f923d0 — the report is one-shot.) RELEASE BY HAND. The MCP contract, tools and transport changed under published 0.0.13, and the SDK's poll, timeout and idempotency behaviour under 0.1.7. Both bumped manually: 0.0.14 and 0.1.8, dual changelogs, every manifest the release verifier checks, and the verifier's own pins. The two changesets are deleted — this repo releases these by hand, so leaving them would have double-bumped. `node scripts/verify-manual-sdk-release.mjs` passes for all four packages, including the plugin npx pins it caught me missing in .mcp.json, .cursor-mcp.json and .codex-mcp.json.
I had changed the backoff here as part of the latency work. #902 (WALM-623) already owns it, does it better, and corrects a fact I had wrong. Its version returns 0 for attempt 0 — the immediate first check I had built by restructuring both wait loops — inside `pollingDelayMs` itself, which is the right place. Its ceiling is 3s against a documented 30 weighted-requests/min quota for status GETs. I had read 60/min off a 429 body and picked 2s, not accounting for status reads being weighted 2, so my cap sat closer to the quota than the one written by someone who knew the weighting. It also moves the function to its own module and applies the same fix to the Python SDK. Keeping my copy would have meant two implementations of one policy, and a conflict on merge: #902 deletes the function my change edited. Reverted here: the 2s ceiling, the 600ms base defaults, and the check-first restructuring of both loops. The latency tests that pinned that behaviour go with it — #902 pins the same thing in test/polling-delay.test.mjs, and asserting it twice would leave one copy silently wrong the next time the cap moves. What stays in that file is what this PR actually owns: the derived idempotency key. The 0.1.8 changelog drops its poll bullet to match. What this PR keeps in the SDK is unrelated to cadence: per-request deadlines (#918's own finding — `fetch` had none, so a stalled socket outran `timeoutMs` entirely) and content-derived idempotency keys. Verified against #902: the two branches now auto-merge, with no conflicting hunks in memwal.ts or either changelog.
Restoring the wait changed which outcome is normal, and three things had not caught up with it. COPY. The session instructions — in both the sidecar and the bridge's own copy — still told agents that `memwal_remember` "normally returns a job_id with the fact ACCEPTED but NOT YET SAVED" and that this "is the healthy path". Since the default went back to blocking, the healthy path is a blob_id and the pending result is the exception. An agent reading the old text would report a stored fact as merely queued, which is the mirror of the failure the pending wording was written to prevent. Both instruction copies and all four tool descriptions (sidecar and the bridge's cold-start set) now lead with the blob_id and describe the job_id as what happens when a write outruns the budget. ANALYZE. `memwal_analyze` ran its extraction under the 15s ACCEPT deadline. That number is sized for an accept — measured ~1.1s — but `/api/analyze` runs the extractor LLM inline before it answers, which is exactly why the SDK allows that call 60s where it allows a remember 30s. Any transcript long enough to be worth extracting from would have been cut off mid-LLM and reported as "did not accept". `withAcceptDeadline` now takes a ceiling and analyze names 60s. DROPPED JOB_ID. `withWaitDeadline` raises MemWalRelayerUnresponsive, which is not status 504, so `isStillRunning` was false and the error fell through to `throw` — discarding the job_id of a write that had been accepted and was still running. That id is the only way to settle the job, and carrying it is the entire reason the pending result exists. Our deadline firing means the relayer went quiet on US, not that the job stopped, so it now reads as still running. A test drives an accept that succeeds followed by a poll that never answers, and asserts the id survives.
The test I added in 68fd4c4 proved nothing. It lived in remember-deadline.test.ts, which sets MEMWAL_MCP_REMEMBER_WAIT_MS=0 at the top so the whole file can exercise the accept leg — and with a zero budget `memwal_remember` returns at accept and never enters the wait at all. It passed against the bug it was written for. Caught by reverting the fix and watching the test stay green, which is the check I should have run before claiming it covered anything. Moved to its own file that keeps a real budget, and driven through the error `withWaitDeadline` actually raises rather than through a hang, so it runs in milliseconds instead of waiting out budget-plus-grace. Verified both ways this time: it fails with the fix reverted and passes with it restored. Two guards against the same mistake recurring. The file asserts its own budget is non-zero, because every case in it is vacuous otherwise. And a companion test pins the opposite direction — a job that genuinely failed (500) must stay an error rather than being laundered into "still uploading" by the branch that now forgives our own deadline.
…ransport doc Three of the review's suggestions. The two bugs among them: BULK SUMMARY. `result.failed` is total-minus-succeeded, so a write still uploading was counted as failed — while the tail of the same message said that job was on its way. An agent reading `failed=` re-sends an in-flight write, which is the duplicate this branch exists to avoid. The summary now counts only terminal failures and names the in-flight ones separately. TRANSPORT DOC. `MEMWAL_MCP_TRANSPORT=http` was documented as having "no replay-on-reconnect". It does not: `openRelaySession` hands the Streamable session to the same `runBridge`, which replays its in-flight map on either transport — and on Streamable a disconnect mid-send can look sent, so a replayed write may duplicate. Making replay transport-aware is a real change to the reconnect path and does not belong in this PR, so the doc now says what the code does and warns what opting in costs. Also trims the comments the review called out — the sweeper block, FailedWrite, and the SDK poll/idempotency essays. Kept the load-bearing constraints (`prepare_claimed_at IS NOT NULL` so bulk and analyze rows are not swept; why the idempotency bucket exists; that a poll loop only checks its budget between polls) and dropped the narration. Not addressed, because it is not mine to do: the suggestion to split this into four PRs. It is the right call — the remember-contract piece is the one D1 deferred, and the transport rewrite is independently risky — but resequencing someone else's PR needs the author.
Two fixes from this branch are each correct alone and wrong together. `claim_remember_preparation` resets a `failed` row for a fresh attempt: status back to `pending`, `error_msg` cleared. The recall failure report fires once per ROW, keyed on `failure_reported_at IS NULL`. The reset did not clear that column, and the row is reused across attempts — so: write fails -> recall reports it -> user re-sends -> row re-claimed and reset -> retry fails again -> next recall skips it, because the row was already marked reported. The second failure is never surfaced. That is silent loss, which is the exact thing the report was added to prevent, reached by way of the retry the report itself asks for. A re-claim now clears `failure_reported_at` alongside `error_msg`, under the same `blob_id IS NULL` guard: a row being re-prepared is a fresh attempt, so its report state belongs to the previous one. Found by asking what happens when the claim-TTL change and the one-shot report touch the same row, rather than by reading either in isolation — neither file's own review would show it.
…return perf(mcp+relayer): stop memwal_remember blocking on the Walrus write, and stop queueing on a busy wallet
harrymove-ctrl
had a problem deploying
to
benchmark-dev
September 17, 2026 04:08 — with 
GitHub Actions
Failure
GitHub Actions
Failure
harrymove-ctrl
had a problem deploying
to
benchmark-dev
September 17, 2026 04:15 — with
GitHub Actions
Failure
GitHub Actions
Failure
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
3 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.
Forward merge of
devintostaging. 40 commits, ~6900 insertions across 85 files. Clean:git merge-treereports 0 conflicts, andgit diff dev...stagingis empty — staging carries no content of its own, only the five priorMerge pull request ... from devcommits.devis red right now — 3 failing checks onde98e6cf:Server / Clippy + Unit testsSDK / E2E (live relayer)Memory API LatencyThe one that matters for staging
Migration
021_failed_write_report_ack.sqlwas never wired into theinclude_str!chain inVectorDb::new(), soremember_jobs.failure_reported_atdoes not exist on a fresh database — whilerecent_failed_remember_jobs(every recall) andclaim_remember_preparation(every remember) both query it.dev only survives this because someone ran 021 by hand on its database. Staging will not have that. On a staging deploy the claim path 500s with
?and the recall path silently returns an empty failed-write report. #921 wires the migration into the chain (and intoidem_test_pool, which builds its own schema and would otherwise still fail).#921 also carries three correctness fixes that shipped broken in #918 and would land here as-is:
memwal_analyzeprintingfailed=Nfor writes that are still uploading, directly above the block telling the agent not to re-send them — an agent that believes the count queues duplicate paid Walrus blobs.memwal_remember_bulkandmemwal_analyzediscarding everyjob_idwhen the relayer goes quiet mid-poll (analyze also discards the LLM extraction already paid for).tokio::spawnbefore the recall could deliver it, so an embedding 429 or a client abort destroyed the only warning a user ever gets that their save failed.The other two are not code
Both fail at
Wait for Railway deploy; their credentials step passes. The dev relayer is serving877a48f9while dev's head isde98e6cf:A redeploy was attempted and the served commit did not change. If that Railway service is set to wait for CI before deploying, this is circular — the migration bug keeps dev red, which keeps the deploy from going out, which keeps these two jobs failing. Landing #921 would break the loop.
What is in this merge
Almost all of it is the
memwal_rememberlatency work plus its follow-ups.Write path / latency
memwal_remember,memwal_remember_bulkandmemwal_analyzereturn on a bounded budget instead of blocking on the whole Walrus write, handing back ajob_idwhen the write outruns it (600632ad,63deb0e5,d83b9007,b793b3fe).memwal_remember_statustool, andjob_idsbatch settling (524c3e80,df314c3e).recalldid (62fd9542,9385153e,10ba9e1c).retry_afteron 429/503 instead of dropping the fact (e903f43c), spaces out upload retries (a3531141), and picks the least-loaded upload wallet (af92c910).956de95d).Correctness / safety
f7ee4ccf).fa918d3f).0c765a03, WALM-332).c1b2561a, WALM-470).7cbe9520, WALM-608).Measured on dev (n=9, SDK direct): accept 0.14-0.16s, time-to-saved 26.7-47.9s, recall 4.5-7.0s, save rate 9/9. The Walrus write itself is unchanged — what changed is that the tool no longer hangs unboundedly or retries a write that already landed.
Suggested order
dev.devgoes green and Railway deploysdevhead (check/health build.commitand thatGET /api/whoamistops 404ing).Merging before step 1 puts the migration bug on staging, where no manual
021run exists to mask it.