ci: stop reaching into the private dcd repo for the mock-api - #124
Merged
Conversation
`lint-and-test` has failed on every same-repo PR since dcd#1036 deleted `mock-api/` from the private devicecloud-dev/dcd repo this morning. CI checked that directory out over an SSH deploy key and ran `pnpm install` in it; the sparse-checkout now matches nothing, so the job dies at that step — before the linter — and takes #120, #122 and #123 down with it. Rather than re-point at a mock, this removes the linkage. dcd-cli is PUBLIC and was holding `DCD_SSH_DEPLOY_KEY`, a credential granting read access to the private repo, and pulling the API's `swagger.json` onto the runner on every same-repo PR. Deleting the checkout drops both. * The `Checkout dcd (mock-api)` and `Install Mock API dependencies` steps are gone, along with the `HAS_PRIVATE_ACCESS` gate that existed only to keep them off fork and Dependabot PRs. Every PR now takes the same path, so forks stop being second-class. * CI runs `pnpm test:unit` — a new script that is the existing runner with `--unit`. `test/unit/*` is pure and needs no backend, so unit coverage is kept rather than dropped along with the integration suite. * `scripts/test-runner.mjs` no longer defaults `MOCK_API_DIR` to `../../dcd/mock-api`. With no mock available it degrades to the unit suite and says so, instead of the bare ENOENT it throws today. Set `MOCK_API_DIR` and the integration specs run exactly as before. `DCD_SSH_DEPLOY_KEY` can now be deleted from the repo's secrets — nothing reads it. That is a separate manual step, not something this commit can do. Two things are genuinely lost, both worth stating plainly rather than discovering later: * `test/integration/*` no longer runs anywhere automatically. * With it goes the CLI<->swagger contract-drift check. Drift used to surface as a Prism 422 — that is how the `googlePlay` multipart break and the `tempPath` missing-example break were both caught. Nothing replaces it yet. Verified locally: `pnpm test:unit` and a bare `pnpm test` both run the unit suite only and print the notice; 81 pass and the 7 `flow-paths` failures are Windows-only, asserting POSIX paths against win32 `path`. The same specs ran green on ubuntu in the last full CI run (job 94750122384, 2026-08-14), which is the platform CI uses. `pnpm lint`, `pnpm typecheck`, `pnpm build` and `pnpm audit --audit-level moderate` are all clean.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Follow-up to 5a713f6, which changed how CI treats the mock-api but only updated CLAUDE.md's Commands section — leaving three descriptions of the machinery it removed. Flagged on #124 for CLAUDE.md; CONTRIBUTING.md and README.md carried the same claim and are the ones contributors actually read. * CLAUDE.md's Contributing bullet said integration tests need the private devicecloud-dev/dcd mock-api via DCD_SSH_DEPLOY_KEY, that `pnpm test` is skipped on fork/Dependabot PRs, and that a maintainer runs the full suite before merge. None of that is true now: every PR runs identical steps and nothing runs the integration suite. * CONTRIBUTING.md's "About the test suite" said the same, framed as forks being the special case. Rewritten around the actual split — test/unit/* everywhere, test/integration/* only with MOCK_API_DIR set — and the reason CI does not reach for a mock: this repo is public and holds no credentials for private infrastructure. * Both command tables and README's quickstart now list `pnpm test:unit` and note it is what CI runs. Each of the three states the consequence rather than burying it: a green PR says nothing about the integration suite. CONTRIBUTING.md asks contributors touching the API surface to flag it so a maintainer can exercise it before merge. Docs only — no workflow or script changes.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
14 tasks
finalerock44
added a commit
that referenced
this pull request
Aug 18, 2026
Promote from dev to stable, carrying the documentation half of #124 that the 5.3.1 promotions dropped. `ci: stop reaching into the private dcd repo for the mock-api` (#124) touched six files on dev. The promote PR (#123) carried four of them — `cli-ci.yml`, `scripts/test-runner.mjs`, the `test:unit` script in `package.json`, and eight of the ten changed `CLAUDE.md` lines — and dropped `CONTRIBUTING.md` and `README.md` entirely. Production therefore ships the new CI shape while its docs still describe the old one: * `CONTRIBUTING.md` told contributors `pnpm test` boots a mock API, and that integration tests are "automatically skipped" on fork PRs only. Neither is true: CI runs no integration tests on any PR, and there is no default mock. It also omitted `pnpm test:unit` from the pre-push checklist, though it is now a required check. * `README.md`'s dev-scripts block still showed `pnpm test # build + boot mock API + integration/unit tests`. * `CLAUDE.md`'s Contributing section still described the `DCD_SSH_DEPLOY_KEY` mock-api checkout, which no longer exists. `README.md` is the one with reach beyond this repo: npm always includes it in the tarball regardless of the `files` field, so the stale snippet would render on the npmjs.com page for 5.3.1. Docs only — no source, workflow, script or lockfile change. The three files are now byte-identical to `dev`, leaving release plumbing (both manifests, `CHANGELOG.md`, `package.json` version) as the only remaining divergence, which is release-please's to own. `Release-As: 5.3.1` keeps the pending release PR (#120) on 5.3.1 rather than rolling it to 5.3.2. Release-As: 5.3.1
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
1 participant
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.
lint-and-testhas failed on every same-repo PR since dcd#1036 deletedmock-api/from the private devicecloud-dev/dcd repo this morning. CI checked that directory out over an SSH deploy key and ranpnpm installin it; the sparse-checkout now matches nothing, so the job dies at that step — before the linter — and takes #120, #122 and #123 down with it.Rather than re-point at a mock, this removes the linkage. dcd-cli is PUBLIC and was holding
DCD_SSH_DEPLOY_KEY, a credential granting read access to the private repo, and pulling the API'sswagger.jsononto the runner on every same-repo PR. Deleting the checkout drops both.Checkout dcd (mock-api)andInstall Mock API dependenciessteps are gone, along with theHAS_PRIVATE_ACCESSgate that existed only to keep them off fork and Dependabot PRs. Every PR now takes the same path, so forks stop being second-class.pnpm test:unit— a new script that is the existing runner with--unit.test/unit/*is pure and needs no backend, so unit coverage is kept rather than dropped along with the integration suite.scripts/test-runner.mjsno longer defaultsMOCK_API_DIRto../../dcd/mock-api. With no mock available it degrades to the unit suite and says so, instead of the bare ENOENT it throws today. SetMOCK_API_DIRand the integration specs run exactly as before.DCD_SSH_DEPLOY_KEYcan now be deleted from the repo's secrets — nothing reads it. That is a separate manual step, not something this commit can do.Two things are genuinely lost, both worth stating plainly rather than discovering later:
test/integration/*no longer runs anywhere automatically.googlePlaymultipart break and thetempPathmissing-example break were both caught. Nothing replaces it yet.Verified locally:
pnpm test:unitand a barepnpm testboth run the unit suite only and print the notice; 81 pass and the 7flow-pathsfailures are Windows-only, asserting POSIX paths against win32path. The same specs ran green on ubuntu in the last full CI run (job 94750122384, 2026-08-14), which is the platform CI uses.pnpm lint,pnpm typecheck,pnpm buildandpnpm audit --audit-level moderateare all clean.What & why
Type of change
fix— bug fixfeat— new featureperf— performance improvementrefactor— code change that's neither a fix nor a featuredocs— documentation onlychore/ci/build/test— tooling, no user-facing change!or PR notes aBREAKING CHANGE:)Checklist
pnpm lintpassespnpm typecheckpassespnpm buildpassesCHANGELOG.md(release-please handles this)README.md/STYLE_GUIDE.mdupdated if behaviour or output changedHow to test