From 5a713f635e0a45953adb635a55517f5d84b0506b Mon Sep 17 00:00:00 2001 From: finalerock44 <77282157+finalerock44@users.noreply.github.com> Date: Tue, 18 Aug 2026 12:12:25 +0100 Subject: [PATCH 1/2] ci: stop reaching into the private dcd repo for the mock-api MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- .github/workflows/cli-ci.yml | 51 ++++++-------------- CLAUDE.md | 8 +++- package.json | 1 + scripts/test-runner.mjs | 91 +++++++++++++++++++++--------------- 4 files changed, 75 insertions(+), 76 deletions(-) diff --git a/.github/workflows/cli-ci.yml b/.github/workflows/cli-ci.yml index 0af984a..1012cae 100644 --- a/.github/workflows/cli-ci.yml +++ b/.github/workflows/cli-ci.yml @@ -45,37 +45,23 @@ jobs: lint-and-test: runs-on: ubuntu-latest - # The mock-api lives in the private devicecloud-dev/dcd repo, checked out via an - # SSH deploy key. GitHub does NOT expose secrets to pull_request workflows - # triggered from forks, so that checkout (and the integration tests that need - # it) can only run for same-repo events. Fork PRs still run lint/typecheck/build. + # This repo is PUBLIC and runs no step that reaches into the private + # devicecloud-dev/dcd repo. It used to check out that repo's mock-api over an + # SSH deploy key to run test/integration/*, which meant a private-repo + # credential lived in a public repo's secrets and the API's OpenAPI spec was + # pulled onto the runner on every same-repo PR. dcd#1036 deleted that mock-api; + # rather than re-point at it, the linkage is gone. # - # Dependabot PRs branch from this repo (so the fork check passes) but ALSO run - # without secrets — treat them like forks and skip the private checkout, or - # the mock-api clone fails with an empty DCD_SSH_DEPLOY_KEY. - env: - HAS_PRIVATE_ACCESS: ${{ (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && github.actor != 'dependabot[bot]' }} - + # The consequence is deliberate: test/integration/* does NOT run here, and + # neither does the swagger contract-drift check it provided (spec drift used to + # surface as a Prism 422). Only test/unit/* runs — pure, no backend. To run the + # integration suite locally, point MOCK_API_DIR at a mock; see CLAUDE.md. steps: - name: Checkout CLI uses: actions/checkout@v7 with: path: cli - - name: Checkout dcd (mock-api) - if: env.HAS_PRIVATE_ACCESS == 'true' - uses: actions/checkout@v7 - with: - repository: devicecloud-dev/dcd - path: dcd - ssh-key: ${{ secrets.DCD_SSH_DEPLOY_KEY }} - # api/swagger.json is a file, which cone-mode sparse checkout rejects - # as of git 2.51 ("is not a directory") — use non-cone patterns. - sparse-checkout-cone-mode: false - sparse-checkout: | - /mock-api/ - /api/swagger.json - - name: Setup pnpm uses: pnpm/action-setup@v6.0.10 with: @@ -93,11 +79,6 @@ jobs: working-directory: ./cli run: pnpm install --frozen-lockfile - - name: Install Mock API dependencies - if: env.HAS_PRIVATE_ACCESS == 'true' - working-directory: ./dcd/mock-api - run: pnpm install --frozen-lockfile - - name: Run CLI linter working-directory: ./cli run: pnpm lint @@ -106,16 +87,12 @@ jobs: working-directory: ./cli run: pnpm typecheck - - name: Run CLI tests - if: env.HAS_PRIVATE_ACCESS == 'true' + - name: Run CLI unit tests working-directory: ./cli - env: - MOCK_API_DIR: ${{ github.workspace }}/dcd/mock-api - run: pnpm test + run: pnpm test:unit - - name: Skip integration tests (fork PR — no mock-api access) - if: env.HAS_PRIVATE_ACCESS != 'true' - run: echo "::notice::Integration tests skipped — the mock-api (private devicecloud-dev/dcd) is not accessible from fork PRs. Lint, typecheck, and build still ran." + - name: Note skipped integration tests + run: echo "::notice::Integration tests are not run in CI — they need a mock of the dcd API, and this public repo does not reach into the private one. Lint, typecheck, unit tests, build and audit all ran." - name: Build CLI working-directory: ./cli diff --git a/CLAUDE.md b/CLAUDE.md index 2cd76a9..58428d3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -9,8 +9,12 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co - `pnpm build:binaries` — `scripts/build-binaries.mjs` produces the bun-compiled, self-contained `dcd--` binaries published to GitHub Releases (the install `dcd upgrade` self-updates). The platform/arch keys must stay in sync with `ASSET_BY_PLATFORM` in `src/commands/upgrade.ts`. - `pnpm lint` — ESLint over `src/` and `test/`. - `pnpm typecheck` — `tsc --noEmit -p tsconfig.test.json` over `src/` and `test/` (strict mode; `pnpm build` only compiles `src/`). Requires Node `>=22`. -- `pnpm test` — runs `scripts/test-runner.mjs`: builds the CLI, boots the mock API, then runs all `test/**/*.test.ts` via mocha. TypeScript is loaded by **tsx** (`.mocharc.json`'s `node-option: ["import=tsx"]`), *not* ts-node — Mocha 11 imports specs as ESM, which bypasses the `require: ts-node/register` hook. The mock API lives in the **sibling `dcd/` repo** (`../dcd/mock-api`, started via `npm run start:auth` on port 3001). Override its location with `MOCK_API_DIR=/path/to/mock-api`. The runner isolates `DCD_CONFIG_DIR` to a temp dir so tests never touch your real `dcd login` session. -- Tests split into `test/unit/*` (pure, no backend) and `test/integration/*` (drive the built CLI against the mock API). Run a single test: `pnpm mocha test/integration/cloud.integration.test.ts --timeout 60000` (picks up `.mocharc.json` which wires tsx; integration specs require the mock API already running on port 3001). +- `pnpm test` — runs `scripts/test-runner.mjs`: builds the CLI, boots the mock API if one is available, then runs mocha. TypeScript is loaded by **tsx** (`.mocharc.json`'s `node-option: ["import=tsx"]`), *not* ts-node — Mocha 11 imports specs as ESM, which bypasses the `require: ts-node/register` hook. The runner isolates `DCD_CONFIG_DIR` to a temp dir so tests never touch your real `dcd login` session. +- `pnpm test:unit` — the same runner with `--unit`: unit specs only, no mock API. **This is what CI runs.** +- Tests split into `test/unit/*` (pure, no backend) and `test/integration/*` (drive the built CLI against a Prism mock of the dcd API on port 3001). +- **There is no default mock API any more.** It used to live in the sibling private `dcd/` repo; dcd#1036 deleted it, and this repo — which is public — deliberately no longer reaches into that one (no deploy key, no `swagger.json` pull). So `pnpm test` with no `MOCK_API_DIR` set **silently degrades to the unit suite** and prints a notice. To run `test/integration/*`, stand up a Prism mock over the API's `swagger.json` and point `MOCK_API_DIR=/path/to/mock-api` at it (it needs a `start:auth` npm script serving port 3001). +- Consequence worth knowing: CI no longer catches **CLI↔swagger contract drift**, which used to surface as a Prism 422 from the integration specs. Nothing replaces that check yet. +- Run a single test: `pnpm mocha test/integration/cloud.integration.test.ts --timeout 60000` (picks up `.mocharc.json` which wires tsx; integration specs require the mock API already running on port 3001). ## Entry point diff --git a/package.json b/package.json index d42ca44..25f936a 100644 --- a/package.json +++ b/package.json @@ -66,6 +66,7 @@ "lint": "eslint src test --ext .ts", "prepare": "pnpm build && husky", "test": "node scripts/test-runner.mjs", + "test:unit": "node scripts/test-runner.mjs --unit", "typecheck": "tsc --noEmit -p tsconfig.test.json" }, "version": "5.3.1-beta.2", diff --git a/scripts/test-runner.mjs b/scripts/test-runner.mjs index 33adcba..c6a4441 100755 --- a/scripts/test-runner.mjs +++ b/scripts/test-runner.mjs @@ -8,13 +8,20 @@ import { fileURLToPath } from 'url'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); -// mock-api lives in the sibling dcd/ repo while the oclif→citty migration settles. -// Override with MOCK_API_DIR=/path/to/mock-api if it moves. -const mockApiDir = - process.env.MOCK_API_DIR ?? - path.resolve(__dirname, '../../dcd/mock-api'); +// The integration suite drives the built CLI against a Prism mock of the dcd API. +// That mock used to live in the sibling private dcd/ repo, which no longer ships +// one (dcd#1036), so there is no default location any more: point MOCK_API_DIR at +// a mock to run those specs. Without one — which includes CI, where this repo is +// public and deliberately does not reach into the private repo — the runner falls +// back to the unit suite, which is pure and needs no backend. +const mockApiDir = process.env.MOCK_API_DIR ?? null; const cliDir = path.resolve(__dirname, '..'); +const unitOnly = + process.argv.includes('--unit') || + mockApiDir === null || + !fs.existsSync(mockApiDir); + const MOCK_API_URL = 'http://localhost:3001/'; const READY_DEADLINE_MS = 30_000; const READY_POLL_INTERVAL_MS = 500; @@ -112,52 +119,62 @@ async function runTests() { }); }); - // Start mock API with authentication - console.log('Starting mock API with authentication...'); - mockApiProcess = spawn('npm', ['run', 'start:auth'], { - cwd: mockApiDir, - stdio: ['ignore', 'pipe', 'pipe'], - shell: true, - // Own process group on POSIX so killMockApi() can signal `npm run` - // *and* the server it spawns, not just the wrapper. - detached: process.platform !== 'win32', - }); + if (unitOnly) { + console.log( + 'Running the unit suite only — no mock API available. ' + + 'Set MOCK_API_DIR=/path/to/mock-api to include test/integration/*.' + ); + } else { + // Start mock API with authentication + console.log('Starting mock API with authentication...'); + mockApiProcess = spawn('npm', ['run', 'start:auth'], { + cwd: mockApiDir, + stdio: ['ignore', 'pipe', 'pipe'], + shell: true, + // Own process group on POSIX so killMockApi() can signal `npm run` + // *and* the server it spawns, not just the wrapper. + detached: process.platform !== 'win32', + }); - forwardOutput(mockApiProcess.stdout, (text) => process.stdout.write(text)); - forwardOutput(mockApiProcess.stderr, (text) => process.stderr.write(text)); + forwardOutput(mockApiProcess.stdout, (text) => process.stdout.write(text)); + forwardOutput(mockApiProcess.stderr, (text) => process.stderr.write(text)); - mockApiProcess.on('error', (error) => { - console.error('Mock API failed to start:', error); - if (!testsFinished) { - process.exit(1); - } - }); + mockApiProcess.on('error', (error) => { + console.error('Mock API failed to start:', error); + if (!testsFinished) { + process.exit(1); + } + }); - mockApiProcess.on('exit', (code, signal) => { - mockApiExited = true; - if (!testsFinished) { - console.error( - `Mock API exited before tests finished (code ${code}, signal ${signal})` - ); - process.exit(1); - } - }); + mockApiProcess.on('exit', (code, signal) => { + mockApiExited = true; + if (!testsFinished) { + console.error( + `Mock API exited before tests finished (code ${code}, signal ${signal})` + ); + process.exit(1); + } + }); - console.log('Waiting for mock API to be ready...'); - await waitForMockApi(); - console.log('Mock API is ready.'); + console.log('Waiting for mock API to be ready...'); + await waitForMockApi(); + console.log('Mock API is ready.'); + } // Run tests. Mocha + .mocharc.json handle TypeScript loading via `tsx` // (see `node-option: ["import=tsx"]` there). Mocha 11 imports files as // ESM, so the `require: ts-node/register` hook doesn't get applied; tsx // registers an ESM loader that resolves TS relative imports correctly. console.log('Running tests...'); - const testProcess = spawn('npx', [ + const mochaArgs = [ 'mocha', '--no-warnings', 'test/**/*.test.ts', '--timeout', '60000' - ], { + ]; + // Quoted so the shell hands mocha the literal glob instead of expanding it. + if (unitOnly) mochaArgs.push('--ignore', '"test/integration/**"'); + const testProcess = spawn('npx', mochaArgs, { cwd: cliDir, stdio: 'inherit', shell: true, From 124a3d87ab514697e8499b5d39c133754884d2af Mon Sep 17 00:00:00 2001 From: finalerock44 <77282157+finalerock44@users.noreply.github.com> Date: Tue, 18 Aug 2026 12:25:00 +0100 Subject: [PATCH 2/2] docs: align the contributor docs with the new CI behaviour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CLAUDE.md | 2 +- CONTRIBUTING.md | 28 +++++++++++++++++++--------- README.md | 3 ++- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 58428d3..a9bf70d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -61,7 +61,7 @@ Full guide in `CONTRIBUTING.md`; the operationally important parts (the ones tha - ⚠️ **A `!` (or `BREAKING CHANGE:` footer) bumps the MAJOR — do not use it casually.** The configs set `bump-minor-pre-major: true`, but that only applies **below 1.0.0**; we are on 5.x, so it is inert and a breaking marker means exactly what semver says. A `refactor(cloud)!:` PR title once produced a `6.0.0-beta.1` release PR for what was only a flag rename in an unconsumed beta. Because PRs are squash-merged, **the PR title IS the commit** — the `!` lands even if no branch commit carried it. - **Never hand-edit `package.json` version, `CHANGELOG.md`, or the `.release-please-manifest*.json` files** — release-please owns all of them. `src/types/generated/schema.types.ts` is likewise generated (openapi-typescript). - A first-time contributor must sign the CLA (the CLA Assistant bot comments on the first PR); the CLA check must be green to merge. -- **CI (`.github/workflows/cli-ci.yml`) runs on every PR** including forks: gitleaks secret scan, `pnpm lint`, `pnpm typecheck`, `pnpm build`, `pnpm audit --audit-level moderate`. The **integration tests need the private `devicecloud-dev/dcd` mock-api** (cloned via the `DCD_SSH_DEPLOY_KEY` secret), and GitHub withholds secrets from fork and Dependabot PRs — so `pnpm test` is **skipped there** and a maintainer runs the full suite before merge. gitleaks also runs as a pre-commit hook (allowlist in `.gitleaks.toml`); without the binary installed the hook self-skips and CI is the backstop. +- **CI (`.github/workflows/cli-ci.yml`) runs the same steps on every PR** — fork, Dependabot and same-repo alike, with no privileged path: gitleaks secret scan, `pnpm lint`, `pnpm typecheck`, `pnpm test:unit`, `pnpm build`, `pnpm audit --audit-level moderate`. **`test/integration/*` is not run by CI at all** (see the Commands section: this public repo no longer reaches into the private `devicecloud-dev/dcd` repo for a mock API), so a green PR says nothing about the integration suite — run it locally with `MOCK_API_DIR` set if a change touches the API surface. gitleaks also runs as a pre-commit hook (allowlist in `.gitleaks.toml`); without the binary installed the hook self-skips and CI is the backstop. ## Releases diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1761173..1560eac 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,19 +37,29 @@ Useful scripts: | `pnpm lint` | ESLint over `src/` and `test/` | | `pnpm typecheck` | Strict `tsc --noEmit` over `src/` and `test/` | | `pnpm build` | Compile to `dist/` | -| `pnpm test` | Build + boot the mock API + run integration/unit tests | +| `pnpm test:unit` | Run the unit suite — no backend needed. **This is what CI runs.** | +| `pnpm test` | The same, plus the integration suite if `MOCK_API_DIR` points at a mock API | -**Before pushing, make sure `pnpm lint`, `pnpm typecheck`, and `pnpm build` -pass.** These run for every PR (including from forks) and are required to merge. +**Before pushing, make sure `pnpm lint`, `pnpm typecheck`, `pnpm test:unit`, and +`pnpm build` pass.** These run for every PR and are required to merge. ### About the test suite -`pnpm test` boots a **mock API that lives in a private repository**, so the full -integration suite only runs on branches inside this repo. **On pull requests from -forks the integration tests are automatically skipped** — you'll see a CI notice -saying so. That's expected: lint, typecheck, and build still run and gate your -PR, and a maintainer runs the full suite before merge. You don't need backend -access to contribute. +Tests split in two. `test/unit/*` is pure — no network, no backend — and runs +everywhere, in CI and locally. + +`test/integration/*` drives the built CLI against a Prism mock of the dcd API on +port 3001. **CI does not run it**, on any PR, from a fork or otherwise: this repo +is public and deliberately holds no credentials for, and makes no requests to, +our private infrastructure. There is no default mock API — set +`MOCK_API_DIR=/path/to/mock-api` (a package exposing a `start:auth` script on +port 3001) and `pnpm test` picks the integration suite up. Without it the runner +prints a notice and runs the unit suite alone. + +So every contributor, maintainers included, gets the same CI signal, and you +don't need backend access to contribute. The flip side is worth knowing: a green +PR says nothing about the integration suite, so if your change touches the API +surface, say so in the PR and a maintainer will exercise it before merge. ### Secret scanning diff --git a/README.md b/README.md index f0da192..fdb4804 100644 --- a/README.md +++ b/README.md @@ -78,7 +78,8 @@ $ pnpm install # install deps, build, set up git hooks $ pnpm dcd # run the CLI from source $ pnpm lint # ESLint $ pnpm typecheck # strict tsc, no emit -$ pnpm test # build + boot mock API + integration/unit tests +$ pnpm test:unit # unit tests, no backend needed — what CI runs +$ pnpm test # the above, plus integration tests if MOCK_API_DIR is set ``` ### Secret scanning