Skip to content

docs: describe safe local Directory verification - #88

Merged
randomblocker merged 4 commits into
mainfrom
codex/community-local-verification-docs
Oct 13, 2026
Merged

randomblocker merged 4 commits into
mainfrom
codex/community-local-verification-docs

Conversation

@randomblocker

@randomblocker randomblocker commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

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):

  • Node 22.22.0
  • SDK pin 44d8ff2a07df8c951b94619d20b957b4bb5ce140
  • SDK npm ci and build: passed
  • Directory npm ci: passed
  • npm run typecheck: passed
  • npm test: passed (201/201, at the August base; the suite has changed since)
  • npm run check:deploy-config with CI placeholder origins: passed
  • npm run build with CI placeholder origins: passed after transient TLS retries
  • Browser tests: skipped; documentation-only change
  • Indexer and live/paid tests: not run

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 head 1a271dc.

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 40 only set $? in an interactive shell, and nothing checked fetch or checkout. If the pin could not be read (format drift, CRLF) or the fetch/checkout failed after the clone, npm ci && npm run build ran on the SDK's default-branch HEAD. The install block now runs in a set -euo pipefail subshell, exits with a message if no 40-hex pin is found, and builds only after git rev-parse HEAD equals the pin.

P3s fixed

  • Rerun with an existing 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.
  • Not linked from the Directory README (both reviews): link added.
  • "Local equivalent of CI" overstated, install differs: now says the check commands match CI and the install uses npm ci rather than npm run setup.
  • Fresh worktree had no ReviewBot seed for next dev: the doc copies data/registrations.example.json first (only read when the SQLite catalog is first created).
  • "Use your version manager" vs hard-coded nvm use 22: now for example nvm use 22 plus a node --version check.
  • Stale once Separate the gateway demo from the Community Directory #84 lands (partly): the placeholder origins now point at the env: block of the CI workflow instead of presenting NEXT_PUBLIC_BUTLER_ORIGIN as fixed.

Follow-ups, not done here

  • When Separate the gateway demo from the Community Directory #84 lands (or this does, second): remove NEXT_PUBLIC_BUTLER_ORIGIN from the check:deploy-config and build examples, the unset RUN_LIVE_PAID_E2E LIVE_BUTLER_ORIGIN line, and the "live/paid browser tests" sentence.
  • The doc still re-implements the clone/fetch/checkout from scripts/setup-sdk.sh (it needs npm ci, which the script does not do) and does not handle DACS_SDK_GITHUB_TOKEN. Teaching setup-sdk.sh an npm ci mode would remove the duplication, but that is application code and out of scope for a docs PR.
  • The file name is tool-specific (codex-local-workflow.md) while the title is generic. Not renamed, to keep the link and history stable.
  • The SDK clone/install/build and the application's own 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

  • A probe pastes the doc's install block into a non-set -e shell 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.
  • 7 mutants of the block (no 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.
  • On the merged tree and on main the offline steps give identical results: typecheck 96 errors on both, all from the absent SDK (@kynesyslabs/dacs, vendor/dacs-sdk); unit tests 85 pass / 24 files not loadable (absent SDK), same per-test results; check:deploy-config with the CI placeholder origins passes. next dev --hostname 127.0.0.1 --port 0 binds a free loopback port.

Scope

Changes: docs/codex-local-workflow.md and a link in reference-implementations/dacs-directory/README.md. No secrets, wallet material, runtime state, lockfiles, business logic, deployment, merge, release, or cleanup operations are included.

Head: 1a271dc

Comments

Reviews

🤖 Generated with Claude Code

random block and others added 3 commits September 4, 2026 22:31
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>
@randomblocker
randomblocker marked this pull request as ready for review October 13, 2026 15:49
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-13T15:50:59.162706Z 1a271dc Draft marked ready
🔒 Security Review ✅ Completed 2026-10-13T15:52:37.941735Z 1a271dc Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

@randomblocker
randomblocker merged commit d1de29d into main Oct 13, 2026
1 of 2 checks passed
randomblocker added a commit that referenced this pull request Oct 14, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant