feat(v1.9): aqa admin CLI verb — boots SPA + API in-process (sub-task 3)#54
Merged
lopadova merged 5 commits intoMay 21, 2026
Merged
Conversation
…b-task 3)
The kit now ships its own admin: `bunx aqa admin` boots a single
node:http server that serves the bundled admin SPA AND wires
@aqa/server.makeApi() against an in-memory store seeded from
.aqa/runs/<id>/{events,findings}.jsonl. No more "bun --filter @aqa/admin
dev" — that flow only worked inside the monorepo.
CLI surface:
aqa admin [--port <n>] [--host <h>]
Behaviour:
- defaults: 127.0.0.1:5173 (matches the prior Vite dev URL); --port 0
→ OS-assigned (useful for tests).
- / → static index.html from bundled SPA
- /assets/* → static asset files (404 on miss, no SPA fallback)
- /api/healthz → kit-owned `{ ok: true }` (NOT in makeApi — lets tests
smoke the boot without depending on auth/store state)
- /api/* → delegated to makeApi() handler table
- everything else → SPA fallback (index.html) so client-side routing works
- Path traversal: forbidden (refuses to serve files outside SPA dist).
- Seeds the in-memory store from local .aqa/runs/, so the admin shows
real runs out of the box — empty state when no runs exist.
Bundling: new packages/kit/scripts/bundle-admin.mjs copies
packages/admin/dist/ into packages/kit/dist/admin/ during build. Topo
build runs @aqa/admin's vite build BEFORE @aqa/kit's bundle step, so
the source dist is guaranteed present.
Dep cycle: @aqa/server depends on @aqa/kit (for runPackNew). Adding
@aqa/server to kit's deps is a workspace cycle — bun handles it, but
the runtime keeps the cycle shallow via dynamic imports of @aqa/server
+ @aqa/store inside runAdmin(). Type imports stay static (erased at
runtime). @aqa/server's index now also re-exports ApiHandler /
ApiMethod / ApiRequest / ApiResponse, which kit consumes type-only.
Tests: 7 new node:test cases in test/admin-cmd.test.ts boot the server
on port 0, exercise the static + API paths, and verify error / 404 /
traversal handling. Uses a fake admin dist so the test isn't coupled
to whether the real SPA built recently.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a new aqa admin CLI verb to make the Admin SPA reachable from an @aqa/kit install by serving the bundled SPA plus @aqa/server.makeApi() in a single Node process, seeded from local .aqa/runs.
Changes:
- Introduces
runAdmin()(Nodehttpserver) and wires a newaqa adminCLI command with--port/--host. - Bundles the built Admin SPA into the
@aqa/kittarball at build/test time. - Re-exports API surface types from
@aqa/serverneeded by kit’s admin command.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/server/src/index.ts | Re-exports additional API types from ./api.js for downstream consumers. |
| packages/kit/src/commands/admin.ts | Implements runAdmin() HTTP server: static SPA hosting + /api/* delegation + run seeding. |
| packages/kit/src/cli/aqa.ts | Adds admin command routing and signal-based shutdown. |
| packages/kit/scripts/bundle-admin.mjs | Copies packages/admin/dist into packages/kit/dist/admin during build. |
| packages/kit/test/admin-cmd.test.ts | Adds node:test coverage for boot, static serving, API reachability, and traversal/missing-dist cases. |
| packages/kit/package.json | Hooks bundling into build/pretest and adds new workspace dependencies. |
| bun.lock | Updates lockfile for the new kit dependencies. |
Comment on lines
+367
to
+370
| res.setHeader('access-control-allow-origin', '*'); | ||
| res.setHeader('access-control-allow-methods', 'GET,POST,PUT,DELETE,OPTIONS'); | ||
| res.setHeader('access-control-allow-headers', 'content-type,x-aqa-org,x-aqa-project'); | ||
|
|
| const chunks: Buffer[] = []; | ||
| for await (const c of req) chunks.push(c as Buffer); | ||
| const raw = Buffer.concat(chunks).toString('utf8').trim(); | ||
| body = raw ? JSON.parse(raw) : undefined; |
| ...matched.params, | ||
| }; | ||
| const out = await matched.route.handle({ headers, params, body }, hctx.ctx); | ||
| res.statusCode = out.status; |
| const { res, url, hctx } = args; | ||
| // Map `/` → `index.html`; everything else is resolved under adminDistDir. | ||
| // Reject any candidate that resolves outside adminDistDir (path traversal). | ||
| const rel = url.pathname === '/' ? 'index.html' : decodeURIComponent(url.pathname.slice(1)); |
Comment on lines
+472
to
+474
| const rel = url.pathname === '/' ? 'index.html' : decodeURIComponent(url.pathname.slice(1)); | ||
| const candidate = normalize(join(hctx.adminDistDir, rel)); | ||
| if (!candidate.startsWith(hctx.adminDistDir + sep) && candidate !== hctx.adminDistDir) { |
Comment on lines
+179
to
+190
| return { | ||
| ok: true, | ||
| port: resolvedPort, | ||
| host, | ||
| url: `http://${host}:${resolvedPort}`, | ||
| close: () => closeServer(server), | ||
| }; | ||
| } | ||
|
|
||
| function closeServer(server: Server): Promise<void> { | ||
| return new Promise((resolve) => { | ||
| server.close(() => resolve()); |
Comment on lines
+248
to
+255
| const stop = async (): Promise<void> => { | ||
| await result.close(); | ||
| process.exit(0); | ||
| }; | ||
| process.on('SIGINT', () => { | ||
| void stop(); | ||
| }); | ||
| process.on('SIGTERM', () => { |
| "@aqa/pack-loader": "workspace:*", | ||
| "@aqa/runner": "workspace:*", | ||
| "@aqa/schemas": "workspace:*", | ||
| "@aqa/server": "workspace:*", |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0749f490a9
ℹ️ 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".
| "@aqa/pack-loader": "workspace:*", | ||
| "@aqa/runner": "workspace:*", | ||
| "@aqa/schemas": "workspace:*", | ||
| "@aqa/server": "workspace:*", |
There was a problem hiding this comment.
Adding @aqa/server as a direct dependency of @aqa/kit creates a circular workspace graph because @aqa/server already depends on @aqa/kit (packages/server/package.json). Our workspace runner explicitly detects this and warns that downstream packages may run before @aqa/kit, which can break clean build/typecheck/test runs when @aqa/server resolves @aqa/kit artifacts before they exist; this is a regression in build reliability introduced by this dependency edge.
Useful? React with 👍 / 👎.
| ok: true, | ||
| port: resolvedPort, | ||
| host, | ||
| url: `http://${host}:${resolvedPort}`, |
There was a problem hiding this comment.
The URL is always built as http://${host}:${port}, which is invalid when --host is IPv6 (for example ::1 must be bracketed as [::1]). In that case runAdmin().url and the CLI health link become malformed, so users launching with IPv6 bind addresses get unusable output even though the server may be listening correctly.
Useful? React with 👍 / 👎.
Sub-task 3's CI exposed the underlying dep cycle: @aqa/server depends on @aqa/kit for runPackNew (POST /api/packs/scaffold), and @aqa/kit now needs @aqa/server (admin command). The topo-sort build order can't resolve a cycle — server tries to build before kit's dist exists and errors with TS2307 cannot find module @aqa/kit. Fix: extract runPackNew + PackNewErrorCode/PackNewOptions/PackNewResult into a brand-new workspace package @aqa/pack-author. Both kit and server depend on it independently; no cycle remains. Mechanics: - New packages/pack-author with package.json + tsconfig + src + test - git-moved packages/kit/src/commands/pack-new.ts → packages/pack-author/src/index.ts - New packages/kit/src/commands/pack-new.ts is a 5-line re-export so the in-kit CLI / tests import paths keep working unchanged - packages/server/src/api.ts imports runPackNew from @aqa/pack-author - packages/server/package.json: replaced @aqa/kit dep with @aqa/pack-author - packages/kit/package.json: added @aqa/pack-author alongside @aqa/server Tests: 2 new node:test cases in pack-author/test confirm the package boundary (callable + structured error). All heavy behavior coverage stays in packages/kit/test/pack-new.test.ts via the re-export shim. Build now passes in topo order: pack-author → server → kit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment on lines
+362
to
+373
| async function handleRequest( | ||
| req: IncomingMessage, | ||
| res: ServerResponse, | ||
| hctx: HandleCtx, | ||
| ): Promise<void> { | ||
| res.setHeader('access-control-allow-origin', '*'); | ||
| res.setHeader('access-control-allow-methods', 'GET,POST,PUT,DELETE,OPTIONS'); | ||
| res.setHeader('access-control-allow-headers', 'content-type,x-aqa-org,x-aqa-project'); | ||
|
|
||
| const method = (req.method ?? 'GET').toUpperCase(); | ||
| if (method === 'OPTIONS') { | ||
| res.statusCode = 204; |
Comment on lines
+149
to
+163
| await new Promise<void>((resolve, reject) => { | ||
| const onError = (e: Error): void => { | ||
| server.off('listening', onListen); | ||
| reject(e); | ||
| }; | ||
| const onListen = (): void => { | ||
| server.off('error', onError); | ||
| resolve(); | ||
| }; | ||
| server.once('error', onError); | ||
| server.once('listening', onListen); | ||
| server.listen(port, host); | ||
| }).catch((e: Error) => { | ||
| throw e; | ||
| }); |
Comment on lines
+472
to
+476
| const rel = url.pathname === '/' ? 'index.html' : decodeURIComponent(url.pathname.slice(1)); | ||
| const candidate = normalize(join(hctx.adminDistDir, rel)); | ||
| if (!candidate.startsWith(hctx.adminDistDir + sep) && candidate !== hctx.adminDistDir) { | ||
| res.statusCode = 403; | ||
| res.end('forbidden'); |
| const chunks: Buffer[] = []; | ||
| for await (const c of req) chunks.push(c as Buffer); | ||
| const raw = Buffer.concat(chunks).toString('utf8').trim(); | ||
| body = raw ? JSON.parse(raw) : undefined; |
Comment on lines
+244
to
+257
| const runStarted = events.find((e) => e.kind === 'run_started'); | ||
| const runFinished = events.find((e) => e.kind === 'run_finished'); | ||
| const profile = readPayloadString(runStarted, 'profile') ?? 'unknown'; | ||
| const project = readPayloadString(runStarted, 'project') ?? 'unknown'; | ||
| const startedAt = | ||
| (typeof runStarted?.ts === 'string' && runStarted.ts) || new Date(0).toISOString(); | ||
| const finishedAt = typeof runFinished?.ts === 'string' ? runFinished.ts : undefined; | ||
| const runDraft = { | ||
| schema_version: '1' as const, | ||
| id: name, | ||
| started_at: startedAt, | ||
| ...(finishedAt ? { finished_at: finishedAt } : {}), | ||
| state: (runFinished ? 'succeeded' : 'running') as Run.RunState, | ||
| project, |
| @@ -1,7 +1,7 @@ | |||
| import { Permission, rolePermissions } from '@aqa/auth'; | |||
| import type { Permission as PermissionType, Role, User, allows } from '@aqa/auth'; | |||
| "exports": { | ||
| ".": { "types": "./dist/index.d.ts", "default": "./dist/index.js" } | ||
| }, | ||
| "files": ["dist", "README.md"], |
| * @aqa/pack-author smoke — the heavy behaviour coverage stays in | ||
| * `packages/kit/test/pack-new.test.ts`, which exercises `runPackNew` | ||
| * end-to-end through the CLI re-export. This file just guards the | ||
| * package contract: `runPackNew` is the default-export shape and |
6 tasks
lopadova
added a commit
that referenced
this pull request
May 20, 2026
6 actionable comments, all addressed: 1. CRITICAL — publish-prep now STRIPS @aqa/* deps entirely (was only pinning workspace:* → version). The @aqa/* packages are inlined into dist/cli.cjs by esbuild and are NOT separately published, so leaving them in `dependencies` makes `bun add @padosoft/agentic-qa-kit` fail with "404 — @aqa/runner not found on registry". Test fixture updated to include both @aqa/* (must be stripped) and non-@aqa workspace:* (must be pinned) so the new behaviour is asserted. 2. build-bundle.mjs header comment said "ESM file at dist/cli.js"; the implementation emits CJS to dist/cli.cjs. Rewrote the docstring to match — plus added the "why CJS-in-.cjs" explanation (bundled deps call `require('process')`; the .cjs extension overrides "type": "module"). 3. test/build-bundle.test.ts header similarly out of date (`dist/cli.js`). Updated. 4. packages/pack-author/test header claimed runPackNew was "default-export shape", but it's a named export. Reworded to "NAMED export". 5. packages/pack-author missing README.md (declared in `files`). Wrote a real README explaining why the package exists (kit↔server cycle) and the API surface. 6. server/api.ts comment still said "Delegates to runPackNew from @aqa/kit" — implementation now imports from @aqa/pack-author. Updated. The `dist/admin` not-in-files note from review is intentional on this sub-task branch — the bundle-admin script is added by sub-task 3 (PR #54). Macro merge will combine both: sub-task 3 adds dist/admin to files, sub-task 4 adds dist/cli.cjs + publish prep. All gates green: 220 tests, lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 actionable comments, all addressed: 1. Stale comment in packages/server/src/api.ts said "Delegates to runPackNew from @aqa/kit" but the import was updated to @aqa/pack-author. Aligned the comment with the implementation. 2. packages/pack-author/package.json declared files: ["dist", "README.md"] but README.md was missing — publish would ship an empty/missing README and violate the repo guideline (every package README required). Wrote a real README explaining the cycle-break rationale and API surface. 3. packages/pack-author/test header claimed runPackNew was the "default-export shape"; it's actually a named export. Reworded. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment on lines
+367
to
+370
| res.setHeader('access-control-allow-origin', '*'); | ||
| res.setHeader('access-control-allow-methods', 'GET,POST,PUT,DELETE,OPTIONS'); | ||
| res.setHeader('access-control-allow-headers', 'content-type,x-aqa-org,x-aqa-project'); | ||
|
|
Comment on lines
+410
to
+444
| for (const r of hctx.api) { | ||
| if (r.method !== method) continue; | ||
| const params = routeMatch(r.path, url.pathname); | ||
| if (params) { | ||
| matched = { route: r, params }; | ||
| break; | ||
| } | ||
| } | ||
| if (!matched) { | ||
| res.statusCode = 404; | ||
| res.setHeader('content-type', 'application/json'); | ||
| res.end(JSON.stringify({ error: 'route not found' })); | ||
| return; | ||
| } | ||
|
|
||
| let body: unknown; | ||
| if (method === 'POST' || method === 'PUT' || method === 'DELETE') { | ||
| const chunks: Buffer[] = []; | ||
| for await (const c of req) chunks.push(c as Buffer); | ||
| const raw = Buffer.concat(chunks).toString('utf8').trim(); | ||
| body = raw ? JSON.parse(raw) : undefined; | ||
| } | ||
| const headers: Record<string, string> = {}; | ||
| for (const [k, v] of Object.entries(req.headers)) { | ||
| headers[k] = Array.isArray(v) ? v.join(',') : String(v ?? ''); | ||
| } | ||
| const params: Record<string, string> = { | ||
| ...Object.fromEntries(url.searchParams.entries()), | ||
| ...matched.params, | ||
| }; | ||
| const out = await matched.route.handle({ headers, params, body }, hctx.ctx); | ||
| res.statusCode = out.status; | ||
| res.setHeader('content-type', 'application/json'); | ||
| res.end(JSON.stringify(out.body)); | ||
| } |
| const chunks: Buffer[] = []; | ||
| for await (const c of req) chunks.push(c as Buffer); | ||
| const raw = Buffer.concat(chunks).toString('utf8').trim(); | ||
| body = raw ? JSON.parse(raw) : undefined; |
Comment on lines
+470
to
+476
| // Map `/` → `index.html`; everything else is resolved under adminDistDir. | ||
| // Reject any candidate that resolves outside adminDistDir (path traversal). | ||
| const rel = url.pathname === '/' ? 'index.html' : decodeURIComponent(url.pathname.slice(1)); | ||
| const candidate = normalize(join(hctx.adminDistDir, rel)); | ||
| if (!candidate.startsWith(hctx.adminDistDir + sep) && candidate !== hctx.adminDistDir) { | ||
| res.statusCode = 403; | ||
| res.end('forbidden'); |
Comment on lines
+106
to
+109
| // Lazy dynamic imports break what would otherwise be a static dependency | ||
| // cycle: @aqa/server depends on @aqa/kit (for runPackNew). Dynamic import | ||
| // keeps the cycle out of the module-load graph and out of any future | ||
| // esbuild bundle that doesn't statically follow these specifiers. |
Comment on lines
+104
to
+105
| --port <n> (admin) HTTP port to listen on (default 5173; 0 = OS-assigned) | ||
| --host <h> (admin) bind host (default 127.0.0.1; use 0.0.0.0 to expose on LAN) |
2 actionable comments, both addressed: 1. Stale comment in admin.ts said "Lazy dynamic imports break what would otherwise be a static dependency cycle: @aqa/server depends on @aqa/kit (for runPackNew)". This was true in an earlier iteration of PR #54, but the cycle is now properly broken by extracting `runPackNew` into @aqa/pack-author. Reworded: the dynamic import is now an optimisation (defer makeApi/RunnerQueue loading to the actual `aqa admin` invocation), not a cycle workaround. 2. `--host 0.0.0.0` in help text had no security warning. `aqa admin` runs without real authentication — binding to LAN exposes the in-memory store + makeApi() to any peer on the network. Added an explicit WARNING block to the help text spelling out the risk and the safe use cases (dev VM / isolated CI runner). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment on lines
+369
to
+379
| res.setHeader('access-control-allow-origin', '*'); | ||
| res.setHeader('access-control-allow-methods', 'GET,POST,PUT,DELETE,OPTIONS'); | ||
| res.setHeader('access-control-allow-headers', 'content-type,x-aqa-org,x-aqa-project'); | ||
|
|
||
| const method = (req.method ?? 'GET').toUpperCase(); | ||
| if (method === 'OPTIONS') { | ||
| res.statusCode = 204; | ||
| res.end(); | ||
| return; | ||
| } | ||
|
|
| const chunks: Buffer[] = []; | ||
| for await (const c of req) chunks.push(c as Buffer); | ||
| const raw = Buffer.concat(chunks).toString('utf8').trim(); | ||
| body = raw ? JSON.parse(raw) : undefined; |
| const ti = t[i] as string; | ||
| const pi = p[i] as string; | ||
| if (ti.startsWith(':')) { | ||
| params[ti.slice(1)] = decodeURIComponent(pi); |
Comment on lines
+474
to
+477
| const rel = url.pathname === '/' ? 'index.html' : decodeURIComponent(url.pathname.slice(1)); | ||
| const candidate = normalize(join(hctx.adminDistDir, rel)); | ||
| if (!candidate.startsWith(hctx.adminDistDir + sep) && candidate !== hctx.adminDistDir) { | ||
| res.statusCode = 403; |
Comment on lines
+1
to
+10
| # `@aqa/pack-author` | ||
|
|
||
| Pack scaffolding primitives — shared between `@aqa/kit` (CLI) and `@aqa/server` (API). | ||
|
|
||
| ## Why this package exists | ||
|
|
||
| Both the CLI (`aqa pack new <slug>`) and the server (`POST /api/packs/scaffold`) need to scaffold runnable AQA packs on disk. Originally the logic lived inside `@aqa/kit` and `@aqa/server` imported it from there. When the v1.9 `aqa admin` command needed `@aqa/kit` to depend on `@aqa/server` (for `makeApi()`), the cycle made the topological build non-deterministic and the kit's TypeScript compile started failing in CI. | ||
|
|
||
| This package breaks the cycle by owning the scaffolding logic. Both kit and server depend on `@aqa/pack-author`; neither depends on the other. | ||
|
|
lopadova
added a commit
that referenced
this pull request
May 20, 2026
5 more actionable comments, all addressed: 1. CRITICAL — packages/kit/LICENSE was declared in `files` but didn't exist (only the repo root has LICENSE). Publishing from packages/kit would omit the license. Copied the root Apache-2.0 LICENSE into packages/kit/LICENSE so the published tarball carries it. 2. build-bundle.mjs header still referenced `dist/admin/` and bundle-admin.mjs — both belong to sub-task 3 (PR #54). Clarified that this sub-task only ships the CLI bundler; the admin static directory + its bundler land in the macro merge alongside PR #54. 3. Stale "tarball's `bin: dist/cli.js`" in build-bundle.mjs inner comment — fixed to cli.cjs. 4. Stale shebang-dedup comment claimed esbuild's banner adds the duplicate shebang; the banner option is no longer used today. Reworded as a defensive idempotent post-process so future re-introduction of a banner doesn't silently break the bundle. 5. Test now asserts the bundle's executable bit (0o111 mask) on POSIX. Skipped on Windows where mode bits don't carry POSIX semantics. The "drop types export" concern is intentional: @aqa/kit was only consumed programmatically by @aqa/server (for runPackNew); that's now via @aqa/pack-author. No remaining consumer imports anything from @aqa/kit's API surface, so the published package can be CLI- only without breaking anyone. The PR 56 docs-claim-don't-exist comments are correct in isolation but will be resolved at macro merge time (sub-task 5 PR's docs match the post-merge macro state, not its own branch state). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lopadova
merged commit 0497da3
into
task/v1.9-junior-quickstart-truthing
8 checks passed
lopadova
added a commit
that referenced
this pull request
May 21, 2026
…sub-task 5) (#56) The v1.8 README claimed `bun add -d agentic-qa-kit` worked (npm 404), `aqa install-agent-files` worked (CLI verb didn't exist), `aqa report` worked (CLI verb didn't exist), and `bun --filter @aqa/admin dev` was how juniors boot the admin (only works inside the monorepo). v1.9 closes those gaps in code (PRs #52/#53/#54/#55); this PR brings the docs back in sync. ## README - New step 2: `.npmrc` snippet for GitHub Packages auth + `GITHUB_TOKEN` export. Public packages on GH Packages still require auth — this is the single biggest junior trap and now has its own step. - Step 3: `bun add -d @padosoft/agentic-qa-kit` (replaces the npm-404 `agentic-qa-kit`). - Step 4 bundles `init` + `doctor` + `validate` so juniors verify the install before moving on. - Step 5: `install-agent-files --targets` — now matches a real verb. - Step 6: explicit `risk-map.yaml` edit with a worked example invariant. - Step 7: `aqa run --profile smoke` with explicit pointer to the per-run artifacts under `.aqa/runs/<id>/`. - Step 8: `aqa report` with format flag matrix. - Step 9: `aqa admin` — single command boots SPA + API (replaces `bun --filter @aqa/admin dev`). - Step 10: reproduce from artifacts. - Footer: `bun run e2e:ecosystem` pointer for monorepo contributors. - "How you use it" 8-step model reflects the new ordering. - Roadmap rows added for v1.8 and v1.9. - Status line bumped: `v1.7 current` → `v1.9 current`. ## docs/getting-started.md Section 1 new: GitHub Packages auth setup with PAT scope + .npmrc + env export. Section 2: bun add @padosoft/agentic-qa-kit. Sections 3–9: init/doctor/validate/install-agent-files/risk/run/report/admin in order, each with the real flag matrix. Closing "Where to go next" links the new PACK-AUTHORING doc. ## CHANGELOG Added [1.9.0] entry covering the 5 PRs + the kit↔server cycle fix + the bundle/publish-prep mechanics. Backfilled [1.8.1] / [1.8.2] / [1.8.3] which had only been recorded in PROGRESS.md. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lopadova
added a commit
that referenced
this pull request
May 21, 2026
* feat(v1.9): GitHub Packages publish pipeline (sub-task 4)
Adds the infrastructure to publish @padosoft/agentic-qa-kit as a single
bundled tarball to GitHub Packages on every v* tag.
## What lands
- esbuild bundler (packages/kit/scripts/build-bundle.mjs):
- Walks the kit's import graph and inlines every workspace dep
(@aqa/*) plus npm deps (yaml, kleur, zod, …) into one
dist/cli.cjs file.
- Externalises only Node built-ins (assert / fs / http / process /
crypto / …) in BOTH prefix forms (`fs` and `node:fs`) — some
bundled CJS deps (yaml, ajv) call `require('process')` without
the prefix and would otherwise hit esbuild's "Dynamic require"
stub.
- Emits CJS-in-.cjs because the kit's package.json sets
`"type": "module"`. Plain `.js` would be parsed as ESM and the
bundled CJS wrappers would throw on `require('process')`.
- Dedupes shebangs (tsc preserves the source `#!/usr/bin/env node`
and esbuild keeps it; a double shebang is a SyntaxError).
- Sourcemap is emitted so crash stacks point at the original
TypeScript sources.
- chmod 0o755 on POSIX so the bin script is executable.
- Writes dist/cli.bundle.meta.json sentinel for CI to verify the
artifact actually got produced.
- Verified locally: bundle is ~460 KB; `node cli.cjs --version`
and `--help` work.
- Publish-time package.json rewriter (packages/kit/scripts/publish-prep.mjs):
- Substitutes `name` from `aqa.publishName` (= @padosoft/agentic-qa-kit).
GitHub Packages requires `<scope> === <owner>`, but renaming the
workspace permanently would break every internal import path.
- Pins every `workspace:*` dep to the kit's current version.
- Idempotent; rewrite lives only in the CI checkout, never committed.
- GitHub Actions workflow (.github/workflows/publish.yml):
- Tag-triggered on `v*`, permissions: contents:read + packages:write +
id-token:write.
- Verifies tag matches packages/kit/package.json version.
- bun install → bun run build → verify bundle exists → publish-prep →
npm publish --provenance --access public to npm.pkg.github.com.
- Pack-author extraction (packages/pack-author/):
- Same cycle fix as sub-task 3 (kit ↔ server cycle on runPackNew).
Required here too so sub-task 4 builds standalone in CI; the
duplicate-add merges cleanly with sub-task 3's identical files.
- Moved packages/kit/src/commands/pack-new.ts → pack-author/src/index.ts
(git-move preserves history). Kit keeps a 5-line re-export shim
so existing CLI imports work unchanged.
- @aqa/server now imports runPackNew from @aqa/pack-author (was @aqa/kit).
- Added 2 boundary tests on pack-author.
- Kit package.json:
- version 0.0.1 → 1.9.0 (target tag).
- publishConfig.registry → https://npm.pkg.github.com.
- aqa.publishName declares the @padosoft scope override.
- main/exports/bin/files point at the new bundled dist/cli.cjs.
- Build-bundle smoke test (packages/kit/test/build-bundle.test.ts):
- 4 tests: bundle present + executable + shebang + size bounds (skipped
if not built); publish-prep rewrites + errors on missing publishName.
- Uses a hermetic temp dir for the publish-prep tests so the real
packages/kit/package.json is never touched even on assertion failure.
All gates green locally: 9 packages, 219 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(v1.9): address Copilot iter 1 on publish-pipeline PR #55
6 actionable comments, all addressed:
1. CRITICAL — publish-prep now STRIPS @aqa/* deps entirely (was only
pinning workspace:* → version). The @aqa/* packages are inlined
into dist/cli.cjs by esbuild and are NOT separately published, so
leaving them in `dependencies` makes `bun add @padosoft/agentic-qa-kit`
fail with "404 — @aqa/runner not found on registry". Test fixture
updated to include both @aqa/* (must be stripped) and non-@aqa
workspace:* (must be pinned) so the new behaviour is asserted.
2. build-bundle.mjs header comment said "ESM file at dist/cli.js"; the
implementation emits CJS to dist/cli.cjs. Rewrote the docstring to
match — plus added the "why CJS-in-.cjs" explanation (bundled deps
call `require('process')`; the .cjs extension overrides "type":
"module").
3. test/build-bundle.test.ts header similarly out of date (`dist/cli.js`).
Updated.
4. packages/pack-author/test header claimed runPackNew was "default-export
shape", but it's a named export. Reworded to "NAMED export".
5. packages/pack-author missing README.md (declared in `files`). Wrote
a real README explaining why the package exists (kit↔server cycle)
and the API surface.
6. server/api.ts comment still said "Delegates to runPackNew from
@aqa/kit" — implementation now imports from @aqa/pack-author.
Updated.
The `dist/admin` not-in-files note from review is intentional on this
sub-task branch — the bundle-admin script is added by sub-task 3 (PR
#54). Macro merge will combine both: sub-task 3 adds dist/admin to
files, sub-task 4 adds dist/cli.cjs + publish prep.
All gates green: 220 tests, lint clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(v1.9): address Copilot iter 2 on publish-pipeline PR #55
5 more actionable comments, all addressed:
1. CRITICAL — packages/kit/LICENSE was declared in `files` but didn't
exist (only the repo root has LICENSE). Publishing from packages/kit
would omit the license. Copied the root Apache-2.0 LICENSE into
packages/kit/LICENSE so the published tarball carries it.
2. build-bundle.mjs header still referenced `dist/admin/` and
bundle-admin.mjs — both belong to sub-task 3 (PR #54). Clarified
that this sub-task only ships the CLI bundler; the admin static
directory + its bundler land in the macro merge alongside PR #54.
3. Stale "tarball's `bin: dist/cli.js`" in build-bundle.mjs inner
comment — fixed to cli.cjs.
4. Stale shebang-dedup comment claimed esbuild's banner adds the
duplicate shebang; the banner option is no longer used today.
Reworded as a defensive idempotent post-process so future
re-introduction of a banner doesn't silently break the bundle.
5. Test now asserts the bundle's executable bit (0o111 mask) on
POSIX. Skipped on Windows where mode bits don't carry POSIX
semantics.
The "drop types export" concern is intentional: @aqa/kit was only
consumed programmatically by @aqa/server (for runPackNew); that's
now via @aqa/pack-author. No remaining consumer imports anything
from @aqa/kit's API surface, so the published package can be CLI-
only without breaking anyone.
The PR 56 docs-claim-don't-exist comments are correct in isolation
but will be resolved at macro merge time (sub-task 5 PR's docs match
the post-merge macro state, not its own branch state).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
8 tasks
lopadova
added a commit
that referenced
this pull request
May 21, 2026
…t + admin verbs + GitHub Packages publish (macro) (#57) * feat(v1.9): aqa install-agent-files CLI verb (sub-task 1) (#52) * feat(v1.9): add `aqa install-agent-files` CLI verb (sub-task 1) Cables the existing `renderForTargets()` from @aqa/adapters into a real CLI verb. The README/getting-started have referenced this command since v0.0.1 but the verb itself was never wired — `bunx aqa install-agent-files --targets ...` errored out at parse time. CLI surface: aqa install-agent-files --targets claude,codex,gemini,copilot [--project-name <slug>] [--force] [--dry-run] Behaviour: - --targets: csv or repeated `--targets <single>`; case-insensitive; deduped - unknown target → fail-fast, no files written (rejects "claude,mistral" before touching disk) - empty/whitespace-only targets list → fail-fast with usage message - project name defaults to slugified basename of cwd; --project-name overrides (also slugified to satisfy @aqa/schemas Project.name = Slug) - existing files preserved unless --force (writeFileSafe contract) - --dry-run reports planned writes without touching disk Tests: 13 new node:test cases in packages/kit/test/install-agent-files-cmd.test.ts covering happy path (4 targets), targets validation (5 cases including dedup + casing), and overwrite semantics (skip / force / dry-run). Drive-by: doctor.ts hint no longer says "(Task 4)" — the verb exists now, the suggestion is the full command a junior can paste. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(v1.9): address Copilot iter 1 on install-agent-files PR #52 4 actionable comments, all addressed: 1. parseTargets array form skipped trim+empty-filter. Inputs like `['claude ', '', 'codex']` would surface a confusing `unknown target ' '` error. Unified normalization across both CSV and array forms; added a regression test. 2. Help text claimed `copilot-instructions.md` but the copilot adapter writes to `.github/copilot-instructions.md`. Fixed the path in HELP so juniors know where to look. 3. Test fixture used `mkdtempSync(join(tmpdir(), 'My Junior Project '))` — Windows rejects path components ending in a space, which would crash the test before any assertion ran. Changed the trailing space to a hyphen so the prefix remains slug-meaningful but is portable. 4. `lastPathSegment` + `slugify` were duplicated between init.ts and install-agent-files.ts. Extracted to `cli-utils.ts` and imported from both — single source of truth for project-name slugging. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(v1.9): address Copilot iter 2 on install-agent-files PR #52 2 actionable comments, both addressed: 1. slugify() could return strings longer than @aqa/schemas Slug.max(64). `runInit` and `runInstallAgentFiles` both feed this value into `.aqa/project.yaml` (and `risk-map.yaml`), which the Slug schema rejects at `aqa validate` time. Cap to 64 chars after normalization and re-trim trailing dashes so a cap that lands inside a `-` run doesn't produce an illegal `-`-terminated slug. Added a regression test that feeds 70 'a's and confirms the embedded slug is ≤64. 2. KNOWN_TARGETS hardcoded `['claude','codex','gemini','copilot']` which duplicates the canonical list in @aqa/adapters.adapters. Adding a new adapter (e.g. opencode) would render but be rejected at parseTargets time. Now derived from `adapters.map(a => a.target).sort()` — single source of truth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(v1.9): aqa report CLI verb (sub-task 2) (#53) * feat(v1.9): add `aqa report` CLI verb (sub-task 2) Cables `@aqa/reporter` into a real CLI verb. README quick-start has referenced `aqa report` since v0.0.1 but the verb itself was never wired — `bunx aqa report` errored at parse time. CLI surface: aqa report [--run-id <id>] [--format md|json|both] Behaviour: - `--run-id` omitted → defaults to the latest run directory (lexical sort on run-id, which has an ISO-like prefix so newest wins) - reads `.aqa/runs/<id>/events.jsonl` + `findings.jsonl` - reconstructs a schema-conformant Run object from the first `run_started` and last `run_finished` events (no new sidecar files — reports stay replayable from the audit trail alone) - writes `report.md` (renderMarkdown) + `report.json` (renderJson) into the same run directory; the JSON shape is the stable one the admin UI already consumes - structured errors for: missing runs dir, missing run-id, malformed JSONL, schema-invalid findings - `--format md|json` opt-out for each artifact Tests: 10 new node:test cases in packages/kit/test/report-cmd.test.ts cover happy path (5: full render, latest-default, md-only, json-only, zero-findings) and error cases (5: missing dir, missing run-id, empty dir, malformed JSONL, schema-invalid finding). Synthetic run dirs match @aqa/schemas Run/Finding exactly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(v1.9): address Copilot iter 1 on report PR #53 5 actionable comments, all addressed: 1. P1 — state was unconditionally 'succeeded' when run_finished was present. But runRun writes run_finished even on most failure paths (pack_errors / scenario_errors / missing_scenarios / unsafe_paths / runtime_errors / zero scenarios). Now derives state by inspecting the payload counters: any non-zero error counter OR zero scenarios → 'failed'; no run_finished → 'running'; else 'succeeded'. Added 3 regression tests. 2. P2 — latest-run selection used lexical sort on directory name, which silently picks the wrong run when `aqa run --seed` produces hash-based IDs (run-<sha>). Now uses statSync.mtimeMs (most recent wins) with the directory name as a deterministic tie-breaker. Added a regression test that intentionally writes the lexically-earlier name LAST. 3. P1 — readJsonl returned [] for missing files. That made `aqa report` succeed on a corrupted/incomplete run dir and emit a synthetic empty report. Now both events.jsonl and findings.jsonl are required up-front with explicit existence checks; readJsonl assumes existence. Added 2 regression tests. 4. Path-traversal — --run-id was forwarded to join() without validation, so a value like '../..' could read/write files outside .aqa/runs. Added a LongSlug-shaped regex (/^[a-z0-9-]{1,80}$/) at the CLI boundary that rejects traversal and any non-slug input. Added 2 regression tests. 5. writeFileSync wasn't wrapped — a read-only FS or disk-full would bubble through the CLI's top-level unhandled-error path instead of returning a structured ReportErr. Wrapped both writes in a single try/catch. Test fixture bug also fixed: writeFindings now accepts an optional runId parameter (default RUN_ID) so findings written for ALT_RUN_ID-style directories carry the matching run_id, not a stale constant. 17 tests now pass (10 original + 7 new regression tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(v1.9): address Copilot iter 2 on report PR #53 4 actionable comments, all addressed: 1. RUN_ID_RE diverged from real LongSlug. Previous /^[a-z0-9-]{1,80}$/ capped at 80 (vs schema's 256) and allowed leading/trailing dashes plus `--` runs that real LongSlug rejects — so a malformed --run-id could slip past this guard and fail later at Finding.parse(). Now uses the canonical SlugPattern /^[a-z0-9](?:-?[a-z0-9])*$/ + a separate 256-char length check so error messages distinguish "bad characters" from "too long". Added regression tests for both. 2. safeIsDir() followed symlinks via statSync. An attacker (or a prior run) leaving a symlink under .aqa/runs/<runId> pointing outside the project would let report.md/report.json land anywhere the link pointed. Now explicit lstatSync().isSymbolicLink() check refuses symlinked run dirs. Added a regression test (skipped on Windows when symlink perms aren't available). 3. readJsonl() silently dropped any non-empty line that parsed as valid JSON but wasn't a plain object (null, [], "x", 42). For events.jsonl that turned corruption into a seemingly-successful report. Now strict: throws "expected a JSON object, got <type>" with explicit null branch (typeof null === 'object' in JS so the naive type check needed a guard). Added 2 regression tests (null + array). 4. Misleading comment claimed admin UI keys on run.id and not config_hash. Admin actually renders config_hash in replay copy, so the all-zeros placeholder will be visible. Replaced with an honest "treat as not-computed sentinel" note so future maintainers don't try to fix a bug that isn't there. 22 tests now pass (was 17 → +5 new regression tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(v1.9): address Copilot iter 3 on report PR #53 3 actionable comments, all addressed: 1. latestRunId considered any directory entry, including symlinks and non-run subdirs. A `.aqa/runs/readme-stash/` with the newest mtime would shadow real runs. Now filters: name matches RUN_ID_RE + not-a-symlink (lstat) + contains events.jsonl. Added a regression test that pollutes `.aqa/runs/` with a non-run subdir and asserts the latest REAL run wins. 2. Per-file symlink protection. The earlier symlink guard only checked the run directory itself. A pre-existing `report.md` or `report.json` symlink would let writeFileSync follow the link and redirect the write outside the project. Now lstat-checks each target file before writing; refuses if it's a symlink. Added a regression test that creates a symlink to an outside file, attempts the report, and asserts both (a) refusal and (b) attacker-controlled file remains byte-identical. 3. reconstructRun could in principle produce Run-schema-incompatible output if the audit chain itself is malformed. Now validates the reconstructed object via Run.Run.safeParse before handing it to the renderers — surfaces a structured error instead of writing a report.json that the admin UI would silently reject. Also flipped the import of Run from type-only to value (needed for safeParse). 24 tests now pass (was 22 → +2 regression tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(v1.9): aqa admin CLI verb — boots SPA + API in-process (sub-task 3) (#54) * feat(v1.9): add `aqa admin` CLI verb — boots SPA + API in-process (sub-task 3) The kit now ships its own admin: `bunx aqa admin` boots a single node:http server that serves the bundled admin SPA AND wires @aqa/server.makeApi() against an in-memory store seeded from .aqa/runs/<id>/{events,findings}.jsonl. No more "bun --filter @aqa/admin dev" — that flow only worked inside the monorepo. CLI surface: aqa admin [--port <n>] [--host <h>] Behaviour: - defaults: 127.0.0.1:5173 (matches the prior Vite dev URL); --port 0 → OS-assigned (useful for tests). - / → static index.html from bundled SPA - /assets/* → static asset files (404 on miss, no SPA fallback) - /api/healthz → kit-owned `{ ok: true }` (NOT in makeApi — lets tests smoke the boot without depending on auth/store state) - /api/* → delegated to makeApi() handler table - everything else → SPA fallback (index.html) so client-side routing works - Path traversal: forbidden (refuses to serve files outside SPA dist). - Seeds the in-memory store from local .aqa/runs/, so the admin shows real runs out of the box — empty state when no runs exist. Bundling: new packages/kit/scripts/bundle-admin.mjs copies packages/admin/dist/ into packages/kit/dist/admin/ during build. Topo build runs @aqa/admin's vite build BEFORE @aqa/kit's bundle step, so the source dist is guaranteed present. Dep cycle: @aqa/server depends on @aqa/kit (for runPackNew). Adding @aqa/server to kit's deps is a workspace cycle — bun handles it, but the runtime keeps the cycle shallow via dynamic imports of @aqa/server + @aqa/store inside runAdmin(). Type imports stay static (erased at runtime). @aqa/server's index now also re-exports ApiHandler / ApiMethod / ApiRequest / ApiResponse, which kit consumes type-only. Tests: 7 new node:test cases in test/admin-cmd.test.ts boot the server on port 0, exercise the static + API paths, and verify error / 404 / traversal handling. Uses a fake admin dist so the test isn't coupled to whether the real SPA built recently. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(v1.9): break kit↔server build cycle by extracting @aqa/pack-author Sub-task 3's CI exposed the underlying dep cycle: @aqa/server depends on @aqa/kit for runPackNew (POST /api/packs/scaffold), and @aqa/kit now needs @aqa/server (admin command). The topo-sort build order can't resolve a cycle — server tries to build before kit's dist exists and errors with TS2307 cannot find module @aqa/kit. Fix: extract runPackNew + PackNewErrorCode/PackNewOptions/PackNewResult into a brand-new workspace package @aqa/pack-author. Both kit and server depend on it independently; no cycle remains. Mechanics: - New packages/pack-author with package.json + tsconfig + src + test - git-moved packages/kit/src/commands/pack-new.ts → packages/pack-author/src/index.ts - New packages/kit/src/commands/pack-new.ts is a 5-line re-export so the in-kit CLI / tests import paths keep working unchanged - packages/server/src/api.ts imports runPackNew from @aqa/pack-author - packages/server/package.json: replaced @aqa/kit dep with @aqa/pack-author - packages/kit/package.json: added @aqa/pack-author alongside @aqa/server Tests: 2 new node:test cases in pack-author/test confirm the package boundary (callable + structured error). All heavy behavior coverage stays in packages/kit/test/pack-new.test.ts via the re-export shim. Build now passes in topo order: pack-author → server → kit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(v1.9): address Copilot iter 2 on admin PR #54 3 actionable comments, all addressed: 1. Stale comment in packages/server/src/api.ts said "Delegates to runPackNew from @aqa/kit" but the import was updated to @aqa/pack-author. Aligned the comment with the implementation. 2. packages/pack-author/package.json declared files: ["dist", "README.md"] but README.md was missing — publish would ship an empty/missing README and violate the repo guideline (every package README required). Wrote a real README explaining the cycle-break rationale and API surface. 3. packages/pack-author/test header claimed runPackNew was the "default-export shape"; it's actually a named export. Reworded. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(v1.9): address Copilot iter 3 on admin PR #54 2 actionable comments, both addressed: 1. Stale comment in admin.ts said "Lazy dynamic imports break what would otherwise be a static dependency cycle: @aqa/server depends on @aqa/kit (for runPackNew)". This was true in an earlier iteration of PR #54, but the cycle is now properly broken by extracting `runPackNew` into @aqa/pack-author. Reworded: the dynamic import is now an optimisation (defer makeApi/RunnerQueue loading to the actual `aqa admin` invocation), not a cycle workaround. 2. `--host 0.0.0.0` in help text had no security warning. `aqa admin` runs without real authentication — binding to LAN exposes the in-memory store + makeApi() to any peer on the network. Added an explicit WARNING block to the help text spelling out the risk and the safe use cases (dev VM / isolated CI runner). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(v1.9): rewrite junior quick-start to match shipped CLI surface (sub-task 5) (#56) The v1.8 README claimed `bun add -d agentic-qa-kit` worked (npm 404), `aqa install-agent-files` worked (CLI verb didn't exist), `aqa report` worked (CLI verb didn't exist), and `bun --filter @aqa/admin dev` was how juniors boot the admin (only works inside the monorepo). v1.9 closes those gaps in code (PRs #52/#53/#54/#55); this PR brings the docs back in sync. ## README - New step 2: `.npmrc` snippet for GitHub Packages auth + `GITHUB_TOKEN` export. Public packages on GH Packages still require auth — this is the single biggest junior trap and now has its own step. - Step 3: `bun add -d @padosoft/agentic-qa-kit` (replaces the npm-404 `agentic-qa-kit`). - Step 4 bundles `init` + `doctor` + `validate` so juniors verify the install before moving on. - Step 5: `install-agent-files --targets` — now matches a real verb. - Step 6: explicit `risk-map.yaml` edit with a worked example invariant. - Step 7: `aqa run --profile smoke` with explicit pointer to the per-run artifacts under `.aqa/runs/<id>/`. - Step 8: `aqa report` with format flag matrix. - Step 9: `aqa admin` — single command boots SPA + API (replaces `bun --filter @aqa/admin dev`). - Step 10: reproduce from artifacts. - Footer: `bun run e2e:ecosystem` pointer for monorepo contributors. - "How you use it" 8-step model reflects the new ordering. - Roadmap rows added for v1.8 and v1.9. - Status line bumped: `v1.7 current` → `v1.9 current`. ## docs/getting-started.md Section 1 new: GitHub Packages auth setup with PAT scope + .npmrc + env export. Section 2: bun add @padosoft/agentic-qa-kit. Sections 3–9: init/doctor/validate/install-agent-files/risk/run/report/admin in order, each with the real flag matrix. Closing "Where to go next" links the new PACK-AUTHORING doc. ## CHANGELOG Added [1.9.0] entry covering the 5 PRs + the kit↔server cycle fix + the bundle/publish-prep mechanics. Backfilled [1.8.1] / [1.8.2] / [1.8.3] which had only been recorded in PROGRESS.md. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(v1.9): GitHub Packages publish pipeline (sub-task 4) (#55) * feat(v1.9): GitHub Packages publish pipeline (sub-task 4) Adds the infrastructure to publish @padosoft/agentic-qa-kit as a single bundled tarball to GitHub Packages on every v* tag. ## What lands - esbuild bundler (packages/kit/scripts/build-bundle.mjs): - Walks the kit's import graph and inlines every workspace dep (@aqa/*) plus npm deps (yaml, kleur, zod, …) into one dist/cli.cjs file. - Externalises only Node built-ins (assert / fs / http / process / crypto / …) in BOTH prefix forms (`fs` and `node:fs`) — some bundled CJS deps (yaml, ajv) call `require('process')` without the prefix and would otherwise hit esbuild's "Dynamic require" stub. - Emits CJS-in-.cjs because the kit's package.json sets `"type": "module"`. Plain `.js` would be parsed as ESM and the bundled CJS wrappers would throw on `require('process')`. - Dedupes shebangs (tsc preserves the source `#!/usr/bin/env node` and esbuild keeps it; a double shebang is a SyntaxError). - Sourcemap is emitted so crash stacks point at the original TypeScript sources. - chmod 0o755 on POSIX so the bin script is executable. - Writes dist/cli.bundle.meta.json sentinel for CI to verify the artifact actually got produced. - Verified locally: bundle is ~460 KB; `node cli.cjs --version` and `--help` work. - Publish-time package.json rewriter (packages/kit/scripts/publish-prep.mjs): - Substitutes `name` from `aqa.publishName` (= @padosoft/agentic-qa-kit). GitHub Packages requires `<scope> === <owner>`, but renaming the workspace permanently would break every internal import path. - Pins every `workspace:*` dep to the kit's current version. - Idempotent; rewrite lives only in the CI checkout, never committed. - GitHub Actions workflow (.github/workflows/publish.yml): - Tag-triggered on `v*`, permissions: contents:read + packages:write + id-token:write. - Verifies tag matches packages/kit/package.json version. - bun install → bun run build → verify bundle exists → publish-prep → npm publish --provenance --access public to npm.pkg.github.com. - Pack-author extraction (packages/pack-author/): - Same cycle fix as sub-task 3 (kit ↔ server cycle on runPackNew). Required here too so sub-task 4 builds standalone in CI; the duplicate-add merges cleanly with sub-task 3's identical files. - Moved packages/kit/src/commands/pack-new.ts → pack-author/src/index.ts (git-move preserves history). Kit keeps a 5-line re-export shim so existing CLI imports work unchanged. - @aqa/server now imports runPackNew from @aqa/pack-author (was @aqa/kit). - Added 2 boundary tests on pack-author. - Kit package.json: - version 0.0.1 → 1.9.0 (target tag). - publishConfig.registry → https://npm.pkg.github.com. - aqa.publishName declares the @padosoft scope override. - main/exports/bin/files point at the new bundled dist/cli.cjs. - Build-bundle smoke test (packages/kit/test/build-bundle.test.ts): - 4 tests: bundle present + executable + shebang + size bounds (skipped if not built); publish-prep rewrites + errors on missing publishName. - Uses a hermetic temp dir for the publish-prep tests so the real packages/kit/package.json is never touched even on assertion failure. All gates green locally: 9 packages, 219 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(v1.9): address Copilot iter 1 on publish-pipeline PR #55 6 actionable comments, all addressed: 1. CRITICAL — publish-prep now STRIPS @aqa/* deps entirely (was only pinning workspace:* → version). The @aqa/* packages are inlined into dist/cli.cjs by esbuild and are NOT separately published, so leaving them in `dependencies` makes `bun add @padosoft/agentic-qa-kit` fail with "404 — @aqa/runner not found on registry". Test fixture updated to include both @aqa/* (must be stripped) and non-@aqa workspace:* (must be pinned) so the new behaviour is asserted. 2. build-bundle.mjs header comment said "ESM file at dist/cli.js"; the implementation emits CJS to dist/cli.cjs. Rewrote the docstring to match — plus added the "why CJS-in-.cjs" explanation (bundled deps call `require('process')`; the .cjs extension overrides "type": "module"). 3. test/build-bundle.test.ts header similarly out of date (`dist/cli.js`). Updated. 4. packages/pack-author/test header claimed runPackNew was "default-export shape", but it's a named export. Reworded to "NAMED export". 5. packages/pack-author missing README.md (declared in `files`). Wrote a real README explaining why the package exists (kit↔server cycle) and the API surface. 6. server/api.ts comment still said "Delegates to runPackNew from @aqa/kit" — implementation now imports from @aqa/pack-author. Updated. The `dist/admin` not-in-files note from review is intentional on this sub-task branch — the bundle-admin script is added by sub-task 3 (PR #54). Macro merge will combine both: sub-task 3 adds dist/admin to files, sub-task 4 adds dist/cli.cjs + publish prep. All gates green: 220 tests, lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(v1.9): address Copilot iter 2 on publish-pipeline PR #55 5 more actionable comments, all addressed: 1. CRITICAL — packages/kit/LICENSE was declared in `files` but didn't exist (only the repo root has LICENSE). Publishing from packages/kit would omit the license. Copied the root Apache-2.0 LICENSE into packages/kit/LICENSE so the published tarball carries it. 2. build-bundle.mjs header still referenced `dist/admin/` and bundle-admin.mjs — both belong to sub-task 3 (PR #54). Clarified that this sub-task only ships the CLI bundler; the admin static directory + its bundler land in the macro merge alongside PR #54. 3. Stale "tarball's `bin: dist/cli.js`" in build-bundle.mjs inner comment — fixed to cli.cjs. 4. Stale shebang-dedup comment claimed esbuild's banner adds the duplicate shebang; the banner option is no longer used today. Reworded as a defensive idempotent post-process so future re-introduction of a banner doesn't silently break the bundle. 5. Test now asserts the bundle's executable bit (0o111 mask) on POSIX. Skipped on Windows where mode bits don't carry POSIX semantics. The "drop types export" concern is intentional: @aqa/kit was only consumed programmatically by @aqa/server (for runPackNew); that's now via @aqa/pack-author. No remaining consumer imports anything from @aqa/kit's API surface, so the published package can be CLI- only without breaking anyone. The PR 56 docs-claim-don't-exist comments are correct in isolation but will be resolved at macro merge time (sub-task 5 PR's docs match the post-merge macro state, not its own branch state). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(v1.9): record macro closure in PROGRESS + LESSON --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Goal
Make the admin reachable from a plain npm install.
bun --filter @aqa/admin devonly worked inside the monorepo, breaking the README quick-start for downstream juniors.What changed
packages/kit/src/commands/admin.ts(runAdmin({root, port, host, adminDistDir?, runsRoot?})) — node:http server that serves bundled SPA + delegates/api/*to makeApi().packages/kit/scripts/bundle-admin.mjs— copiespackages/admin/dist/intopackages/kit/dist/admin/at build time so the SPA ships in the kit tarball.case 'admin'inpackages/kit/src/cli/aqa.tswith--port/--hostflags + signal-based clean shutdown.@aqa/server,@aqa/store,@aqa/authadded as workspace deps of@aqa/kit. Cycle (@aqa/serveralready depends on@aqa/kitforrunPackNew) is kept shallow via dynamic imports of@aqa/server/@aqa/storeinsiderunAdmin(); type imports stay static.@aqa/server/index.tsre-exportsApiHandler,ApiMethod,ApiRequest,ApiResponse(needed by kit).test/admin-cmd.test.ts(boot on port 0, static + API + traversal + missing-dist + bad-port).Junior-readable command
Seeds in-memory store from
.aqa/runs/<id>/{events,findings}.jsonlso the admin shows real local data out of the box.DoD checklist
Notes for reviewer
Sub-task 3 of macro
task/v1.9-junior-quickstart-truthing. Will conflict with sub-tasks 1 + 2 onpackages/kit/package.json(deps + test script) andpackages/kit/src/cli/aqa.ts(case statements + VALUE_FLAGS). Resolutions at macro-merge time are straightforward unions.Path traversal protection in static handler: rejects any URL whose normalised path doesn't start with the SPA dist directory.
Dep cycle is handled — bun workspaces install fine; the runtime cycle is broken by
await import()insiderunAdmin().