Repository navigation
Promote dev → staging - #1056
Merged
Merged
Conversation
…M-682)
`assertPublicUrl` resolved a user-supplied hostname, confirmed every answer
was public, then returned the URL and threw the addresses away. `fetch` then
resolved the same name a second time, and nothing tied the two answers
together. A host that replies with a public address to the check and a
private one to the connection walks through — DNS rebinding, which the old
docstring named as a known gap. Every redirect repeated it.
`fetch` cannot express the fix: it takes a URL and does its own resolution.
Send the request with `node:http`/`node:https` instead, which take the
connect address and the TLS identity separately:
- the socket goes to an address the check approved,
- `Host` stays the original host, including a non-default port,
- `servername` stays the hostname, so SNI and certificate verification are
unchanged and pinning costs nothing in TLS terms,
- an IP literal gets no SNI, because the certificate must then carry the
address itself; `rejectUnauthorized` stays on either way.
The second resolution is simply gone, so there is no longer a window to win.
Redirects keep being followed by hand and now re-pin as well as re-check on
each hop. Redirect bodies are cancelled rather than left streaming, and a
30s timeout replaces `fetch`'s absence of one.
`fetchPublicUrl` takes an injectable transport. Two existing tests staged a
public first hop by stubbing `globalThis.fetch`, which this no longer goes
through; the seam lets them drive the real redirect loop instead of a
monkeypatched global.
Not addressed here: the ticket's preferred shape, a server-issued upload id
replacing the arbitrary download URL. That removes the surface rather than
fencing it, but it changes the chat file flow, so it stays open.
Verified: 20 tests in this file, the 15 that existed plus 5 for the pin —
including one that dials a loopback server through a URL naming example.com,
where the handler running at all is what proves the name was never
re-resolved. Researcher suite 49 pass (44 before), build clean.
…ated addresses (WALM-682) Review follow-up on #984. `new Response()` ran inside the http/https callback with no guard. It throws on a status outside 200-599 and on a reason-phrase that is not HTAB/SP/VCHAR/obs-text, and a throw there is an uncaughtException — the process exits by default — while the promise never settles, because the status line is already parsed and the socket timeout no longer applies. A chat pdf-url pointing at a server answering `HTTP/1.1 600` could take the worker down. Out-of-range statuses now reject with ChatbotError and destroy the response, an invalid reason-phrase becomes "", and the construction itself is wrapped. 101/103 leave NULL_BODY_STATUS since they can no longer reach it. Only `addresses[0]` was dialled. Resolvers often put AAAA first, and pinning the connect host also switched off the `autoSelectFamily` fallback that fetch had, so on an image without IPv6 egress every dual-stack URL failed even though a public A record was already validated. Each validated address is now tried in turn, but only past failures that happen before a connection exists — refused, unreachable, or a new 10s connect deadline. Once connected, a TLS or HTTP failure is the server's answer and is not retried. The fallback never leaves the validated list: no second lookup, and the hostname does not go back into `host`. Verified: raw-socket tests for status 600, a bare 101, and a control character in the reason phrase; all three fail on the previous commit with the RangeError/TypeError from the review and pass here. Plus tests for refused-connection classification and the fallback order. File 27 pass; researcher suite and build clean.
Enoki returns 404 not_found for an unused sponsored register. That used to stay ambiguous, so Rust kept the dead sponsor digest and the upload died after the short retry budget. A digest lookup that proves the transaction is absent now clears the journal and sponsors again. Reconcile attempts stay ambiguous.
…(WALM-682) Review round 2 on #984 (Henry). An immediate refusal already fell through the validated address list; a connect that *hangs* did not — which is the case the 10s connect deadline was added for, and what a blackholed AAAA record looks like. Node >=19 creates http(s).globalAgent with `timeout: 5000`, armed on the socket while it is still connecting. It fired first, surfaced as the request's 'timeout', and the handler rejected with a plain ChatbotError, which `fetchFirstReachable` treats as a finished fetch. The connect deadline was cleared at 5s and never ran, and the next address was never dialled. Reproduced on Node 22.23.2 (the image's runtime) exactly as reported: rejected at 5005ms with the second address untried. Two changes: - The agent's timer is disarmed (`socket.setTimeout(0)`) while the socket is connecting; `req.setTimeout` re-arms the idle timeout once connected, so the connect deadline is the one that fires. - The request timeout handler classifies by whether a connection exists: before connect it rejects with PinnedConnectError, so the next address gets its turn whichever timer fires. Same pin as before: no second lookup, and the hostname stays out of `host`. Verified: same scenario on Node 22 with default timeouts now reaches the second address after the 10s deadline. Two new tests against a TEST-NET-1 address that blackholes — fallback with a short deadline, and the agent timer shrunk to 100ms to show it no longer ends the connect as a whole-fetch failure. Both fail on the previous commit. Researcher suite and build clean.
query-blobs and verify-blob rebuilt the shared client whenever it was older than one second. Each reset drops the system-state cache, and the next parse runs synchronously on the Node main thread. The expiry sweep was doing this dozens of times a minute, so /health, encrypt, and upload-step-v3 stopped being accepted. Use WALRUS_CLIENT_MAX_AGE_MS (60s). A stale epoch only delays expiry.
…ock-pool stalls A certified blob can still read certified_epoch as null for a short window. Wait that out only after the certify transaction succeeds. A lock-pool timeout requeues on the congestion delay instead of burning the five upload attempts.
The busy-count followed the retry wallet while the sidecar queued on the journaled signer, so new jobs kept landing on the wallets that were full.
A full Walrus upload queue stalls fetchKeys on the shared event loop, so recall misses its deadline. Serve encrypt and decrypt from a child process, and signal that whole process group on shutdown so the seal port is not left bound.
Resolve services/server/src/main.rs by keeping both the PR sidecar process-group helpers and origin/dev's rate-limited mcp_routes (WALM-700).
The dev merge added rate_limit_test_config without seal_sidecar_url, so the relayer test binary no longer compiles.
0x158a78f0… is the only owner exempt. Storage quota and the IP and global sponsor limits stay in place.
A disconnect no longer starts a second upload or burns an attempt. The sidecar keeps one in-flight step per job, and each wallet admits only one upload at a time.
Percent-decode Enoki dry-run text so the existing stale-object rebuild runs. Hold the upload permit through metadata, refresh updated_at while that wait is in flight, and requeue slot congestion on the same wallet. An uploaded row keeps the Enoki error instead of becoming "stale/orphaned remember job".
/api/mcp no longer shares the accounts-exists budget. Every IP gets 2000/min and 8000/hour, with a global ceiling of 2500/min and 15000/hour.
Apalis already orders a higher priority first. An index-only finalize runs before metadata, and metadata runs before a new upload. A congestion requeue still waits until its scheduled time.
…M-682) The socket handler treated any socket that was not connecting as an open pooled socket. A connect that fails synchronously is not connecting either: libuv returns ENETUNREACH, EHOSTUNREACH, EACCES and EPERM from connect() itself (only ECONNREFUSED is deferred), and net destroys the socket before the request's 'socket' event fires. The failure was then classed as post-connect, fetchFirstReachable stopped, and the validated address behind it was never tried. Reproduced on node:22-alpine (Node 22.23): pinning to an IPv6 address with no IPv6 route rejected with a plain ChatbotError after 2ms, and [v6, v4] never reached the IPv4 address. The case this PR set out to cover is a container with an IPv6 address but no public IPv6 route. A destroyed socket is no longer counted as connected. Two regression tests use the limited broadcast address, which fails synchronously on Linux the same way; both fail before this change and pass after it.
One wallet_jobs queue stays pending until the dispatcher can claim a row for a key that is free to sign it. An unpinned retry continues from the key that claim actually received, so an idle pool does not keep resigning wallet 0. Finalize does not take a signing slot. A cold delegate key with no account id is rejected on every network. Auth no longer walks AccountRegistry, which did not finish inside a client timeout.
The dispatcher made the write benchmark slower. Wallet jobs go back to the Apalis worker. A cold delegate key with no account id still returns 401 instead of scanning AccountRegistry.
Migration 020 locks memory_tombstones and then vector_entries, while insert_vector locks them in the opposite order. The test harness replays that file on every schema setup, so CI aborts one side with 40P01. The statements are idempotent, so the aborted setup retries.
Each wallet worker was locking 10 pending jobs, so a metadata or finalize job could not pass those uploads. One locked job per worker leaves the rest pending, and get_jobs already runs priority 2, then 1, then 0.
Buffer 1 let a metadata job pass queued uploads, but the previous prefetch was faster. Wallet workers go back to the default batch of 10.
Uploads stay on the prefetch queue. Metadata and finalize use a second queue with its own workers, and the metadata transaction no longer holds the upload slot.
Restores 0ea8ac5. Uploads stay on the prefetch queue. Metadata and finalize use a second queue with their own workers, and the metadata transaction does not hold the upload slot.
A prefetched burst checked out a lock-pool connection before waiting for the per-wallet permit, then treated the pool timeout as relay congestion and parked those jobs for 30s or more.
The pool timeout is gone, but a 100-remember on the public testnet relay got slower: wallets no longer sat out the 30s backoff, the relay returned SHARED_SERVICE_UNAVAILABLE, and those retries burned the 5-attempt budget. Restore the previous lock order.
…url-check-does-not-pin-the-resolved-address fix(researcher): pin the outbound fetch to the validated address (WALM-682)
…kets The 100-request dev burst uses one account, and one delegate key allows 12 remembers a minute. Exempt exactly owner 0x8afc5559... from the request-rate buckets in rate_limit_middleware, and only when RAILWAY_ENVIRONMENT_NAME=dev and MEMWAL_DEV_BENCH_UNMETERED_OWNER equals that address. Auth, storage quota, sponsor, read-API, restore and owner-token limits are unchanged; every other owner is metered.
ducnmm
approved these changes
Sep 30, 2026
A prefetched upload waits for the wallet slot before it checks out a lock-pool connection. A pool miss retries in 1s on the same wallet. A structured SHARED_SERVICE_UNAVAILABLE parks on the 30s congestion ladder, same journal and signer, instead of the 2s five-attempt budget. Metadata encoding runs off the sidecar event loop. A lost metadata response is reconciled from the blob owner before signing again.
Register writes namespace, owner, package, and job id, then transfers the blob to the signing wallet. When that wallet is the memory owner, the metadata step died on a missing memwal_agent_id. Stamp the agent id instead. A field that is already present and disagrees stays a permanent mismatch.
Register leaves the blob with its signer. When that signer is also the memory owner, the blob already looks transferred and register's tags match a finished write, so recovery skipped the metadata step and failed the job with METADATA_RECEIPT_MISMATCH (bench-100k-20260930: every job on wallet 5). Ownership is not a receipt in that case. Sign the metadata step, which is idempotent and the only step that adds the agent id and SEAL fence. A tag that conflicts with the request still fails.
seal/encrypt reads the MemWalAccount before encrypting. A single gRPC-web "fetch failed" there failed the whole remember before any paid work (bench-w7-pr1055-204e580-100-20261001, 1 of 100). The read is read-only, so retry it with backoff. The shared retry classifier is unchanged: widening it made dropped connections in durable paths retry forever.
Takes Henry's f581043 agent-id stamping and keeps PR #1055's broader rule: when the memory owner is the upload wallet, a matching agent id proves the metadata step ran; without that proof the metadata step is signed again (idempotent), including requests with no agent id. Both test sets kept. jobs.rs conflict was formatting only.
The encrypt step reads key-server objects over the same gRPC-web transport. A bare "fetch failed" there failed a remember before any paid work (bench-w9-merged-9e9a10c-r3, 1 of 100). Encrypt has no side effects, so it uses the same bounded retry as the account read.
…onto it Choosing the least-loaded wallet and marking it busy were separate steps with an awaited journal write between them. Every worker choosing in that window saw the same idle wallet, steered its job there and pinned it. In a 100-job staging burst one wallet took 27 jobs while most took 3, and the run lasted as long as that one queue (8m28s vs 4m13s for an even spread). KeyPool::claim_least_loaded picks and increments in one compare-exchange; the steering path holds that claim as the attempt's busy slot.
The staging gate matched one hardcoded owner, so any new burst-test owner (e.g. one that is also an upload wallet) needed RATE_LIMIT_DISABLED, which unmeters every caller. Accept the exact owners listed in MEMWAL_STAGING_CHATGPT_UNMETERED_OWNER; staging only, write-path buckets only.
…r a mint (WALM-743) (#1065) * fix(app): say what the key action did in the Back to Console dialog (WALM-743) A mint from /keys showed the new-account copy ("Your account is ready"). Mint now names the key, revoke counts the removed keys, and the /setup arrival keeps its copy. * fix(app): keep the new delegate key in view after a mint (WALM-743) A first mint in the browser saves the key, which mounts the SDK credentials card above the delegate keys section and pushes the one-time key block off screen. Scroll it back into view once it renders. * fix(app): leave a gap above Delegate keys when /keys scrolls to it (WALM-743) The scroll margin matched the sticky nav height exactly, so the card landed flush under the nav with its top edge hidden. --------- Co-authored-by: Nikola Le <nikola0x0@users.noreply.github.com>
Removes every benchmark-only rate-limit exemption before this merges: - a hardcoded owner (0x158a...) that skipped write, read, restore, owner-window and sponsor limits in every environment, including production; - the dev and staging bench-owner gates (MEMWAL_DEV_BENCH_UNMETERED_OWNER, MEMWAL_DEV_CHATGPT_UNMETERED_OWNER, MEMWAL_STAGING_CHATGPT_UNMETERED_OWNER); - services/server/benchmarks/isolated-dev (bench runbook and guard script). Also restores mcp_proxy.rs, memory_read.rs and recall.rs to dev: they only carried rustfmt churn. RATE_LIMIT_DISABLED (already on dev) is unchanged.
…rker start The probe measured how long the upload process stayed unresponsive from inside its own worker, starting the clock when the worker first saw the go flag. On a slow CI runner the worker started late, so an 800ms block measured as 723ms and the test failed its own setup check (Compile & CLI Smoke). Wait for both probe workers to be ready before blocking, then compare absolute answer times with when the block ended: the upload process must answer after it, the seal listener before it.
…t VectorDb DB_SETUP_LOCK only orders test setups against each other. Migration 020 locks memory_tombstones then vector_entries, while insert_vector from a test that is already running takes them in the opposite order, so CI sometimes aborted VectorDb::new() with 40P01 (stale_sweep_tests::orphaned_preparation_is_failed). The migrations are idempotent, so the shared test helper retries the setup.
…60930 Fix burst upload admission, metadata recovery and sidecar CPU stalls
Tunggiang25890
approved these changes
Oct 1, 2026
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
5 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.
Summary
Promotes
devintostaging: 4 commits since #1044, all from #984. This carries MW-03 (WALM-682), the Researcher outbound-fetch fix, which was held back from the last promotion while it waited on re-review.Changes
Security (WALM-679)
Touches only
apps/researcher/lib/rag/ingest/safe-fetch.tsand its unit test. No relayer, SDK or app changes.Not in this promotion
fix/enoki-not-found-rebuild-registeris still open againstdev. The staging and production relayers are deployed from that branch (build.commit=79f241ec), not fromstaging/main.Test plan
staging. Ingest a public URL source and a PDF upload.staging → main) to include this commit, then re-check it before promoting