Repository navigation
Separate the gateway demo from the Community Directory - #84
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The production-origin check accepted https://localhost, loopback and unspecified IP literals, so a mistyped NEXT_PUBLIC_DIRECTORY_URL could pass the deploy gate and poison canonical URLs, the sitemap and llms.txt. - Reject localhost, *.localhost, *.local, and loopback, unspecified, private, CGNAT and link-local IPv4/IPv6 literals (including IPv4-mapped IPv6). - Require the raw variable to be exactly the origin (optionally with one trailing slash), because directoryBaseUrl() uses it verbatim; values like " https://x ", "https://x?" or "https://X" no longer pass. - Assert the specific error for each rejection, add the password-only credential case, and move CI's directory URL off .local. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-config The deploy-config check rejected loopback, private, CGNAT and link-local literals but still passed other addresses that cannot be a deployment's public origin, such as https://192.0.2.1, https://224.0.0.1, https://[2001:db8::1], https://[fec0::1] or https://[64:ff9b::7f00:1]. It also passed single-label and private-use names (https://directory, https://directory.railway.internal) and names with empty labels (https://localhost..). - IPv4 literals: also reject the remaining RFC 6890 special-use ranges that are not globally reachable (192.0.0.0/24, 192.0.2.0/24, 192.88.99.0/24, 198.18.0.0/15, 198.51.100.0/24, 203.0.113.0/24), plus multicast, reserved and broadcast (224.0.0.0/4, 240.0.0.0/4). - IPv6 literals: accept only global unicast (2000::/3) outside the IETF-protocol (2001::/23, including Teredo), documentation (2001:db8::/32, 3fff::/20) and 6to4 (2002::/16) blocks. IPv4-mapped addresses still follow the IPv4 rules; NAT64, IPv4-compatible and site-local forms are now rejected. - Host names: require at least two non-empty labels (one trailing dot is still allowed) and also reject .internal, .home.arpa and .localdomain. - Tests pin both edges of every blocked range, so each range can neither shrink nor grow, and cover the special-use, transition-form and name cases. The README states the full contract. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docs/codex-local-workflow.md arrived from main (#88) after this branch retired the gateway demo, so it still named that demo's variables and told contributors to run NEXT_PUBLIC_DIRECTORY_URL=https://directory.ci.local \ NEXT_PUBLIC_BUTLER_ORIGIN=https://agents.ci.local \ npm run check:deploy-config check:deploy-config now rejects .local hosts, so that command exits 1. - Pass only NEXT_PUBLIC_DIRECTORY_URL=https://directory.ci.example, the value CI sets, to check:deploy-config and build, and say why a local origin must not be substituted. - Drop `unset RUN_LIVE_PAID_E2E LIVE_BUTLER_ORIGIN` and the "live/paid browser tests" wording; that suite no longer exists. - Add a test that reads the CI workflow and the guide: CI's origin must pass the check, every NEXT_PUBLIC_* value the guide passes must equal CI's, and the guide must not name the retired variables. CI does not build docs, so this is what keeps the guide runnable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
https://directory.example:0 passed as the production origin, but no browser can connect to port 0, so every canonical URL, sitemap entry and discovery document built from it would be unusable. URL parsing also turns :00 into :0, so the check reads the parsed port. The README now lists port 0 among the rejected forms, and it names the 6to4 relay range (192.88.99.0/24), which the check already blocked. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The landing-page rewrite kept only the "Browse the directory" link's href, so no browser test loaded /discover any more, although the e2e README says the suite covers discovery. Restore main's check that /discover renders its heading. The e2e README also lost the `npx playwright install chromium` step when the live-suite instructions were removed; add it back. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ries - Reject IANA special-use names and their subdomains (test, invalid, onion, alt, everything under arpa) besides localhost/local/internal/localdomain. RFC 6761 documentation names are rejected unless the check runs with --allow-documentation-names; only CI's placeholder step passes it, and railway.json keeps the strict check. - Reject all IPv4-mapped IPv6 literals (outside global unicast), every Fetch Standard bad port, and a trailing-dot host name. - Document the fail-closed deviations (192.0.0.0/24 and 2001::/23 anycast exceptions, NAT64, 6to4 relay) and pin the upstream tables in test/fixtures/origin-policy-sources.json, which the tests check the denylists against. - Compare real paths in the CLI guard so a symlinked invocation still runs. - Test the CLI itself: exit 1 with the [deployment config] message on a missing, insecure or non-public origin; exit 0 and the success line on a valid one; the symlink path; and that importing the module is silent. - Pin Node 22.23.3 in .nvmrc and use it in CI, pin checkout/setup-node by commit SHA, run on ubuntu-24.04, and correct the stale private-SDK comment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CI installs Node from .nvmrc, but package.json engines still allowed any Node 22, and the README asked for a manual RAILPACK_NODE_VERSION service variable. Railpack applies engines.node first and then lets mise's idiomatic version files (.nvmrc) override it (railpack ec5d727, core/providers/node/node.go:419-494), so name the exact .nvmrc release in engines (and the lockfile's root entry): every source Railpack reads now gives 22.23.3, whatever their order. test/node-runtime.test.mjs fails if engines, the lockfile or CI drift from .nvmrc, or if another version file appears. The README and the local guide now describe the pin, and the guide selects the .nvmrc release instead of any Node 22. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…g matching The spawned check inherited the whole test environment, so a runtime warning it enabled reached the asserted stderr: with NODE_USE_ENV_PROXY=1 behind a proxy, Node prints an UNDICI-EHPA warning at startup and four CLI tests failed. Run the children with NODE_NO_WARNINGS=1, and add a test that injects an inherited warning (and checks that it does fire without the guard) while the exact exit status and output assertions stay. Also pin that only the exact --allow-documentation-names flag relaxes the check: a misspelt flag or any other argument leaves it strict. And check that the local guide runs the flagged origin check, since CI's placeholder origin fails the strict one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…gistry notes - The SDK repository is public, so DACS_SDK_GITHUB_TOKEN is optional and only authenticates the clone. The README table and deployment notes and the setup-sdk.sh comment still called it the private SDK's token. - The rejected exceptions inside 2001::/23 are not all anycast; list them (protocol anycast, AMT, AS112, ORCHIDv2, DRIP) next to the PCP and TURN anycast exceptions inside 192.0.0.0/24. - Add the SRv6 SID block 5f00::/16 (RFC 9602) to the fixture's IPv6 rows. CPython v3.15.0's copy lacks it; it lies outside 2000::/3, so the check already rejected it. The fixture and README now say the snapshot is the pinned copies plus the registry rows they lack. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cX3po
reviewed
Oct 14, 2026
cX3po
left a comment
cX3po
left a comment
Contributor
There was a problem hiding this comment.
Read draft head 3dbdbb2dd3454ee3865f68d423efce51f7599c0e against base d1de29d3aca3fda3958a8d3ddc615000a66a2878, scoped to Directory impact.
There are no changes under src/catalog or app/api/dacs in that diff. Navigation retains discover, register, verify and the developer API; discover becomes the featured link. The /try and /try-chat pages and their sitemap entries are removed. The check:deploy-config script switches from gateway-origin to Directory-origin validation; the setup documentation now describes the SDK clone as public with optional authentication. I ran test/directory-origin.test.mjs and test/node-runtime.test.mjs offline on the pinned Node 22.23.3: 23 passed in total, none failed or skipped.
I found no admission-code change in this scope. This does not certify the separate #85 admission work: #84 retains SDK pin 44d8ff2a07df8c951b94619d20b957b4bb5ce140. The removed demo URLs lose their route files; no replacement redirect is added in this diff. Existing links to those URLs will need to be updated when the separate demo is available.
This is a COMMENT on a draft. No browser, gateway, deployed Directory, live indexing or seller execution was tested.
— PATH-OS (cX3po)
…nvmrc release Railway doesn't pin which Railpack release builds the service, so the engines-then-mise precedence the README relies on could change without notice. Check the outcome instead: scripts/check-node-runtime.mjs exits non-zero unless the running Node is the .nvmrc release, and the Railway build command, start-railway.sh and CI all run it first. The version-file guard now covers every mise config file and glob that Railpack v0.41.0 hands to mise (mise/, .mise/, .config/mise*, mise.*.toml, conf.d), not just five names. The README cites the v0.41.0 tag instead of an untagged main commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The origin-policy fixture claimed a complete registry snapshot, but it lacked the IPv6 Dummy Prefix (100:0:0:1::/64), eap.arpa, eap-noob.arpa and the individual reverse-mapping zones. It also accepted the globally reachable AS112, AMT and Direct Delegation AS112 blocks, even though the README says special-purpose blocks are rejected. - Reject 192.31.196.0/24, 192.52.193.0/24, 192.175.48.0/24 and 2620:4f:8000::/48 with the other protocol-only blocks. - Pin both IP registries in full through Wireshark's IANA-generated tables. A new test requires every registry row to be rejected at both edges and traced to a fixture row. - Record each pinned copy as a raw URL at a fixed revision, with its sha256 and line ranges. scripts/verify-origin-sources.mjs (npm run verify:origin-sources, needs network) re-fetches them, checks the hashes and compares the re-extracted rows with the fixture. - Add hard-coded vectors for invalid, onion, alt and eap-noob.arpa. - Add a test for the CLI guard's realpath failure branch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- test/directory-separation.test.ts fails if a gateway route, component, script, package script, Railway/CI/Playwright setting or sitemap entry comes back. It also checks that the sitemap's static routes are exactly the Directory's own pages. - Move the landing page's summary strip into homeCatalogSummary. Unit tests now cover the indexing, indexed-empty and populated states, the verified-only deal count, the 59/60-minute boundary and the clamp for a catalog timestamp in the future. - The home e2e test now asserts /discover's URL and title. The landing page shares /discover's heading, so the old check could not fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The local workflow guide still said the SDK clone might need GitHub authentication; the repository is public. playwright-report-live/ is no longer produced now that the live suite is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The names snapshot traced each name only through Unbound's and Knot Resolver's lists, and eap.arpa appears in neither, so nothing pinned it. Pin IANA's Special-Use Domain Names registry XML itself, byte for byte from a mirror of rsync.iana.org::assignments at a fixed commit, and make verify:origin-sources require the fixture's special-use and documentation names to equal the registry's names in both directions. eap.arpa now cites RFC 9965, which also deprecates eap-noob.arpa. verifyOriginSources takes an injected fetcher, and a new offline test checks it against small in-memory copies with real sha256s: the matching fixture verifies, and flipped or dropped registry rows, a missing CPython row, missing or extra names, a dropped port, a wrong sha256, changed bytes and HTTP 404 are each reported. The parser test now has IPv4 and IPv6 Wireshark rows whose forwardable and global flags differ, so reading the wrong column fails. The Wireshark copy holds every unexpired row and records an N/A Globally Reachable as false; say so, and note that the two terminated blocks IANA still lists are rejected anyway. Document running the verifier with --use-env-proxy behind a proxy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CI's Node always matches .nvmrc, so the CLI's exit 1 branch and the start script's errexit never ran: a check that exited 0 on the wrong release, or a start script without set -e, still passed. Run copies of check-node-runtime.mjs and start-railway.sh in a temp app whose .nvmrc pins 0.0.0, and assert exit 1, the [deployment config] message, and that start:railway stops before the data directory, the indexer and the server. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The summary helper was tested, but app/page.tsx could bind a stat to the wrong field or label and every test would still pass. Render the page against pending, indexed-empty and populated catalogs saved in an isolated data directory, with the clock fixed, and assert the four value/label pairs; the populated values all differ. Also pin the page's link targets and its canonical URL. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
randomblocker
marked this pull request as ready for review
October 15, 2026 13:51
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
2 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.
Separate the gateway demo from the Community Directory
Summary
/tryand/try-chatapplications, Butler contracts/forms, environment variables, probes, and demo tests from the Community Directory.npm run check:deploy-config). The origin must be credential-free, have no path, query or fragment, avoid the Fetch Standard's bad ports, and be written exactly as the origin (one trailing/allowed). Its host must be a multi-label DNS name that is not an IANA special-use name,internalorlocaldomain(documentation names only with--allow-documentation-names, which only CI passes), or a global unicast IP literal outside every block in the IANA IPv4 and IPv6 Special-Purpose Address Registries. The check does not resolve DNS..nvmrcandpackage.jsonengines. CI installs from.nvmrc, and the Railway build andstart:railwayrunnpm run check:node-runtime, which fails unless the running Node is that release.Why
The Community application owns public indexing and discovery. The gateway owns its custodied buyer demo. The Directory should not depend on, select for, or embed one demo gateway.
Impact and rollout
Removal only. The Try DACS demo and the Butler gateway are retired, so nothing replaces them: no gateway-owned UI, no drain step, no recovery-only release, and no redirect or runtime gateway dependency in Community. The live Directory already returns 404 for
/tryand/try-chat(checked 2026-10-15 13:11Z), so no buyer is mid-purchase there.Deployment config change.
NEXT_PUBLIC_DIRECTORY_URLmust now be written exactly as the public origin, with at most one trailing/.check:deploy-configrejects surrounding whitespace, an empty?or#, uppercase host letters, an explicit default port, and non-public hosts. Railway is no longer used: the live Directory runs on our VPS, wherenext buildandnext startdon't callcheck:deploy-configorcheck:node-runtime, so runnpm run check:deploy-configthere before deploying. CI's build origin moves fromhttps://directory.ci.localtohttps://directory.ci.example. LeaveRAILPACK_NODE_VERSIONunset: the build now fails if Railway's builder picks any Node other than.nvmrc's.Where the origin rules come from
test/fixtures/origin-policy-sources.jsonpins:192.88.99.0/24,2001:10::/28) are rejected anyway.rsync.iana.org::assignments, cross-checked with Unbound and Knot Resolver.Each comes from a copy pinned to an upstream revision and sha256, as of 2026-10-15. The unit tests check the module against the fixture offline, and check the verifier against small in-memory copies.
npm run verify:origin-sources(needs network; behind a proxy usenode --use-env-proxy scripts/verify-origin-sources.mjs) re-fetches each pinned copy, checks its sha256, re-extracts the rows to compare with the fixture, and requires the fixture's special-use and documentation names to equal the registry's names exactly.Validation
Results for head
55858c2, on Node 22.23.3:directory-origin,node-runtime,home-page,home-hero-state,directory-separation,discovery-contract): 50/50 pass, at most two processes at a time.tsc --noEmit: clean.npm run verify:origin-sources: all six pinned copies match their sha256 and the fixture. Removing any one of the 42 names from the fixture in memory is reported.set -eand the check's position in the start script, each verifier comparison including the sha256, HTTP status and name checks, reading Wireshark's forwardable column as global, and the homepage's stat bindings, labels, links and canonical URL): 30/30 killed.git diff --check: pass.55858c2. That includese2e/home.spec.ts, which asserts/discover's URL and title, because the landing page shares its heading.Review round 1
scripts/check-directory-origin.mjsrejectslocalhost,*.localhost,*.local, and every non-public IP literal described above, including IPv4-mapped IPv6 forms. The check does not resolve DNS. Tests:test/directory-origin.test.mjs. The README describes the contract and its DNS limitation././discovershare the H1 "Find agents you can verify." (a copy decision).next/linkis used for JSON route handlers on the homepage, in the layout footer and on seller pages. That is an existing main-wide idiom, so it is better changed everywhere in one PR.Review round 2 (
d1a470a).nvmrcrelease.Review round 3 (
55858c2)eap.arpa(RFC 9965) was the one name no pinned copy listed. The verifier now compares the full name set both ways, and is tested offline against in-memory copies.start-railway.shwith a mismatched.nvmrc.🤖 Generated with Claude Code