Skip to content

feat(research): follow same-site subpages in web_read - #454

Merged
lukemaj merged 9 commits into
mainfrom
feat/441-web-read-subpages
Oct 18, 2026
Merged

lukemaj merged 9 commits into
mainfrom
feat/441-web-read-subpages

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

What: web_read now includes matching same-site depth-one subpages with separate provenance and bounded continuation.
Why: Room and event facts often live beyond the homepage, so homepage-only reads miss research evidence.
So what: The parent merged this as def07e920a27c4d266b47cf15ecde4be0db70c27 and #441 is closed; dependent owners released their refs, the remote/proof branches are retired, and only this thread's active local checkout remains retained.

Closes #441. Part of the approved #423 detour and its review/user decisions.

Merged candidate

Behavior

Initial URL reads follow at most six matching depth-one anchors. Same-site is conservatively limited to the final root's origin, including HTTP and meta-refresh redirects. URL parsing canonicalizes fragments, host case, default ports and dot segments. Root cycles, duplicate URLs and aliases onto returned pages are skipped; children are not traversed. Redirects reuse compatible fresh targets, including targets known only by an earlier alias, while preserving actual body age. Alias attempts count toward the page limit.

followSubpages:false disables following. subpagePattern overrides camere|alloggi|appartament|ospitalit|rooms|soggiorn|eventi|prezzi|tariffe with case-insensitive literal alternatives separated by |, matched against path/query or anchor label. maxSubpages accepts 1-6. No arbitrary regex evaluation.

The existing service/cache/limiter own every fetch. Each child has its own URL/final URL/time/rung/status/hash/cache id and text continuation; failures preserve successful siblings. Cache lookup chooses the newest origin-policy-compatible entry. The disposable index gains nullable origin/final-URL/publication metadata, keeping old cache ids and compressed bodies readable. Only successful bodies advertise reusable final URLs, so a foreign refused redirect cannot substitute for a target. Publication order is independent of body age.

Text retains the core's 4 Mi-character extraction limit. Link metadata has a separate 32 MiB budget measured in actual serialized UTF-8 JSON bytes, preserving dense flat pages while bounding nested-label amplification. Exceeding extraction limits produces a visible per-page failure and preserves siblings. Cache writes and bounded decompression share a 64 MiB serialized-body ceiling; oversized writes fail before publication. No state database migration.

Traversal has a 30-second total deadline including root origin queue time, and shares maxChars (default/maximum 20,000) across returned text. Root expiry produces an explicit uncached deadline error with retry guidance. With following disabled, core single-page queue/active-HTTP budgets remain intact. Partial results expose truncated, truncationReasons, up to six pendingSubpages and unvisitedSubpages. Continue each page through cacheId/nextOffset, or read pending URLs with following disabled. Full text and anchors stay cached. Cache-id and nonzero-offset reads do not traverse.

Proof and review

  • Prior reviewed proof and Opus 5.5 round 6 PASS apply to historical head 5175cca6464da3fec1c56eb921b46dc9dec7026b, not this new main-based candidate. All six prior review findings were accepted and fixed with failing-before/passing-after MCP regressions: policy cache shadowing, late alias duplication, escaped metadata cache read/write mismatch, final targets known only through aliases, nested-label amplification, and rejection of dense flat pages by an inaccurate metadata estimate.
  • On the current exact head: 89 tests passed in four files, server typecheck, lint on all seven changed TypeScript files, server exports, fork stack/46-feature ownership and diff checks passed. Logs under /tmp/chromeria-441-r7-*.log include commands, exact base/head/tree, frozen-lockfile digest, Warsaw times and exit codes. The current proof comment records commands, results and log hashes.
  • MCP tests use the registered tool with real reader/compressed cache and external HTTP fixtures. Orchestration integration drains OrchestrationEffectWorkerV2 before/after, preserves thread state and measures bounded wire output/continuation. TestClock/Deferred only; no sleeps or polling. Existing core queue/active-HTTP/cancellation tests and all six review regressions remain unchanged.
  • Fresh independent Opus 5.5 round 7 passed on this exact head, confirming the complete candidate and all eight unchanged child replays. Newest review/independent is success, creator lukemaj, linked to that review. One optional low legacy-cache alias note was explicitly accepted for #447 by the parent before merge; it is not claimed fixed. The new T3 task explicitly requested xhigh; the reviewer could not independently observe its runtime effort setting. Current CI passed: lint, test, all eight server shards, build/typecheck and Check succeeded; unaffected web/mobile/Rust/release-smoke jobs were skipped by project CI. Fork passed on the same head after the explicitly authorized same-SHA close/reopen triggered both workflows. No earlier verdict or CI result is transferred to the new SHA.
  • Thread-transfer report names current head 388648949738ea8320bc4a780d96b39b55db94cb and main baseline c1d3960a1d1c0da46e714553ec32bf08b3136844: zero change in all measured Codex/Claude wire, decoded-live and message-count metrics, with every enforced ceiling passing.

Scope and limits

Server/MCP behavior only. Web, desktop and mobile retain generic MCP presentation. Provider adapters, WebSocket contracts, command-palette/keybinding entry points and connection routing do not change; shared-host forwarding is #443. No custom UI, browser/computer use, rendering, session access, search, live network/external-client or multi-environment/adoption claim. No browser proof is applicable to this slice. Website impact: none, because no website content or routes change. No version bump or machine/service/config activation.

Finalization

The implementation, exact-head proof, independent review, parent-owned merge and Issue closure are complete. Issue #441 owns the final handoff and cleanup proof. No version bump or activation was performed.

Verified disposable task scripts, redundant published bodies, rebase patch copies and historical review probes were retired. Raw proof logs and identities remain available. Cleanup does not touch the core/sibling branches, other owners' workspaces, shared installs or live state.

Both dependent owners released their #441 needs: #442 explicitly after #453 merged as f27d8ee146295ed2b76d8d15d415dd7cff574df5, and #443 in its final integration handoff. No open PR targets this branch. Remote feat/441-web-read-subpages and local proof/441-reviewed-5175cca646 were deleted with exact-SHA guards and verified absent. The local branch remains at 388648949738ea8320bc4a780d96b39b55db94cb, with its upstream unset, solely because this active T3 thread still binds /home/luke/.t3/worktrees/chromeria/feat-441-web-read-subpages. Retire that local branch/checkout after the active binding is retired. Raw proof and GitHub delivery records remain preserved; no core/sibling branch, other workspace, shared install or runtime was changed. The cleanup update and exact command/after-state proof are on #441.

Elon record

Implemented with GPT-6.1-Sol (max) through the Codex harness in Chromeria; independent review uses Claude Opus 5.5 through Claude Code, with xhigh requested and verdict/status bound to the exact head.

@github-actions github-actions Bot added size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Oct 18, 2026
@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: Scoped proof passed on PR #454 head ffe993bef5e973c01142dcb484ef572daeffeff2.
Why: The candidate now includes the updated #449 base and the feature-map link, so proof was repeated on that clean head.
So what: Opus 5.5 through Claude is independently reviewing this exact candidate; merge, deployment and restart remain unauthorized.

Base: feat/423-web-read-core at 763156d7667fc56fce088a06b40a10e7cd88ce47. git status --short is empty, and the remote PR head matches the local commit.

Check Result
node_modules/.bin/vp test run apps/server/src/mcp/toolkits/webResearch/handlers.test.ts apps/server/src/mcp/WebResearch.integration.test.ts apps/server/src/webResearch/PublicHttp.test.ts apps/server/src/htmlRender/publicProxy.test.ts 4 files, 61 tests passed; 20.09 seconds
node_modules/.bin/tsc-rs --noEmit -p apps/server/tsconfig.json exit 0
node_modules/.bin/vp lint on the six changed TypeScript files exit 0, no findings
node_modules/.bin/knip --workspace apps/server --exports --preprocessor ./scripts/knip-schemas.ts --no-config-hints exit 0
git diff origin/feat/423-web-read-core HEAD --check exit 0

The MCP tests verify default matching, cycles, normalized duplicate URLs, overrides/disable, page/text/time limits, partial failures, cached continuation, concurrent sharing/freshness, scoped redirect refusal, public/private destinations and exact origin counts. The orchestration integration drains OrchestrationEffectWorkerV2 before and after reading and verifies unchanged thread state plus bounded serialized output. TestClock/Deferred only; no sleeps. Existing core regressions also pass. An initial added assertion that required every child to wait 3 seconds was replaced with actual request start-spacing evidence, because a clock advance can exhaust the limiter wait before the page starts.

No complete-repository or live multi-environment/adoption claim is made; CI and #447 own those respectively. No version bump under project policy.

@lukemaj lukemaj left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What: Independent review of PR #454 at head ffe993bef5e973c01142dcb484ef572daeffeff2 (base 763156d7667fc56fce088a06b40a10e7cd88ce47, feat/423-web-read-core) found no blocking findings, only two low-severity cache/budget inefficiencies.
Why: The subpage slice meets #441's acceptance: default and overridable literal matching, normalized same-origin depth-one candidates, scoped redirect refusal, per-page provenance, shared 20,000-character and 30-second budgets with explicit continuation, and retained Effect Cache cancellation.
So what: review/independent is success on this SHA. The two notes are optional follow-ups for the author; merge stays with the user after #449 lands.

Findings

1. Low, non-blocking: a scoped refusal shadows a valid explicit-read cache entry

apps/server/src/webResearch/WebResearch.ts:356-369, with PageCache.get({ url }) returning only the newest row for a URL (apps/server/src/webResearch/PageCache.ts:58).

Trigger:

  1. web_read({ url: "https://a.example/camere", followSubpages: false }), where /camere 302s to an external booking engine. This stores an unrestricted ok entry with a seven-day TTL and an off-origin finalUrl.
  2. web_read({ url: "https://a.example/" }) links /camere. Traversal correctly refuses to reuse that entry, so a restricted fetch writes a newer unsupported row with restrictedToOrigin.
  3. Repeat step 1 within seven days. cache.get({ url }) now returns the refusal row. The explicit-read branch rejects it (restrictedToOrigin set, off-origin finalUrl), so the read requests a.example/camere and the external target again instead of reusing the still-valid entry from step 1.

Effect: the explicit result is correct (no poisoning), but a TTL-valid read pays origin requests and reports rung: "http". The handler test covers only the refusal-then-explicit order, not explicit, then refusal, then explicit. One possible repair is to select the newest row compatible with the caller's restriction.

2. Low, non-blocking: a redirect onto an already-returned page is refetched and duplicated

apps/server/src/webResearch/WebResearch.ts:470-503.

fetched skips a candidate only when the candidate's own URL equals an earlier final URL. A later candidate that redirects onto the root or an earlier child is fetched in full. Examples: the root links /camere then /rooms, and /rooms 302s to /camere; or /camere 302s to /. The restricted fetch requests the alias and then the target again (the hop loop at lines 218-251 does not know the pages this read already holds), and subpages receives text already returned. That spends a page slot and part of the shared output budget on duplicate text, which can push real pages into pendingSubpages. The existing alias test covers only the alias-before-target order.

Cleared

  • Default pattern, followSubpages: false, case-insensitive literal | override without regex evaluation, empty-term rejection, and maxSubpages 1-6 schema bounds.
  • Normalization of fragments, host case, default ports and dot segments; root self-links and the root's redirect endpoints are skipped; children's links are never followed.
  • External, credentialed, non-HTTP and private literal links are excluded by origin. Same-origin rebinding still goes through PublicHttp and returns blocked_local. Cross-origin HTTP and meta-refresh redirects are refused before robots or any request to the new origin.
  • Scoped refusals are never returned to explicit reads, and restricted flight keys never join unrestricted flights.
  • Effect 4.0.4 Cache: PageRequest keys hash and compare structurally. timeoutOption races in a forked fiber, so a timed-out child releases its awaiter, and the lookup is interrupted only when no other reader waits. Root sharing is unchanged.
  • Budgets: root plus child text never exceeds maxChars (at most 20,000). The deadline runs from read start; the TestClock test ends at exactly 30,000 ms with the slow child interrupted. pendingSubpages is a correct contiguous slice of at most six, unvisitedSubpages counts the rest, and every limit sets a truncation reason.
  • Partial failures keep successful siblings, each with its own status, httpStatus, hash and cache id.
  • Old cache entries decode because links and restrictedToOrigin are optional; anchorless roots are refetched only when following. Cache-id and nonzero-offset reads never traverse.
  • Claude and Codex T3 MCP tool timeouts are 65 minutes, so the 30-second budget fits. The MCP tool is the only consumer of WebReadResult.
  • Scope: server/MCP only. No recursion, rendering, forwarding, search, session or UI changes.

Proof checked

  • /tmp/chromeria-441-tests-final.log: 4 files, 61 tests passed; /tmp/chromeria-441-proof-head matches this head.
  • Typecheck log: suggestions only, no error TS, none in webResearch.
  • Final lint and knip logs show no findings. Their exit codes appear only in the author's proof note, not in the logs.
  • I did not rerun checks. GitHub CI Lint, Typecheck and Test were still running during this review.
  • Both findings come from reading the exact head source; I did not execute them.

Reviewed by Claude Opus 5.5 (claude-opus-5-5) through the Claude Code harness in T3 Code, dispatched at xhigh effort. Read-only review: no checkout, branch change or repository edit.

@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: c1d3960 · PR result: 3886489 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: Both low-severity findings from the Opus review are accepted and reproduced by new MCP tests.
Why: The newest URL row can shadow another read policy's valid cache entry, and aliases encountered after their target can duplicate text and origin requests.
So what: I am fixing both in this same PR and will repeat proof and independent review on the new head.

The first regression reproduces explicit read, then restricted refusal, then explicit read: the original head incorrectly reports http and a new cache id. The second reproduces late HTTP and meta-refresh aliases onto a returned child and the root: the original head returns four children instead of the two unique pages. Both tests failed at these intended assertions, without sleeps or timeout-based success.

The cache owner will select the newest entry compatible with the caller's origin policy using indexed metadata. The disposable index gains nullable origin columns while existing cache ids remain readable. Restricted redirects can reuse a compatible fresh target, preserving the target body's actual fetch time and expiry; redirect freshness participates in restricted in-flight identity. Traversal skips an already-returned successful final URL and counts alias attempts toward the page budget. This does not change #449's branch or unrestricted shared-fetch cancellation.

Elon record

  • Requirements and who asked: the user requested findings be fixed; Opus identified two unnecessary origin/budget costs inside feat(research): follow same-site subpages in web_read #441's cache/deduplication acceptance.
  • Deleted: scanning and decompressing all historical page bodies, new cache stores, read-specific mutable state in shared fetches and duplicate successful output.
  • Bottleneck: selecting a compatible existing page and keeping duplicate aliases out of the shared output budget.
  • Checked myself: traced both exact-head findings and reproduced each through the registered MCP seam before changing the implementation.

@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: Scoped proof passed on PR #454 head ff27f54bf76f55e9078e306818b5d01e7c9c10d6.
Why: The candidate now includes the latest #449 repairs, both accepted review fixes and the feature-map link, so proof was repeated on that clean head.
So what: Opus 5.5 through Claude is independently reviewing this exact candidate; merge, deployment and restart remain unauthorized.

Base: feat/423-web-read-core at 362cf419dcf4fe13027f9b3d958ab2e091336287. git status --short is empty, and the remote PR head matches the local commit.

Check Result
node_modules/.bin/vp test run apps/server/src/mcp/toolkits/webResearch/handlers.test.ts apps/server/src/mcp/WebResearch.integration.test.ts apps/server/src/webResearch/PublicHttp.test.ts apps/server/src/htmlRender/publicProxy.test.ts 4 files, 74 tests passed; 18.84 seconds
node_modules/.bin/tsc-rs --noEmit -p apps/server/tsconfig.json exit 0
node_modules/.bin/vp lint on the seven changed TypeScript files exit 0, no findings
node_modules/.bin/knip --workspace apps/server --exports --preprocessor ./scripts/knip-schemas.ts --no-config-hints exit 0
git diff origin/feat/423-web-read-core HEAD --check exit 0

The MCP tests verify default matching, cycles, normalized duplicate URLs, overrides/disable, page/text/time limits, partial failures, cached continuation, concurrent sharing/freshness, scoped redirect refusal, public/private destinations and exact origin counts. The orchestration integration drains OrchestrationEffectWorkerV2 before and after reading and verifies unchanged thread state plus bounded serialized output. TestClock/Deferred only; no sleeps. Existing core regressions also pass. An initial added assertion that required every child to wait 3 seconds was replaced with actual request start-spacing evidence, because a clock advance can exhaust the limiter wait before the page starts.

No complete-repository or live multi-environment/adoption claim is made; CI and #447 own those respectively. No version bump under project policy.

Both findings from review 5563998782 were reproduced before fixing and now pass. Added cases also cover cached redirect targets with forced freshness, alias publication after expired failures, root queue expiry without false origin cache failures, and reopening the original three-column index. All latest core queue/active-HTTP/cache-startup/race regressions pass, with single-page queue intent explicit and a separate default traversal deadline test. Opus follow-up review is running on this exact head.

@lukemaj lukemaj left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What: Follow-up independent review of PR #454 at head ff27f54bf76f55e9078e306818b5d01e7c9c10d6 (base 362cf419dcf4fe13027f9b3d958ab2e091336287, feat/423-web-read-core) found no blocking findings. Both earlier findings are fixed, and two new low-severity notes remain.
Why: Policy-aware cache selection and alias deduplication work as claimed, the rebase keeps every #449 round-2 guarantee, and the cumulative #441 acceptance holds. One note is a cache-poisoning path from hostile page content; the other is a leftover refetch case from finding 2.
So what: review/independent is success on this SHA. Both notes are optional author follow-ups; merge stays with the user after #449 lands.

Findings

1. Low, non-blocking: a hostile page can store a cache body the cache cannot read back

Locations: apps/server/src/webResearch/extract.ts:98 stores every anchor's label and absolute URL beside the text. apps/server/src/webResearch/PageCache.ts:91 reads bodies back with a fixed 32 MiB gunzip limit. apps/server/src/webResearch/WebResearch.ts:458 (and :290 for redirect targets) calls cache.get before anything else, and nothing catches its error.

Trigger: a page under the 4 MiB HTTP cap whose anchors are filled with C0 control characters, e.g. <a href=/> followed by 256 \u0001 characters, repeated about 15,000 times. JSON.stringify escapes each control character to 6 bytes, and the new links array repeats every label, so the cached JSON is twice the core's size.

Evidence: a disposable probe with the real extract, classify and PageCache's gunzip limit. A 4,050,055-byte page classifies ok with 4,170,000 text characters. Its body without links (the core's shape) is 23.4 MB and reads back. Its body with links is 47.0 MB, and reading it fails with ERR_BUFFER_TOO_LARGE. The threshold is about 2.9 MB of such HTML. The MCP path from that point follows from source; I did not run it end to end.

Effect: the first read succeeds and stores the body. After that, every read of that URL fails with orchestration_error ("The research page cache could not be read or written"), including maxAgeHours: 0, because every path calls get before it can store a newer row. If that page is a same-origin child, every default traversal of its root fails outright, losing the root and all successful siblings, until someone deletes web-cache. Only hostile same-origin content triggers this; ordinary ASCII link-heavy pages stay near 3x their HTML size. Possible repairs: bound the stored links, size the read limit for the largest body the write path can produce, or treat an unreadable URL row as a cache miss.

2. Low, non-blocking: an alias onto a page known only by its final URL still costs one extra request

Locations: apps/server/src/webResearch/WebResearch.ts:289-307 checks only for a row whose url equals the redirect target. apps/server/src/webResearch/PageCache.ts:112-113 indexes each page by its request URL only.

Trigger: the root redirects (e.g. https://hotel.example/ to https://www.hotel.example/it/, or /start to /home/), and a matching child redirects or meta-refreshes to the root's final URL. A second case: two aliases, /rooms and /suites, both redirect to /camere/, and /camere/ is not itself a candidate. The alias's target hop finds no row for /home/ (or /camere/), so it requests that page again before the traversal's duplicate check drops the result.

Effect: output stays deduplicated and the slot is counted as designed. The only cost is one extra origin request, plus at least 3 s of spacing out of the 30 s budget, per such alias. The new tests cover only targets that have their own URL row (an unredirected root /, or /camere). The /start to /home/ test has no child redirecting to /home/. Found from source; not executed.

Earlier findings, rechecked

  • Finding 1 (policy shadowing) is fixed. Unrestricted lookups select restriction_origin IS NULL OR final_origin = <url origin>. Restricted lookups select restriction_origin = S OR final_origin = S OR final_origin IS NULL. Both match acceptsOrigin, so the newest compatible row wins. A restricted fetch only ever targets its own origin, so restriction_origin is always NULL or S. The new MCP case runs explicit read, then scoped refusal, then both again: it returns the original cache ids with no new requests. Scoped refusals still never reach explicit reads, and target reuse cannot spread a refusal, because a refusal's final URL is off-origin.
  • Finding 2 (late aliases) is fixed for targets that have their own row. HTTP and meta-refresh aliases onto a returned child or an unredirected root skip duplicate text, with the exact request list asserted. The reverse order skips via fetched. Alias attempts count toward maxSubpages, and pendingSubpages stays a correct contiguous slice. maxAgeHours: 0 disables target reuse (isFresh is false at 0), and the flight-key freshness agrees. Reused targets keep their body fetchedAt and expiresAt, and requestedAt stops a forced flight from accepting an alias row with an older body. Separating publication order from body age makes the recovered-alias case correct.

Rebase onto 362cf41

  • request's active-HTTP budget, kept separate from queue and spacing, is unchanged. The disabled-cache service on open failure is kept: the migration runs inside make, and its errors map to PageCacheError("open"). The late storage recheck is kept and now also enforces origin policy, links and forced freshness.
  • The 30 s root deadline applies only when following. followSubpages: false and nonzero-offset URL reads keep the core's unbounded queue wait. An expired root returns WebResearchBudgetError, mapped to orchestration_error with retry guidance. The interrupted flight writes no cache row, as the Crawl-delay 40 case shows. This is a deliberate, documented trade-off: with following on (the default), same-origin reads queued past 30 s get a retryable error instead of waiting. The existing queue and active-HTTP tests opt out explicitly, and their oracles are unchanged.

Cumulative acceptance, cleared

Default pattern and limits (at most 6, schema 1-6), disable, and case-insensitive literal | overrides with empty terms rejected. Normalized depth-one candidates from the root's final URL, with cycles and duplicates skipped and children never traversed. External, private and credentialed links excluded, and off-origin redirects refused before robots or any request. Per-page provenance and failures that keep successful siblings. A 30 s total deadline and a shared 20,000-character output budget with explicit reasons and pending continuation. Cache-id and nonzero-offset reads never traverse. Old index columns are added and old bodies decode. Scope is server/MCP only: no recursion, rendering, forwarding, search, session or UI work.

Proof checked

  • /tmp/chromeria-441-tests-final.log: 4 files, 74 tests passed. It was written at 19:27:05, after the 19:25:29 amend that produced this head, and /tmp/chromeria-441-proof-head matches.
  • The local typecheck, lint and knip logs (19:24:32 to 19:24:45) predate that amend, so they cannot be tied to this commit. That gap is covered by GitHub CI on this exact SHA (run 39149558306): Typecheck, Lint, Test, Test Server 1-6 and Test Web all pass.
  • Proof gaps, not defects: the old-index test reopens a downgraded index only by cacheId. No test covers a URL lookup over legacy NULL-metadata rows, or a follow read refetching an anchorless legacy root. The source handles both.
  • I did not rerun the suite. The only execution was the disposable probe in /tmp described in finding 1.

Reviewed by Claude Opus 5.5 (claude-opus-5-5) through the Claude Code harness in T3 Code, at xhigh effort. Read-only review: no checkout, branch change or repository edit.

@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: Both new findings from the second independent review are reproduced and repaired in the #441 layer.
Why: Stored link labels can expand JSON beyond the old decode ceiling, and redirect targets known only through aliases were absent from URL cache lookup.
So what: Focused verification and another exact-head independent review will cover the new candidate before handoff; merge, deployment and restart remain outside this task.

Review: #454 (review), on ff27f54bf76f55e9078e306818b5d01e7c9c10d6, base 362cf419dcf4fe13027f9b3d958ab2e091336287.

  • The two new MCP tests failed on that implementation: the second read of the large-label subpage returned the typed cache error; redirect aliases produced ten HTTP requests instead of eight, repeating /home/ and /camere/.
  • Cache serialization and bounded decompression now share a 64 MiB ceiling. A write exceeding that ceiling fails before publishing a body or index row. This accommodates JSON escaping in both text and retained labels without discarding source links or hiding corruption as success.
  • Redirect-target lookup now includes indexed final URLs, retaining origin-policy filtering, freshness checks and the original body fetch time. Ordinary URL reads still use requested URLs so cached aliases cannot silently replace their redirect behavior.
  • The original-index reopening test also checks a URL cache hit over nullable migrated metadata. No feat(research): read public pages with shared cache and provenance #449 branch edits are involved.

Requirements and who asked: The user requested findings fixed; #441 requires bounded caching, shared budgets and duplicate suppression.
Deleted: A second cache, scans of historical bodies, silently dropped links, broad error fallbacks and repeated target HTTP requests.
Bottleneck: Unreadable persisted metadata and redundant target fetches consume or defeat the same traversal budget; the existing cache owns both repairs.
Checked myself: Read the review and source, ran both failing MCP reproductions before the implementation change, and retained the same cases for candidate verification.

@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: The final #441 candidate passes 77 focused MCP, network and integration tests after both second-round findings were fixed.
Why: Regression tests reproduced unreadable cached label metadata and repeated requests through final-URL aliases on the previous head.
So what: The new exact head is pushed for independent review; this remains an open stack above #449 with merge, deployment and restart unperformed.

Head: d2d6dd3d7094ad9bed92e69dcf0c145e1cdb88a4.
Tree: 471672c3621e3217321bdb3ca16314369bdbe733.
Exact base: 362cf419dcf4fe13027f9b3d958ab2e091336287, feat/423-web-read-core, PR #449. Refreshed live and unchanged before this push.

All final checks ran after the commit on a clean checkout of this exact head:

Command Observed result
CI=1 NO_COLOR=1 node_modules/.bin/vp test run apps/server/src/mcp/toolkits/webResearch/handlers.test.ts apps/server/src/mcp/WebResearch.integration.test.ts apps/server/src/webResearch/PublicHttp.test.ts apps/server/src/htmlRender/publicProxy.test.ts 4 files, 77 tests passed, 15.74 s, exit 0
node_modules/.bin/tsc-rs --noEmit -p apps/server/tsconfig.json exit 0; no errors
node_modules/.bin/vp lint apps/server/src/webResearch/schemas.ts apps/server/src/webResearch/extract.ts apps/server/src/webResearch/PageCache.ts apps/server/src/webResearch/WebResearch.ts apps/server/src/mcp/toolkits/webResearch/tools.ts apps/server/src/mcp/toolkits/webResearch/handlers.test.ts apps/server/src/mcp/WebResearch.integration.test.ts exit 0; no findings
node_modules/.bin/knip --workspace apps/server --exports --preprocessor ./scripts/knip-schemas.ts --no-config-hints exit 0
git diff --check and git diff --exit-code HEAD exit 0

The two new MCP regressions failed on ff27f54bf76f55e9078e306818b5d01e7c9c10d6, then passed after repair. The alias fixture fetched /home/ and /camere/ twice before the fix; the final candidate requests each target once. The large-label fixture's first read succeeded but its next cached root read failed before the fix; the final candidate returns the cached child and its cache-id continuation without network. A third case failed during repair until refused redirects were excluded from advertised final URLs, then passed. Legacy index reopening now checks both URL and cache-id reads.

Tests exercise the registered MCP boundary with the real reader and compressed cache, external HTTP fixtures, TestClock/Deferred milestones and no sleeps or polling. Orchestration integrations drain OrchestrationEffectWorkerV2 before and after and check unchanged thread state. Queue/active-HTTP regression oracles remain intact. Complete repository checks are CI-owned. No UI/browser, live multi-environment or deployment verification is claimed.

Local logs: /tmp/chromeria-441-r3-tests-final.log, /tmp/chromeria-441-r3-typecheck-final.log, /tmp/chromeria-441-r3-lint-final.log, /tmp/chromeria-441-r3-knip-final.log; exact head/tree recorded in /tmp/chromeria-441-r3-proof-identity.

@lukemaj lukemaj left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What: Round 3 independent review of PR #454 at head d2d6dd3d7094ad9bed92e69dcf0c145e1cdb88a4 (base 362cf419dcf4fe13027f9b3d958ab2e091336287, feat/423-web-read-core) found no blocking findings. Both round-2 findings are fixed. One new low-severity note remains.
Why: The final-URL lookup removes the duplicate alias requests, and the shared 64 MiB ceiling stops unreadable cache bodies from being published. That ceiling can still be reached by crafted nested anchors, and a child that hits it fails the whole traversal.
So what: review/independent is success on this SHA. The note is an optional author follow-up. Merge stays with the user after #449 lands.

Finding

1. Low, non-blocking: a crafted same-origin child can still exceed the 64 MiB ceiling, and its write error fails the whole traversal

Locations: apps/server/src/webResearch/extract.ts:98 stores a label of up to 256 characters for every anchor, nested ones included. apps/server/src/webResearch/PageCache.ts:113-117 rejects JSON over MAX_CACHE_JSON_BYTES (:22). apps/server/src/webResearch/WebResearch.ts:419 yields cache.put without recovery. The child load at WebResearch.ts:606-609 does not catch it either, and apps/server/src/mcp/toolkits/webResearch/handlers.ts:15 maps it to orchestration_error.

Mechanism: in HTML, <a> elements cannot nest. In MathML foreign content the parser does nest them. Each ancestor's label is the first 256 characters of its inner text, so one run of 256 control characters under 256 nested anchors is repeated in about 32,900 label characters, each escaped to 6 JSON bytes. The PR body says the 64 MiB ceiling "accommodate[s] JSON escaping in retained labels and text". That holds only while each label character also appears once in the text, which nesting breaks.

Trigger and evidence: a disposable probe used this head's real extract, classify and CachedPage JSON encoder. The page was <math> + 256 x <a href=x> + 256 x \u0001 + </math>, repeated.

Chains HTML bytes Status Serialized JSON bytes put
260 735,679 ok 64,996,347 accepted
270 763,969 ok 67,496,187 rejected
300 848,839 ok 74,995,707 (2,534,908 without links) rejected

All three are far below the 4 MiB HTTP cap and the 4M-character text cap.

Effect: the oversized body is never published, so the round-2 persistence is gone. But when such a page is a matching same-origin child, PageCacheError("write") fails the entire web_read with "The research page cache could not be read or written." The root and every successful sibling are dropped. The child is never cached, so every later default traversal of that root requests the child again and fails again. Reads with followSubpages:false still work. This contradicts the PR body's "failures preserve successful siblings", and the message blames the cache for what is a page-size failure. Only deliberately crafted same-origin content triggers it; ordinary pages do not nest anchors. I followed the error path from source and did not run it end to end through MCP.

Possible repairs, if wanted: bound the stored link metadata separately from the text, or turn an oversized page write into an uncached per-page failure so the siblings survive.

Round-2 findings, rechecked

  • Unreadable cached body: fixed for the reported mechanism. The writer now rejects any JSON that the reader's 64 MiB gunzip limit could not return, before the body or the index row is written (PageCache.ts:113-131). Reads therefore always cover writes. The 11,000-anchor MCP regression is about 34 MB of JSON, so it would have failed under the old 32 MiB read limit. The note above is the only remaining path to the limit.
  • Aliases onto final-URL-only targets: fixed. redirectTarget (PageCache.ts:75-79) matches url or final_url with the same restricted-origin filter. Only restricted redirect hops use it (WebResearch.ts:289-290). Unrestricted URL reads still match only url (:80-87).
    • A final_url match always has final_origin equal to the traversal origin, so the status === "ok" gate (:129) is what keeps refused redirects from standing in for a target. The new foreign-site regression covers this.
    • Successful restricted rows always end on their restriction origin, so no row can cross policies.
    • An unrestricted successful row that reached the target through a cross-origin redirect holds the target's real body, and robots was checked on every hop.
    • The new test asserts the exact eight-request list. Under ff27's URL-only lookup, /home/ and /camere/ would each be requested twice.
    • Legacy rows get final_url NULL without any body scan. They only lose alias reuse.
  • The integration test's in-memory cache returns null for redirectTarget. That only disables reuse; the queue and active-HTTP oracles are unchanged.
  • The legacy-index reopen test now also reads by URL over NULL-metadata rows, closing that round-2 proof gap. A follow read that refetches an anchorless legacy root is still untested; the source handles it.

Cumulative acceptance

The base is unchanged at 362cf419 (#449's live head), and the only changes since round 2 are the two cache changes above, so the round-2 acceptance clearance still holds:

  • Default pattern, at most 6 subpages, disable, and literal case-insensitive | overrides.
  • Normalized depth-one candidates and cycle and duplicate skipping.
  • External, private and credentialed destinations refused, along with off-origin redirects.
  • Per-page provenance, the 30 s deadline including the root queue, the shared 20,000-character output, and explicit truncation with pending continuation.
  • Cache-id and offset reads never traverse.
  • Server/MCP scope only.

The tool description and orchestrationInstructions.ts guidance remain accurate.

Proof checked

  • The commit is dated 19:48:24 +0200. /tmp/chromeria-441-r3-proof-identity (19:48:58) holds this head and tree 471672c3621e3217321bdb3ca16314369bdbe733, matching git log.
  • /tmp/chromeria-441-r3-tests-final.log: started 19:48:59; 4 files and 77 tests passed in 15.74 s.
  • The typecheck log (19:49:15) has 0 errors, only existing suggestions. The lint log has no findings and the knip log is empty. All three were written after the commit, which closes the round-2 timing gap. The logs do not record exit codes, but their output agrees with exit 0.
  • GitHub CI on this exact SHA passed: Typecheck, Lint, Test, Test Server 1-6, Test Web, Build and Check. One duplicate "Native fingerprint diff" run was cancelled and its rerun passed.
  • I did not rerun the suite. The only execution was the disposable /tmp probe described in the finding.

Reviewed by Claude Opus 5.5 (claude-opus-5-5) through the Claude Code harness in T3 Code, at xhigh effort. Read-only review: no checkout, branch change or repository edit.

@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: The nested-anchor finding is reproduced through MCP and repaired at the extraction boundary.
Why: Bounding cache writes prevents unreadable persistence but cannot classify amplified source metadata as a per-page extraction failure.
So what: The candidate will receive focused proof and a fresh independent review before handoff; #449 is still the base and is unchanged by this task.

Accepted finding: #454 (review), on d2d6dd3d7094ad9bed92e69dcf0c145e1cdb88a4.

The new registered-MCP regression uses 270 chains of 256 MathML anchors containing control characters, between two normal same-site children. It failed on that head with the reported cache error, losing the entire traversal. The fix gives retained link URLs, labels and per-entry overhead an independent 4 Mi-character extraction budget. This bounds metadata even when ancestor labels repeat descendant text. Existing extraction error handling returns a visible provider_error for that page and caches its small failure result; the root and siblings remain available and the next read is cached.

The 4 Mi-character text and metadata budgets together serialize to at most approximately 48 MiB under worst-case six-byte JSON escaping, plus small fixed provenance fields, below the existing shared 64 MiB cache ceiling. The ceiling is unchanged. URL.parse replaces the anchor's URL-only try/catch so a metadata-limit error cannot be swallowed as a malformed URL. Genuine cache I/O failures retain their existing error behavior.

Requirements and who asked: The user requested findings fixed; #441 requires bounded extraction and per-page failures preserving the root and successful siblings.
Deleted: Another cache-limit increase, silent omission of source anchors and broad recovery from storage errors.
Bottleneck: Repeated nested labels amplified metadata independently of page text; extraction owns the source-size failure and its existing per-page error mapping.
Checked myself: Read the review and caller/error paths, reproduced the complete MCP failure before the edit, and retained the same root/failed-child/sibling/cache-hit assertions for candidate proof.

@lukemaj
lukemaj force-pushed the feat/441-web-read-subpages branch from d2d6dd3 to d3cf0f5 Compare October 18, 2026 18:08
@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: The rebased #441 candidate passes 83 focused tests and all scoped static checks, with the nested-anchor finding fixed.
Why: Link metadata needed its own extraction budget, and #449 advanced with bounded origin-queue behavior during the repair.
So what: This exact candidate is pushed for a fresh independent review; the stack remains open above #449 and unmerged.

Head: d3cf0f548ee922115e9c91f84b7b1bcdbe2c5447.
Tree: b4758f3f5a17a701df31054eb860fb5fbf50bd47.
Exact base: 7a4b1f1537928d7193ca090d726547f788a2e5fc, feat/423-web-read-core, PR #449. Refreshed live and unchanged before push. Original requested base and intervening rebases remain recorded in the PR body.

All final checks ran after committing this exact head on a clean checkout. Each log records the exact head and exit_code=0:

Command Observed result
CI=1 NO_COLOR=1 node_modules/.bin/vp test run apps/server/src/mcp/toolkits/webResearch/handlers.test.ts apps/server/src/mcp/WebResearch.integration.test.ts apps/server/src/webResearch/PublicHttp.test.ts apps/server/src/htmlRender/publicProxy.test.ts 4 files, 83 tests passed, 18.35 s, exit 0
node_modules/.bin/tsc-rs --noEmit -p apps/server/tsconfig.json exit 0; no errors
node_modules/.bin/vp lint apps/server/src/webResearch/schemas.ts apps/server/src/webResearch/extract.ts apps/server/src/webResearch/PageCache.ts apps/server/src/webResearch/WebResearch.ts apps/server/src/mcp/toolkits/webResearch/tools.ts apps/server/src/mcp/toolkits/webResearch/handlers.test.ts apps/server/src/mcp/WebResearch.integration.test.ts exit 0; no findings
node_modules/.bin/knip --workspace apps/server --exports --preprocessor ./scripts/knip-schemas.ts --no-config-hints exit 0
git diff --check and git diff --exit-code HEAD exit 0

Third-review finding and repair: the real-cache MCP regression failed on d2d6dd3d7094ad9bed92e69dcf0c145e1cdb88a4 with the reported cache error. It now returns the root, a normal child, a visible provider_error for the oversized nested-anchor page, and another normal child. The next read returns the same cached pages without more network. Text and metadata budgets independently bound JSON expansion below the existing cache ceiling, which was not raised again.

The rebase retains the new core's 60-second origin-queue limit, uncached backoff refusals, separate 30-second active-HTTP budget and cache-open diagnostics. The 24-read semaphore-contention test initially failed under default subpage following because this layer's total traversal deadline is shorter. It now explicitly sets followSubpages:false to keep proving the single-page queue contract; all existing timing, status, delay, backoff and non-persistence assertions are unchanged. The other four new core queue cases pass without edits. Separate default-traversal tests retain the 30-second total deadline proof. The core's extracted memory-cache helper retains the new lookup-key union. Rebase range-diff was inspected; no #449 branch was edited.

Tests exercise the registered MCP boundary with real reader/cache and external HTTP fixtures, TestClock/Deferred milestones and no sleeps or polling. Integrations drain OrchestrationEffectWorkerV2 before and after and check unchanged thread state. No existing regression was removed or its oracle weakened. Complete repository checks remain CI-owned; native-main integration remains coordinator-owned on #449. No browser, live adoption, deployment or restart claim.

Local evidence: /tmp/chromeria-441-r4-proof-identity, /tmp/chromeria-441-r4-tests-final.log, /tmp/chromeria-441-r4-typecheck-final.log, /tmp/chromeria-441-r4-lint-final.log, /tmp/chromeria-441-r4-knip-final.log.

@lukemaj lukemaj left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What: Round 4 independent review of PR #454 at head d3cf0f548ee922115e9c91f84b7b1bcdbe2c5447 (base 7a4b1f1537928d7193ca090d726547f788a2e5fc, feat/423-web-read-core) found no blocking findings. All five earlier findings are fixed, the rebase keeps the new core queue fixes, and one new low-severity note remains.
Why: The new link-metadata budget stops nested-anchor amplification, but it also rejects flat, link-dense pages that the core reads, including reads with followSubpages:false.
So what: review/independent is success on this SHA, and the note is an optional author follow-up. #449 has since moved to a9da7866dc, so this child needs a rebase, affected proof and a fresh exact-head review. Merge stays with the user after #449 lands.

Finding

1. Low, non-blocking: link-dense flat pages now fail extraction where the core returned them

Locations:

  • apps/server/src/webResearch/extract.ts:36 sets MAX_LINK_CHARS equal to the 4 Mi-character text budget.
  • extract.ts:101-103 charges every HTTP(S) anchor href + label + 32 characters.
  • The throw becomes PageExtractionError at apps/server/src/webResearch/WebResearch.ts:379-382, then provider_error at :411-412.

fetchPage always extracts, so this also applies with followSubpages:false.

Mechanism: an anchor costs about label + href + 4 characters of text ([label](href)), but label + href + 32 characters of metadata. On a page whose text is mostly anchors, the link budget runs out before the text budget. No nesting is needed.

Trigger and evidence: a disposable /tmp probe ran the real extract and classify from the base and from this head on the same HTML.

Page HTML bytes Core (7a4b1f1) This head
36,000 x <li><a href="/strutture/agriturismo-000123-toscana">Agriturismo 123 Toscana</a></li> 3,084,958 ok, 3,264,888 chars ok
38,000 of the same 3,256,958 ok, 3,446,888 chars throws Research page links exceed the extraction limit.
45,000 of the same 3,858,958 ok, 4,083,888 chars throws
80,000 x <a href=/a>b</a> 1,280,068 ok, 2,240,000 chars throws

All of these are under the 4 MiB HTTP body cap and the 4 Mi-character text cap.

Effect: a page with about 37k ordinary anchors or more returns provider_error with no text, where the core returned its full text. That is about 3.2 MB of HTML, or about 1.2 MB with very short links. Default reads and followSubpages:false reads are both affected. When the page is the root, its subpages are lost too. The review brief says followSubpages:false keeps the core's single-page behavior; for this page shape it does not.

Severity is low: only very large HTML sitemaps and index pages are this dense, and venue homepages are not. I did not run this end to end through MCP. The source path from the throw to provider_error is direct.

Possible repairs, if wanted:

  • Give the metadata the remaining headroom. Worst-case text JSON is 24 MiB, so about 6.5 Mi characters of metadata still fit under 64 MiB. That covers ordinary anchors up to the 4 MiB body cap.
  • Or keep the text and report truncated link metadata visibly, instead of failing the page.

Earlier findings, rechecked on this head

  1. Scoped refusals shadowing explicit reads: fixed. Unrestricted URL lookups skip restricted rows unless their final origin matches (PageCache.ts:80-84). acceptsOrigin repeats the check (WebResearch.ts:145-148). The MCP regression covers both policy orders.
  2. Late aliases refetching and duplicating output: fixed. Restricted hops reuse fresh compatible targets (WebResearch.ts:320-339), keeping the target's real fetchedAt/expiresAt (:428-436). The traversal dedupes successful final URLs and still counts the attempt (:643-661).
  3. Escaped labels over the old 32 MiB read limit: fixed. Writes reject JSON over 64 MiB before publication, and reads allow the same 64 MiB (PageCache.ts:22, :102, :113-117).
  4. Final URLs known only through aliases: fixed. redirectTarget matches url or final_url under the origin filter (PageCache.ts:75-79). Only ok rows advertise final_url (:129).
  5. Nested anchors exceeding the write ceiling: fixed.
    • Text is at most 4 Mi characters, at no more than 6 JSON bytes each: 24 MiB.
    • A link entry serializes to at most 21 + 2*href + 6*label bytes, because URLs need only \\ escapes. That is below 6 * (href + label + 32), so all links together take at most 24 MiB.
    • The total therefore stays under the shared 64 MiB ceiling.
    • The MCP regression keeps the root and both siblings, and the next read is served from cache.

Rebase onto 7a4b1f1

  • Core queue code untouched. The child diff leaves alone the core's request queue and 60 s acquisition deadline (WebResearch.ts:180-238), the zero-TTL robots refusal (:288-292) and the OriginQueueWaitError catch (:414-420). if (cacheable) yield* cache.put(page) also covers the reused-target path (:457).
  • One core test edited, oracle unchanged. The 24-read contention case gains followSubpages: false and still proves the single-page contract.
    • The Retry-After and Crawl-delay cases pass unedited under default following, because those refusals are immediate.
    • /tmp/chromeria-441-r4-rebase-queue.log shows the default-follow version failing on Exit.isSuccess, as the author described.
  • Deliberate behavior change. With default following, a root that waits 30 to 60 s now returns WebResearchBudgetError instead of the core's uncached challenge. The tool description and PR body document this, and an MCP test covers it. followSubpages:false keeps the core's 60 s wait.
  • Timeouts cannot cache an interruption. The pinned effect@4.0.4 Cache.get drops interrupted lookups (Cache.ts:641-647). A shared flight keeps running while another reader awaits it (:686-697). PageRequest keys compare structurally, so restricted and unrestricted flights stay separate.

Cumulative acceptance

Met:

  • Default pattern, at most 6 subpages, disable, and literal case-insensitive | overrides without regex evaluation.
  • Normalized depth-one candidates, with cycles and duplicates skipped.
  • External, private, credentialed and off-origin-redirect destinations refused, with public-network validation on every fetch.
  • Shared cache, limiter and flight cancellation.
  • Per-page provenance, with failures that keep successful siblings.
  • A 30 s total deadline that includes the root.
  • A shared 20,000-character output, explicit truncationReasons, bounded pendingSubpages, and cacheId/nextOffset continuation.
  • Cache-id and offset reads never traverse.
  • Old three-column indexes reopen.
  • Server/MCP scope only.

Docs:

  • The tool description and orchestrationInstructions.ts remain accurate.
  • The docs/map.md ref follows the secondary-PR rule, since #454 does not close the primary #423.
  • docs/fork-features.md needs no new PR entry, because the carried file set is unchanged.

Considered and not actionable:

  • pendingSubpages can list an alias of a page already returned.
  • A forced refresh can fetch a redirect target twice in one traversal.
  • An anchorless legacy row can race a concurrent followSubpages:false flight, but legacy rows exist only in unreleased core caches.

Proof checked

  • Identity. The commit is dated 20:06:55 +0200. /tmp/chromeria-441-r4-proof-identity (20:07:17) holds this head and tree b4758f3f5a17a701df31054eb860fb5fbf50bd47, matching git rev-parse.
  • Tests. 4 files and 83 tests passed in 18.35 s.
  • Static checks. The typecheck, lint and knip logs were written after the commit. Each ends with this head and exit_code=0. The typecheck log has only existing suggestions.
  • CI on this exact SHA passed: Typecheck, Lint, Test, Test Server 1-6, Test Web, Build, Check and Release Smoke. One duplicate Native fingerprint diff run was cancelled, and its rerun passed.
  • Not rerun. I did not rerun the suite. The only execution was the disposable /tmp extraction probe above.

Stack note

feat/423-web-read-core moved to a9da7866dc ("fix(research): cancel queue deadline on acquisition", 20:17 +0200), after this candidate's base. That commit changes request and the OriginQueueWaitError catch, and adds 135 lines of integration tests. This review does not cover it. The child needs a rebase, affected proof and a new exact-head review. This verdict applies only to d3cf0f5 on 7a4b1f1.

Reviewed by Claude Opus 5.5 (claude-opus-5-5) through the Claude Code harness in T3 Code, at xhigh effort. Read-only review: no checkout, branch change or repository edit.

@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: The dense-page finding is reproduced at MCP and repaired by measuring serialized link metadata.
Why: A raw-character estimate reserved too much space per ordinary link and rejected flat pages before the core's text limit.
So what: The child will be rebased onto #449's newest queue fix, then receive refreshed proof and independent review; no parent branch edit or merge is involved.

Accepted finding: #454 (review), on d3cf0f548ee922115e9c91f84b7b1bcdbe2c5447.

The 80,000-link MCP regression failed on that head: followSubpages:false returned provider_error with no text. The repair counts each link's actual JSON UTF-8 bytes, including escaping and separators, against a 32 MiB metadata budget. It preserves the core's dense-page text, cache continuation and default cached reread without more network. The nested-anchor failure and >32 MiB escaped-label roundtrip regressions remain required.

The unchanged text bound requires at most 24 MiB under worst-case JSON escaping, so 32 MiB of encoded metadata plus fixed provenance stays below the existing 64 MiB cache ceiling. This replaces the estimate rather than guessing another raw-character threshold. Nested amplification remains an explicit per-page extraction failure; no anchors are silently dropped and no storage errors are hidden.

Requirements and who asked: The user requested findings fixed; #441 must preserve usable single-page reads and bound metadata without losing healthy siblings.
Deleted: Raw-character metadata estimates, another cache-ceiling increase and silent link truncation.
Bottleneck: The guard measured a proxy for serialized size rather than the bytes that determine cache readability.
Checked myself: Read the review and new parent diff, reproduced the dense-page failure through MCP, and retained dense/control-character/nested-anchor cases as independent compatibility and bounds proof.

@lukemaj
lukemaj force-pushed the feat/441-web-read-subpages branch from d3cf0f5 to c3e6698 Compare October 18, 2026 18:30
@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: The rebased #441 candidate passes 89 focused tests and scoped static checks, with all six prior findings fixed.
Why: Actual serialized metadata preserves dense-page compatibility while bounding nested amplification, and the parent repaired queue-deadline cancellation.
So what: The new exact head is pushed for independent review above #449; merge, deployment and restart remain unperformed.

Head: c3e66983def5a30ad0c97dc4f0e624da7490e257.
Tree: 8cf0ac2eed89d7a5546b5613347196bcd3889cc5.
Exact base: a9da7866dcc2fcaa377dc6a39f44e0f84211ece9, feat/423-web-read-core, PR #449. Refreshed live and unchanged before push. The original requested base and intermediate rebases remain in the PR body.

All final checks ran after committing this exact head on a clean checkout. Every log records this head and exit_code=0:

Command Observed result
CI=1 NO_COLOR=1 node_modules/.bin/vp test run apps/server/src/mcp/toolkits/webResearch/handlers.test.ts apps/server/src/mcp/WebResearch.integration.test.ts apps/server/src/webResearch/PublicHttp.test.ts apps/server/src/htmlRender/publicProxy.test.ts 4 files, 89 tests passed, 18.02 s, exit 0
node_modules/.bin/tsc-rs --noEmit -p apps/server/tsconfig.json exit 0; no errors
node_modules/.bin/vp lint apps/server/src/webResearch/schemas.ts apps/server/src/webResearch/extract.ts apps/server/src/webResearch/PageCache.ts apps/server/src/webResearch/WebResearch.ts apps/server/src/mcp/toolkits/webResearch/tools.ts apps/server/src/mcp/toolkits/webResearch/handlers.test.ts apps/server/src/mcp/WebResearch.integration.test.ts exit 0; no findings
node_modules/.bin/knip --workspace apps/server --exports --preprocessor ./scripts/knip-schemas.ts --no-config-hints exit 0
git diff --check and git diff --exit-code HEAD exit 0

Fourth-review finding and repair: the 80,000-link MCP case failed on d3cf0f548ee922115e9c91f84b7b1bcdbe2c5447, returning provider_error instead of readable single-page text. It now returns ok, full cached text with bounded continuation and a default cached reread without new network. Link metadata uses actual JSON UTF-8 size, at most 32 MiB; worst-case text JSON is at most 24 MiB, leaving headroom below the existing 64 MiB cache ceiling. The escaped-label roundtrip and nested-anchor partial-failure regressions still pass.

Rebased only the child onto #449's a9da7866dc. Its acquisition cancels the queue deadline before active HTTP, reports unknown contention backoff as null, and clears stale HTTP status on a refused redirect. All five newest core tests pass. The four cases requiring more than 30 seconds use followSubpages:false to exercise their original single-page queue/active-HTTP contract; their status, timing, backoff, HTTP-status and cache assertions are unchanged. The immediate refused-redirect case passes unedited. Earlier contention and default-traversal deadline tests remain. Range-diff shows the authored commits replayed unchanged; no parent branch was edited.

Registered MCP tests use the real reader/cache and external HTTP fixtures. Orchestration integrations drain OrchestrationEffectWorkerV2 before and after and check unchanged thread state. Timing uses TestClock/Deferred milestones, with no sleeps or polling. No existing regression was removed or its oracle weakened. Complete checks remain CI-owned and parent native-main integration remains coordinator-owned. No browser or live-adoption verification claimed.

Local evidence: /tmp/chromeria-441-r5-proof-identity, /tmp/chromeria-441-r5-tests-final.log, /tmp/chromeria-441-r5-typecheck-final.log, /tmp/chromeria-441-r5-lint-final.log, /tmp/chromeria-441-r5-knip-final.log.

@lukemaj
lukemaj force-pushed the feat/423-web-read-core branch from a9da786 to fe1bfe3 Compare October 18, 2026 18:31
@lukemaj
lukemaj force-pushed the feat/441-web-read-subpages branch from c3e6698 to 5175cca Compare October 18, 2026 18:37
@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: The #441 layer now passes 89 focused tests and scoped static checks on #449's native-main-based head.
Why: The parent rebased onto native main while review was starting, requiring a new exact child candidate and dependency-sensitive proof.
So what: The stale review was stopped without a verdict; fresh independent review will cover this exact head and base before handoff.

Head: 5175cca6464da3fec1c56eb921b46dc9dec7026b.
Tree: 964569e42b0dd7c76cb5bab9087155495d220970.
Exact base: fe1bfe38579aba2e81390e829ad66fdc7eda402b, feat/423-web-read-core, PR #449. Refreshed live and unchanged before push. No parent branch was edited.

git range-diff a9da7866dcc2fcaa377dc6a39f44e0f84211ece9..c3e66983def5a30ad0c97dc4f0e624da7490e257 fe1bfe38579aba2e81390e829ad66fdc7eda402b..HEAD shows all eight authored commits replayed unchanged. The cumulative child diff remains eight files. The core's webResearch, MCP toolkit and research integration files are unchanged between those parents. Native main changes dependency/runtime inputs and removes comments from the proxy test; its map update is preserved.

The frozen offline dependency install completed in 12.3 s with no downloads or tracked changes. Installed Effect is 4.0.5. Root AGENTS.md and the unchanged direction triad were reloaded after rebase.

All final checks ran on the clean exact head after dependency refresh. Each check log records the exact head and exit_code=0:

Command Observed result
CI=1 NO_COLOR=1 node_modules/.bin/vp test run apps/server/src/mcp/toolkits/webResearch/handlers.test.ts apps/server/src/mcp/WebResearch.integration.test.ts apps/server/src/webResearch/PublicHttp.test.ts apps/server/src/htmlRender/publicProxy.test.ts 4 files, 89 tests passed, 17.69 s, exit 0
node_modules/.bin/tsc-rs --noEmit -p apps/server/tsconfig.json exit 0; no errors
node_modules/.bin/vp lint apps/server/src/webResearch/schemas.ts apps/server/src/webResearch/extract.ts apps/server/src/webResearch/PageCache.ts apps/server/src/webResearch/WebResearch.ts apps/server/src/mcp/toolkits/webResearch/tools.ts apps/server/src/mcp/toolkits/webResearch/handlers.test.ts apps/server/src/mcp/WebResearch.integration.test.ts exit 0; no findings
node_modules/.bin/knip --workspace apps/server --exports --preprocessor ./scripts/knip-schemas.ts --no-config-hints exit 0
git diff --check and git diff --exit-code HEAD exit 0

The six findings and their failing-before/passing-after regressions are retained unchanged and all pass here. Registered MCP tests use the real reader/cache and external HTTP fixtures; integrations drain effects and verify unchanged thread state. TestClock/Deferred milestones, no sleeps or polling. Latest core queue/active-HTTP regressions retain their assertions and single-page intent. No test was removed or weakened.

The previous review attempt was interrupted with no pending child runs after detecting the parent move, before a verdict. Its old-head pending status was marked superseded/error. A new review/status applies to this current candidate. Complete repository checks remain CI-owned. No merge, release, deployment, restart, browser or live-userdata changes.

Local evidence: /tmp/chromeria-441-r6-proof-identity, /tmp/chromeria-441-r6-install.log, /tmp/chromeria-441-r6-tests-final.log, /tmp/chromeria-441-r6-typecheck-final.log, /tmp/chromeria-441-r6-lint-final.log, /tmp/chromeria-441-r6-knip-final.log.

@lukemaj lukemaj left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What: Round 6 independent review of PR #454 at head 5175cca6464da3fec1c56eb921b46dc9dec7026b (base fe1bfe38579aba2e81390e829ad66fdc7eda402b, feat/423-web-read-core) found no findings, blocking or otherwise.
Why: The sixth finding's fix now counts real serialized JSON bytes. No non-nested page that the core reads as ok can exceed the 32 MiB link budget. All five earlier fixes, and the rebase onto the native-main core, hold on Effect 4.0.5.
So what: review/independent is success on this SHA. #449 must land first, and merge stays with the user.

Reviewer: Claude Opus 5.5 (claude-opus-5-5) through Claude Code. xhigh effort was requested, but the session does not show me its effort setting, so I cannot confirm it. Read-only: no checkout, no edits, no test reruns.

Sixth finding (dense flat pages): fixed

  • extract.ts:99-104 charges Buffer.byteLength(JSON.stringify({url,label})) + 1 per HTTP(S) anchor, starting from 2 for []. This is the real array JSON plus at most one byte. The cache encodes with Schema.fromJsonString, which in pinned effect@4.0.5 is a plain JSON.stringify with no spacing (SchemaGetter.js:979), in the same url, label field order. The accounting therefore matches what is stored.
  • No flat page the core reads can fail this budget. Per non-nested anchor:
    • Metadata is at most 22 + 2*href + 6*label bytes. URLs need at most \\ escapes, and labels are compact(inner).slice(0,256).
    • That anchor's text [X](href) is at least 4 + href + max(label, 1) characters, or 4 + 2*href when the label is empty.
    • So metadata is at most 6 times the anchor's own text. Flat anchors' text pieces are disjoint and total at most 4 Mi characters, so metadata stays at or below 24 MiB, under the 32 MiB budget.
    • Only nested anchors (MathML, or <a> inside table cells) count the same text more than once. That is finding 5's deliberate visible provider_error case.
  • The write ceiling still holds. Text JSON is at most 24 MiB, links at most 32 MiB, and the title (500 characters, extract.ts:73) and URLs (4096 characters, WebResearch.ts:88) add a few KB. That stays under the shared 64 MiB write/read ceiling (PageCache.ts:22,102,113). Redirect-reuse aliases copy an already-bounded body.
  • Tests discriminate the fix. The 80,000 <a href="x">Rooms</a> case writes about 4.5 MB of links and about 3.0 M characters of text. The old raw estimate charged about 5.8 M characters and failed it. The case also checks text, continuation and a cached default reread with two requests. The 11,000-label control-character case stores about 35 MB of JSON and reads it back. The 270 x 256 nested-anchor case keeps the root and both siblings, then serves the next read from cache.

Earlier findings, rechecked on this head

  1. Scoped refusals no longer shadow explicit reads. URL lookups filter rows by origin policy (PageCache.ts:75-87), and acceptsOrigin repeats the check (WebResearch.ts:145-148). Restricted rows always have restriction_origin = origin(url), because children are filtered to the root's final origin.
  2. Late aliases are reused and deduplicated. Restricted hops reuse fresh, compatible targets, keeping the target's real fetchedAt and expiresAt (WebResearch.ts:321-339,431-438). The loop deduplicates by successful final URL and still counts the attempt (:655-663).
  3. The 64 MiB ceiling is shared by writes and reads, and an oversized write is rejected before the index row is published.
  4. redirectTarget matches the requested URL or the stored final URL. Only ok rows advertise final_url (PageCache.ts:129).
  5. Nested anchors produce a per-page provider_error. The root and siblings are kept, and no unreadable row is persisted.

Rebase and runtime

  • git range-diff a9da7866dc..c3e66983de fe1bfe3857..5175cca646 shows all 8 authored commits as =.
  • git diff a9da7866dc fe1bfe3857 leaves webResearch/, the MCP toolkit and the integration test unchanged. The base changes only comments in McpHttpServer.ts and publicProxy.test.ts, and the web research layer wiring is identical.
  • Relied-on Effect sources are byte-identical between 4.0.4 and 4.0.5: Cache.ts, Equal.ts, Hash.ts, Data.ts, SchemaGetter.ts, SchemaTransformation.ts and Semaphore.ts.
    • Interrupted lookups are still dropped (Cache.ts:641-647), and a shared flight survives one reader leaving (:682-697).
    • PageRequest keys still compare structurally through default Equal/Hash, so restricted and unrestricted flights stay separate.
    • The Effect.ts and internal/effect.ts changes are transaction metadata and interrupt-cause bookkeeping. They do not touch the timeout and race paths used here.
  • URL.parse is available on the supported runtimes: the engine is ^24.13.1 and Electron is 44.4.5.

Proof inspected

  • /tmp/chromeria-441-r6-proof-identity holds this head and tree 964569e42b, which matches git rev-parse HEAD^{tree}.
  • The test, typecheck, lint and knip logs each end with this head and exit_code=0. The test log shows 4 files and 89 tests passed. The typecheck log has only pre-existing suggestions elsewhere.
  • The logs do not record their commands. The file and command list comes from the durable proof comment.
  • git diff --check is clean, and the added tests contain no sleeps or timers.

Cumulative acceptance (#441)

These all hold in the code and in the MCP fixtures:

  • depth one, at most 6 pages, the default pattern, disable and literal case-insensitive | override with no regex
  • normalisation, deduplication and cycles
  • refusal of external/private destinations and off-origin redirects
  • the shared cache, limiter and in-flight cancellation
  • per-page provenance
  • partial page failures
  • the 30 s total deadline including the root, with uncached root queue expiry
  • the shared 20k output limit, with truncationReasons, pendingSubpages and unvisitedSubpages
  • no traversal for cacheId or nonzero-offset reads

Scope stays server/MCP only. The T3_CODE_ORCHESTRATION_INSTRUCTIONS text stays accurate, and the docs/map.md secondary ref follows the map rule.

Cleared, not findings

  • When the root uses the whole 20k output budget, no child is fetched and every candidate is listed as pending. This is documented as explicit continuation.
  • Origins with Crawl-delay of 30 s or more return WebResearchBudgetError on the first default read. The retry, or followSubpages:false, succeeds, as the tool description says.
  • A legacy row whose final URL is off-origin can still win the SQL LIMIT 1 and then fail acceptsOrigin. The only cost is a refetch.

Not verified: live network behavior and end-to-end MCP clients. Neither is required for this layer.

@lukemaj
lukemaj force-pushed the feat/441-web-read-subpages branch from 5175cca to 3886489 Compare October 18, 2026 19:20
@github-actions github-actions Bot added size:XXL and removed size:L labels Oct 18, 2026
@lukemaj
lukemaj changed the base branch from feat/423-web-read-core to main October 18, 2026 19:20
@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: PR #454 is retargeted to merged main and all refreshed local proof passes on the exact candidate.
Why: #449 was squash-merged, so child commits had to be carried from their historical parent boundary without replaying the core or losing incoming main work.
So what: Fresh exact-head Opus 5.5/xhigh review and CI remain required; the #423 parent owns the serialized merge and this task will not self-merge.

  • PR: feat(research): follow same-site subpages in web_read #454, base branch main.
  • Exact base: c1d3960a1d1c0da46e714553ec32bf08b3136844; base tree 7964231a16f885ef6c5b59f9d8ccab6a4b22c3f9.
  • Exact head: 388648949738ea8320bc4a780d96b39b55db94cb; tree 9a6be0a97db74150f52e02a8697c75856ab3c775.
  • Actual historical boundary: fe1bfe38579aba2e81390e829ad66fdc7eda402b, prior reviewed child 5175cca6464da3fec1c56eb921b46dc9dec7026b. Old child history remains at local proof/441-reviewed-5175cca646; the core branch was not edited or deleted.
  • Rebase: git rebase --onto c1d3960a1d1c0da46e714553ec32bf08b3136844 fe1bfe38579aba2e81390e829ad66fdc7eda402b; all eight child commits replayed without conflicts. Range-diff reports = for every authored commit. One subsequent map-only commit replaces the closed feat(research): read public pages with shared cache and provenance #449 reference with this open PR. Total: nine child commits; eight changed files, the same research implementation as the prior reviewed candidate.
  • Integrity: the research code, tests, MCP hooks and frozen dependencies are byte-identical to the prior candidate. New main's observer source/tests and feature registry are byte-identical to the base in this child. The main commit is an ancestor, and the merged core's complete source is present. No feature API changes, dependency changes or versions.

Every command below ran with CI=1 NO_COLOR=1 from the exclusive task workspace and ended on the same clean head/tree. Logs include exact base/head/tree, command, lockfile hash, Warsaw start/finish times and exit code.

Check Result Full local log SHA-256
tests PASS: 4 files, 89 tests, 17.57 s /tmp/chromeria-441-r7-tests.log d7d36e2503e1c03131df1a418364ba484d5c492c44e86464b1314f92d138a73f
typecheck PASS: no errors; existing Effect suggestions elsewhere /tmp/chromeria-441-r7-typecheck.log af458fab85743d68c39ac3e397b7ae991475e75145d0d8449541e74b696750f9
lint PASS: seven changed TypeScript files /tmp/chromeria-441-r7-lint.log 981000e8e28920a9cb29e9de8089e9c3e257f0f4964fed0bcdce3194bf9ecd95
knip PASS: server exports /tmp/chromeria-441-r7-knip.log ad912da5782be461b67c06cfc2cd78cca5c26f7318068a53ee1193405fbdac6c
fork PASS: stack model and all 46 features /tmp/chromeria-441-r7-fork.log 616b7b0ba2abbeb149926436b4f68751a986f9c267e81bf5b70e79adf427a29e
diff PASS /tmp/chromeria-441-r7-diff.log 27990994716f10d8d984843c166ecfb928adc78bc12177568f58d2f481aa8ad4

Commands:

node_modules/.bin/vp test run apps/server/src/mcp/toolkits/webResearch/handlers.test.ts apps/server/src/mcp/WebResearch.integration.test.ts apps/server/src/webResearch/PublicHttp.test.ts apps/server/src/htmlRender/publicProxy.test.ts
node_modules/.bin/tsc-rs --noEmit -p apps/server/tsconfig.json
node_modules/.bin/vp lint apps/server/src/mcp/WebResearch.integration.test.ts apps/server/src/mcp/toolkits/webResearch/handlers.test.ts apps/server/src/mcp/toolkits/webResearch/tools.ts apps/server/src/webResearch/PageCache.ts apps/server/src/webResearch/WebResearch.ts apps/server/src/webResearch/extract.ts apps/server/src/webResearch/schemas.ts
node_modules/.bin/knip --workspace apps/server --exports --preprocessor ./scripts/knip-schemas.ts --no-config-hints
bash scripts/fork-check.sh --upstream upstream
git diff --check c1d3960a1d1c0da46e714553ec32bf08b3136844...HEAD

Decisive test output:

Test Files  4 passed (4)
     Tests  89 passed (89)
  Duration  17.57s
worktree_unchanged=true
exit_code=0

The registered MCP tests use the real reader/compressed cache with HTTP fixtures. Integrations drain OrchestrationEffectWorkerV2 before/after, preserve thread state, and check bounded wire output/continuation. Timing remains TestClock/Deferred, with no sleeps or polling. All six prior review regressions and the latest core queue/active-HTTP/cancellation tests remain intact; no assertion was weakened.

The initial optional fork-check invocation incorrectly supplied fork main as its upstream baseline and consequently classified fork files as upstream edits. That invocation was discarded (retained at /tmp/chromeria-441-r7-fork-invalid-main-base.log); the documented --upstream upstream invocation passed against actual upstream merge-base d0918f1659f4c19172796c8d1910ca6e4bc18b51, including all 46 feature ownership records. No source fix was needed.

Limits: local verification is scoped under repository policy; complete repository checks remain CI-owned. No live network, real external MCP-client, browser/rendering, multi-environment or adoption proof is claimed for this server/MCP slice; those are separate #423 tickets. There is no frontend/browser behavior in this diff. No deployment, restart, machine/service/config activation or version bump. User approval covers the ordered merges, but the parent alone executes #454's merge after current gates pass.

@lukemaj lukemaj closed this Oct 18, 2026
@lukemaj lukemaj reopened this Oct 18, 2026
@github-actions github-actions Bot added size:L and removed size:XXL labels Oct 18, 2026

@lukemaj lukemaj left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What: Round 7 independent review of PR #454 passes with no blocking findings. The rebase is faithful, and I have one low, optional old-cache note.
Why: The research source, tests and dependencies are byte-identical to the round 6 candidate. The merged core and the incoming #457 work are intact, and current-head proof and CI are green.
So what: The #423 parent can carry #454 through its ordered merge once its own gates pass. The note below needs no change before merge.

Candidate

  • PR #454, OPEN, target main.
  • Head 388648949738ea8320bc4a780d96b39b55db94cb, tree 9a6be0a97db74150f52e02a8697c75856ab3c775.
  • Base c1d3960a1d1c0da46e714553ec32bf08b3136844 (#449 squash), tree 7964231a16f885ef6c5b59f9d8ccab6a4b22c3f9, parent 45a30107ba.
  • I re-read the live PR and remote refs just before posting. Both match.

Rebase integrity (verified)

  • git range-diff fe1bfe3857..5175cca646 c1d3960a1d..3886489497 shows = for all eight child commits. The only new commit is map-only commit 9.
  • Diff from base to head: the same 8 files and +1335/−96 as fe1→old-head. The research, MCP and htmlRender paths have zero diff from old head 5175cca646 to head.
  • The old head and this head differ only in #457's observer source/tests, the fork-features.md registry and docs/map.md. None of these files is in base→head, so the incoming main work is byte-identical to base. pnpm-lock.yaml has no diff; SHA-256 82d9eede…3dfc matches the proof logs.
  • The merged core is complete: feat/423-web-read-core at 0072aecc89 (local and origin, preserved) has the same tree as base c1d3960a1d. Research paths are identical between fe1→base. The #457 changes touch no research code.
  • Map net change: WR: #423 PR #449 → WR: #423 PR #454. This follows the secondary-ref rule, because #454 closes #441, not #423.
  • proof/441-reviewed-5175cca646 is retained.

Complete-candidate review

I read the full base→head diff and the surrounding owners: WebResearch.ts (limiter, request, fetchPage, flights, loadPage, read), PageCache.ts, extract.ts, schemas.ts, tools.ts, handlers.ts and PublicHttp bounds. I rechecked all six earlier fixes against the code and their MCP regressions:

  1. Policy-compatible cache selection.
  2. Alias/returned-page deduplication, with the alias still counted against the page budget.
  3. A shared 64 MiB write/read ceiling, rejected before blob/index publication.
  4. Successful-only final_url advertisement, with true body age preserved.
  5. A separate link-metadata budget that keeps the root and siblings.
  6. Exact serialized UTF-8 link accounting. The bound holds: body ≤4 MiB, text ≤4 Mi chars (≤24 MiB escaped), links ≤32 MiB, title ≤500, so the total stays under 64 MiB.

Acceptance holds: default/override/disable, ≤6 same-origin depth-one pages, refused external/private destinations and redirects, a shared limiter/cache, per-page provenance, partial failures, a 30 s budget, a shared 20k output, explicit truncation and continuation, and no traversal on cacheId or nonzero-offset reads. Structural PageRequest keys are proved by the concurrent-shared-fetch test (3 observed requests).

Findings

Low, non-blocking, optional. An alias redirect can turn an anchorless pre-upgrade cache body into an entry that claims to have no links.

  • Where: apps/server/src/webResearch/WebResearch.ts:321-338, with PageCache.ts:76-79.
  • Trigger:
    1. A restricted child A redirects within the origin to T.
    2. T's only fresh entry was written by the core-only build, so it has no links. The legacy index row matches final_origin IS NULL, and acceptsOrigin passes via T's same-origin finalUrl.
    3. The reuse returns links: stored.links ?? [], and A is published with links: [].
    4. A later explicit web_read({url: A}) accepts that entry, because the requireLinks check at WebResearch.ts:505 treats the defined [] as having links. It returns the cached page and follows no subpages. It reports truncated:false and unvisitedSubpages:0.
  • Contrast: a direct read of T correctly refetches and upgrades.
  • Impact: at most 7 days of missing automatic traversal for explicit reads of such an alias. No text is lost, because the anchors stay as markdown links in text, and maxAgeHours:0 bypasses it.
  • Why it doesn't block: it is reachable only when a core-only build wrote cache entries before this layer merges.
  • Evidence: traced in the source; I did not run it.
  • Optional fix: add stored.links !== undefined to the redirect-reuse condition.

Nothing else blocking or actionable. I cleared these concerns:

  • A failed child's finalUrl suppresses a later identical candidate, but the failure stays visible.
  • Legacy rows can shadow older compatible rows, which only causes a refetch.
  • Flights shared by intent cannot hand a links-less page to a follow reader, because only legacy entries lack links and the flight recheck is stricter.
  • WebResearchBudgetError maps to orchestration_error with retry guidance.

Verification

  • I inspected the proof logs myself, not only their names. SHA-256 digests match comment 6254094064 for tests, typecheck, lint, knip, fork and diff.
  • Each log records the exact base/head/tree, the command, the lockfile digest, Warsaw times, worktree_unchanged=true and exit_code=0.
  • Results: 89/89 tests in 4 files; typecheck 0 errors (245 suggestions, none new from this diff); lint on 7 files; knip exports.
  • Fork check reports fork-features: OK (46 features) and fork-check: OK against upstream merge-base d0918f1659. The discarded invalid-base fork log is acknowledged and was not used.
  • The tests use no sleeps or polling and TestClock/Deferred only. The core queue/active-HTTP regressions keep their assertions; the only change to an existing core test adds the legacy-index migration path.
  • GitHub CI on this exact head: Check, Lint, Build and Typecheck, Test, Test Server 1-8 and Fork Stack Model all succeeded. The parent still owns final CI acceptance.
  • Limits: I reran nothing and ran no probe. There is no live network, external MCP client, browser or multi-environment proof, and this slice requires none. I wrote nothing beyond this review and the review/independent status.

Reviewer: Claude Opus 5.5 (claude-opus-5-5) through the Claude Code harness in T3 Code, as a read-only delegated task with no further delegation. xhigh effort was requested; I cannot observe the runtime effort setting from inside the session.

@lukemaj
lukemaj merged commit def07e9 into main Oct 18, 2026
40 checks passed
@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: PR #454 merged into main as def07e920a27c4d266b47cf15ecde4be0db70c27; #441 is closed.
Why: The current candidate passed CI and independent Claude Opus 5.5 round 7 in the user-approved ordered landing window.
So what: #453 is next and must refresh its integration, proof and review on this exact predecessor; this parent continues to serialize main merges. No deployment or restart occurred.

Merge input: head 388648949738ea8320bc4a780d96b39b55db94cb, base c1d3960a1d1c0da46e714553ec32bf08b3136844. Used repository-supported squash with --match-head-commit. GitHub confirms MERGED and #441 CLOSED; remote main matches the merged SHA. First parent is c1d3960, preserving #449, #457 and native upstream ancestry. Merged tree 9a6be0a97db74150f52e02a8697c75856ab3c775 equals the reviewed tree exactly; git diff --exit-code 388648949738ea8320bc4a780d96b39b55db94cb origin/main passed.

Evidence: 89-test scoped proof, CI39158544642, Fork39158544596, Opus R7 PASS, and transfer report. Latest review/independent success was created by authenticated lukemaj on the exact input head. Latest fingerprint success also verified. Transfer measurements show zero measured Codex/Claude regression within the enforced ceilings; no broader performance inference.

The reviewer’s optional legacy-cache alias note is nonblocking and is retained under #447. No core-only build was deployed in this task. The #441 owner retains cleanup ownership of its workspace and will preserve only resources still needed by #442/#443; root does not remove another active owner's workspace. Core/rebase proof refs remain retained while descendants establish their final predecessor boundaries. This is an active-stack retention exception, not completed cleanup. #443 stays open for pre-activation cache lifecycle work. No deployment, staging, restart, machine configuration, public release or version bump is part of this landing.

@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: #455 does not need #441's historical branch or worktree; its own exact history is retained independently.
Why: Local ref proof/443-pre-main-1860d4850b retains child head 1860d4850bb02a865c10b1669d0e0e6ee1db05db, all four child commits and actual historical parent 0072aecc89a8847662a3d1b4b5dff225c6035aaf. Merged #454 def07e920a27c4d266b47cf15ecde4be0db70c27 retains tree 9a6be0a97db74150f52e02a8697c75856ab3c775, identical to reviewed head 388648949738ea8320bc4a780d96b39b55db94cb.
So what: This releases #443's dependency on #441's branch/worktree only; #442 must confirm its own needs before the #441 owner retires resources. #455 still waits for ROOT's actual #453-MERGED notice before final rebase/proof/review/CI; ROOT alone executes main merges.

Verified git merge-base --is-ancestor 0072aecc89a8847662a3d1b4b5dff225c6035aaf proof/443-pre-main-1860d4850b (exit 0), four child commits, clean exclusive workspace, GitHub #454 MERGED and remote main at the merge SHA. No other owner's branch or workspace was edited. Shared core/subpage/render behavior remains required in the final candidate. #443 remains OPEN for pre-activation retention/pruning; no deployment, staging, restart, host configuration or activation.

@lukemaj
lukemaj deleted the feat/441-web-read-subpages branch October 18, 2026 20:50
@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: #443 releases its dependency on #441 and #442's historical branches/workspaces, including their old core references.
Why: This task independently retains proof/443-pre-main-1860d4850b at 1860d4850bb02a865c10b1669d0e0e6ee1db05db (all four host commits and actual parent 0072aecc89a8847662a3d1b4b5dff225c6035aaf) and proof/443-render-predecessor-19a987a505 at reviewed renderer head 19a987a5051ae99c10e0f2657ec922f05e898b8c. Its slice is now rebased onto merged main f27d8ee146295ed2b76d8d15d415dd7cff574df5, whose tree equals that renderer head.
So what: #441/#442 owners may retire their own resources after their other consumers confirm; #443 needs no branch/worktree owned by them or the core owner. Do not remove these #443-owned proof refs or its active feat/443-research-host workspace. ROOT still owns main merges; #455's final integration/proof/review is in progress.

Final integration identity/authority. No other owner's branch/workspace was modified by this task. This confirmation releases only #443's dependencies and is not cleanup authority over any other owner's work.

lukemaj added a commit that referenced this pull request Oct 29, 2026
Follow matching same-origin depth-one subpages through the shared research service, cache and limiter, with per-page provenance, bounded output and partial failures.

Closes #441. Refs #423.

Proof: 89 focused MCP/runtime tests with drained effects and no sleeps; scoped typecheck/lint/exports/fork checks; CI39158544642 and Fork39158544596 passed on388648949738ea8320bc4a780d96b39b55db94cb.
Independent review: Claude Opus5.5 R7, review5564609099, PASS on exacthead.
User-approved ordered landing coordinated on411. No deployment/restart or version bump.

Authored with Codex through T3 Code; reviewed with Claude Opus5.5 through Claude Code in T3 Code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(research): follow same-site subpages in web_read

1 participant