Repository navigation
docs: describe safe local Directory verification - #88
Conversation
The SDK install block was meant to be pasted into an interactive shell, where a failed fetch or checkout (or an empty pin) did not stop it, so npm ci and the build ran on the SDK's default-branch HEAD instead of the pinned revision. Run it in a set -euo pipefail subshell, refuse an existing vendor path that is not a clean checkout of its own, and verify HEAD equals the pin before building. Reruns on a clean checkout no longer fail on the clone. Also: seed data/registrations.json before next dev, say the checks match CI's commands rather than its install, point at the CI env block for the placeholder origins, and link the workflow from the Directory README. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a271dcc05
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| created, so copy it before the first `next dev`: | ||
|
|
||
| ```bash | ||
| [ -f data/registrations.json ] || cp data/registrations.example.json data/registrations.json |
There was a problem hiding this comment.
Seed the catalog before running unit tests
On a fresh worktree following this document in order, the earlier npm test imports src/catalog/store.ts (for example from test/current-indexer.test.ts), which creates data/directory.sqlite and completes the one-time migration while registrations.json is absent. Copying the seed here afterward does not reload it because the database already contains its schema marker, so the subsequent development server has an empty registration catalog. Move this copy before npm test, or explicitly recreate/reseed the database before starting development.
Useful? React with 👍 / 👎.
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>
docs: describe safe local Directory verification
What & why
Documents a worktree-local contributor workflow for verifying the DACS Directory with Node 22, the exact SDK revision already pinned on main, locked dependency installation, isolated runtime data, and loopback-only development. It does not add setup wrappers, business logic, or new requirements. The Directory README now links to it from its "Run it" section.
Local verification
Original run (August base
074f040; not repeated, since this environment has no network and no vendored SDK, see Review round 1):These are local verification results, not a claim that remote CI passed. Because this PR now touches
reference-implementations/dacs-directory/README.md, the path-filtered Directory CI workflow will run on it; its result is the authoritative check of the application at head1a271dc.Review
An independent read-only DACS reviewer requested removal of a machine-specific NVM path. That finding was resolved, and the reviewer approved the revised documentation with no remaining findings.
Review round 1
Two reviews (code, Daybreak) of
f33da6e: both "ready", the code review with one P2 and several P3s. The branch now includes main (831082f, #94) via a merge commit; there were no conflicts.P2 (fixed) — the SDK pin guard did not stop a paste.
test "${#sdk_rev}" -eq 40only set$?in an interactive shell, and nothing checkedfetchorcheckout. If the pin could not be read (format drift, CRLF) or the fetch/checkout failed after the clone,npm ci && npm run buildran on the SDK's default-branch HEAD. The install block now runs in aset -euo pipefailsubshell, exits with a message if no 40-hex pin is found, and builds only aftergit rev-parse HEADequals the pin.P3s fixed
vendor/dacs-sdk(Daybreak): the clone is skipped when the path exists. An existing path must be its own Git checkout (not a directory inside another repository) with no local changes, else the block stops and leaves it alone. A clean checkout at any commit is re-pinned.npm cirather thannpm run setup.next dev: the doc copiesdata/registrations.example.jsonfirst (only read when the SQLite catalog is first created).nvm use 22: nowfor example nvm use 22plus anode --versioncheck.env:block of the CI workflow instead of presentingNEXT_PUBLIC_BUTLER_ORIGINas fixed.Follow-ups, not done here
NEXT_PUBLIC_BUTLER_ORIGINfrom thecheck:deploy-configandbuildexamples, theunset RUN_LIVE_PAID_E2E LIVE_BUTLER_ORIGINline, and the "live/paid browser tests" sentence.scripts/setup-sdk.sh(it needsnpm ci, which the script does not do) and does not handleDACS_SDK_GITHUB_TOKEN. Teachingsetup-sdk.shannpm cimode would remove the duplication, but that is application code and out of scope for a docs PR.codex-local-workflow.md) while the title is generic. Not renamed, to keep the link and history stable.npm ci, typecheck, tests and build were not re-run after the merge (no network, no vendored SDK here). CI on this head covers them.Verification of the fix
set -eshell against a local stand-in SDK repo (10 scenarios: fresh, rerun clean at pin and at another commit, pin format drift, CRLF pin, failed fetch, failed checkout, checkout that silently does nothing, dirty vendor, vendor that is not its own checkout). Before the fix 9 of 10 failed (built the unpinned HEAD or errored on rerun); after the fix all pass.set -e, no HEAD==pin check, no dirty check, no own-checkout check, no pin-length check, unconditional clone, checkout of HEAD instead of the pin) are all caught by the probe.@kynesyslabs/dacs,vendor/dacs-sdk); unit tests 85 pass / 24 files not loadable (absent SDK), same per-test results;check:deploy-configwith the CI placeholder origins passes.next dev --hostname 127.0.0.1 --port 0binds a free loopback port.Scope
Changes:
docs/codex-local-workflow.mdand a link inreference-implementations/dacs-directory/README.md. No secrets, wallet material, runtime state, lockfiles, business logic, deployment, merge, release, or cleanup operations are included.Head:
1a271dcComments
Reviews
🤖 Generated with Claude Code