diff --git a/bun.lock b/bun.lock index f30e367..d957745 100644 --- a/bun.lock +++ b/bun.lock @@ -156,6 +156,7 @@ "version": "0.0.1", "dependencies": { "@aqa/auth": "workspace:*", + "@aqa/kit": "workspace:*", "@aqa/schemas": "workspace:*", "@aqa/store": "workspace:*", }, diff --git a/docs/PROGRESS.md b/docs/PROGRESS.md index 8503f4a..3a744a2 100644 --- a/docs/PROGRESS.md +++ b/docs/PROGRESS.md @@ -28,6 +28,7 @@ ## 2026-05-18 +- **v1.7 slices 1+2 shipped — pack authoring tutorial + `aqa pack new` CLI.** PR #25 merged (`6cc0013`), prerelease tag `v1.7.0-rc.1` published. **19 review iterations** with Copilot + Codex; the convergence pattern hit a sharp tail (5→1→4→2→1→2→0 real items per round) after Copilot started re-flagging the same ~13 already-addressed comments. Real issues caught and fixed before merge: slug-length validation against derived-ID schema cap (52-char limit), in-memory schema validation of generated Scenario/RiskMap/PackManifest before writing, symlink rejection at both packs/ parent and packDir, non-directory parent rejection, atomic backup-rename `--force` (failed scaffolds restore the original pack), `package.json#files` matching reality, scoped publish guidance, schema-valid profile snippet, integration test asserts `scn-pack-demo-starter` actually executed (rejects false-positives via bundled packs), honest NO_NETWORK_PROBE documentation. 54 tests in `@aqa/kit` (12 pack-new + 42 run-cmd). **Still pending in v1.7:** slice 3 (admin Create-pack wizard) and slice 4 (audit + wire/implement 81 silent admin placeholder buttons, plan in `docs/internal/admin-placeholder-audit.md`). Final `v1.7.0` tag after those slices ship. - **v1.6 shipped — `aqa run` + bundled packs + ecosystem foundation.** PR #24 merged (`21d7b10`), tag `v1.6.0` pushed, GitHub release published. The CLI now has the missing `aqa run` command that closes the loop between `aqa init` and a real audit trail. **21 review iterations** with Copilot + Codex, every one surfacing a real bug or coverage gap (zero false alarms). 42 TDD tests in `packages/kit/test/run-cmd.test.ts` cover every behavior. Highlights: SUT-aware init pack selection, three-tier pack discovery (project / node_modules / kit-bundled — all 5 baseline packs now ship inside `@aqa/kit`'s tarball via `bundle-packs.mjs`), atomic run-dir creation (TOCTOU-safe for concurrent seeded runs), path-traversal + symlink-escape rejection, `applies_when` filtering, manifest-name dedup with priority, legacy bare-slug aliasing, agent-mode rejection until that driver lands, unrelated-broken-pack tolerance with structured `warnings`, capped error strings (`MAX_DETAIL_PER_KIND`), detail samples in `run_finished` audit event for auditors. **Known scoped follow-ups:** real HTTP probe runner (current is no-network stub → release-gate strict semantics deferred), `EventChainWriter` ↔ `verifyEventChain` canonical-form reconciliation, browser-driven ecosystem smoke. - **Next macro task — v1.7 pack-authoring story.** Per user confirmation: (a) `docs/PACK-AUTHORING.md` community tutorial, (b) `aqa pack new ` CLI scaffolding, (c) Admin "Create pack" wizard wired over the new CLI. PLUS: a full audit pass on every placeholder button/interaction in the admin panel — no `onClick={() => {}}` or no-op silent clicks. Each placeholder either gets wired to a real endpoint, gets a client-side implementation, or gets an explicit "decorative" doc note. - **v1.5 admin design integration shipped.** PR #23 merged (`f7b879f`), tag `v1.5.0` pushed, GitHub release created. The 30-screen hi-fi prototype from Claude Design is now the official admin web panel: bundled into `packages/admin/src/app.tsx` (8.9k LOC, `@ts-nocheck`), token-driven CSS, Vite production build. New `E2E (Playwright, admin UI)` CI job runs the full Playwright suite (`*.e2e.ts`) — per-screen smoke for all 19 nav routes + audit-chain verify (OK/tampered) + Findings views (Clusters/List/Kanban) + Replay tabs + risk-map matrix + theme + palette. Total 36 Playwright tests green in 1m27s. **Known scoped tradeoffs (deferred):** in-memory routing only (not URL-driven), live-mode still reads in-file mocks (no real fetch layer wired). Both intentional for the design port; will be picked up in v1.6. diff --git a/docs/internal/admin-placeholder-audit.md b/docs/internal/admin-placeholder-audit.md index d964d80..dba8122 100644 --- a/docs/internal/admin-placeholder-audit.md +++ b/docs/internal/admin-placeholder-audit.md @@ -54,7 +54,7 @@ The line numbers below were captured against commit `~v1.6.0` (post-merge of PR ### Packs (lines ~6850-7040) -- `6850`, `6853` — top-bar "Import manifest" / "Install pack" (slice 3 wires "Install pack" via the new wizard) +- ~~`6850`, `6853` — top-bar "Import manifest" / "Install pack" (slice 3 wires "Install pack" via the new wizard)~~ **DONE (slice 3, PR #26):** the "Install pack" placeholder was renamed to "Create pack" and wired to the new `` component that POSTs to `/api/packs/scaffold`. "Import manifest" remains a placeholder for slice 4b. - `6910`, `6914`, `6925` — pack row actions (toggle / inspect) - `6986` — pack detail actions - `7028`, `7033`, `7038` — scenario picker inside pack detail diff --git a/packages/admin/src/app.tsx b/packages/admin/src/app.tsx index 1a00d8b..6ac4c10 100644 --- a/packages/admin/src/app.tsx +++ b/packages/admin/src/app.tsx @@ -4153,6 +4153,322 @@ function FindingsKanban({ findings: initialFindings, onConfirmTerminal }) { Object.assign(window, { AuditChainViewer, LiveTerminal, ReplayCommandPanel, FindingsKanban }); +// ============================================================= +// Create-pack wizard (v1.7 slice 3) +// ============================================================= +// Front-end for POST /api/packs/scaffold. The endpoint delegates to +// `runPackNew` from @aqa/kit, the same code path as the `aqa pack new` +// CLI — so the form validation here is a usability layer; authoritative +// validation lives server-side. We deliberately keep the form thin: a +// slug + sut-type covers the minimum viable pack, and the optional +// fields (description/author/license/force) are collapsed behind an +// "Advanced" disclosure. +function CreatePackWizard({ open, onClose }) { + const [slug, setSlug] = React.useState(''); + const [sutType, setSutType] = React.useState('api'); + const [description, setDescription] = React.useState(''); + const [author, setAuthor] = React.useState(''); + // license is intentionally empty by default — only forwarded when the + // user explicitly types something in Advanced. Otherwise we'd silently + // bake "Apache-2.0" into every pack scaffolded from the admin, which + // overrides whatever default `runPackNew` would otherwise pick and + // surprises users in orgs with a different default license. + const [license, setLicense] = React.useState(''); + const [force, setForce] = React.useState(false); + const [advancedOpen, setAdvancedOpen] = React.useState(false); + const [submitting, setSubmitting] = React.useState(false); + const [error, setError] = React.useState(null); + const [result, setResult] = React.useState(null); + const toast = useToast(); + + // Mirror the Slug schema regex from @aqa/schemas so the user gets + // immediate feedback rather than waiting for the server round-trip. + // The server is still the source of truth (this is a usability check, + // not a security boundary). + const SLUG_PATTERN = /^[a-z0-9](?:-?[a-z0-9])*$/; + const MAX_SLUG_LEN = 52; + const slugTrimmed = slug.trim(); + const slugError = (() => { + if (slugTrimmed === '') return null; // empty is "not yet entered", not an error + if (slugTrimmed.length > MAX_SLUG_LEN) { + return `${slugTrimmed.length} chars — max ${MAX_SLUG_LEN}`; + } + if (!SLUG_PATTERN.test(slugTrimmed)) { + return 'lowercase a-z, 0-9, single dashes only'; + } + return null; + })(); + const canSubmit = slugTrimmed !== '' && slugError === null && !submitting; + + function reset() { + setSlug(''); + setSutType('api'); + setDescription(''); + setAuthor(''); + setLicense(''); + setForce(false); + setAdvancedOpen(false); + setError(null); + setResult(null); + setSubmitting(false); + } + + function handleClose() { + if (submitting) return; // don't abandon a request mid-flight + reset(); + onClose?.(); + } + + async function handleSubmit() { + if (!canSubmit) return; + setSubmitting(true); + setError(null); + try { + const body = { + slug: slugTrimmed, + sut_type: sutType, + ...(description.trim() ? { description: description.trim() } : {}), + ...(author.trim() ? { author: author.trim() } : {}), + ...(license.trim() ? { license: license.trim() } : {}), + ...(force ? { force: true } : {}), + }; + const res = await fetch('/api/packs/scaffold', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }); + const text = await res.text(); + let parsed = null; + try { + parsed = text ? JSON.parse(text) : null; + } catch { + parsed = null; + } + if (!res.ok) { + const msg = parsed?.error ?? `HTTP ${res.status}`; + setError(msg); + toast.push({ kind: 'error', title: 'Create pack failed', body: msg }); + return; + } + setResult(parsed); + toast.push({ + kind: 'success', + title: 'Pack scaffolded', + body: parsed?.pack_dir ?? slugTrimmed, + }); + } catch (e) { + // Network error / no server / CORS. In mock-data dev (admin running + // without @aqa/server), we surface a clear message instead of a + // generic failure so the user knows it's an environment thing. + // Surface BOTH the inline alert and a toast for parity with the + // HTTP-failure branch above — otherwise the user gets different + // feedback for "server returned 500" vs "couldn't reach server" + // even though both mean "your request didn't succeed". + const msg = e instanceof Error ? e.message : String(e); + const full = `Could not reach /api/packs/scaffold (${msg}). The admin is in mock-data mode or the server is down — no files were written.`; + setError(full); + // Toast body uses `full` (with the mock-mode hint) rather than the + // raw exception `msg` — that hint is the most useful piece of + // context when a fetch fails, and the toast disappears on its + // own so the user might miss it if it only contains the cryptic + // exception message. + toast.push({ kind: 'error', title: 'Create pack failed', body: full }); + } finally { + setSubmitting(false); + } + } + + return ( + /packs//. Same code path as `aqa pack new` on the CLI.' + } + size="md" + footer={ + result ? ( + <> + + + ) : ( + <> + + + + ) + } + > + {result ? ( +
+ +
+
+ Location:{' '} + + {result.pack_dir} + +
+
+ Files:{' '} + + {(result.files ?? []).join(', ')} + +
+
+ +
+ Next: open pack.yaml and scenarios/starter.yaml in your editor, + replace the placeholder probe URL and oracle with real ones, then wire this pack into a + profile in .aqa/profiles.yaml. +
+
+ ) : ( +
+ {error && ( + + {error} + + )} +
+ + setSlug(e.target.value)} + autoFocus + /> +
+ {slugError + ? slugError + : 'lowercase a-z and 0-9 with single dashes; up to 52 chars. Used as both the manifest `name:` and the on-disk directory name.'} +
+
+
+ + +
+ Controls applies_when.sut_type in the generated manifest. The pack will + only run against projects whose detected SUT type matches. +
+
+ + {advancedOpen && ( + <> +
+ + setDescription(e.target.value)} + /> +
+
+ + setAuthor(e.target.value)} + /> +
+
+ + setLicense(e.target.value)} + /> +
+ Leave blank to use the kit's default (Apache-2.0). The CLI flag is{' '} + --license <spdx>. +
+
+ + + )} +
+ )} + + ); +} +Object.assign(window, { CreatePackWizard }); + // ============ shell.jsx ============ // ============================================================= // agentic-qa-kit · admin panel — Shell (Sidebar + TopBar + Palette) @@ -6900,6 +7216,7 @@ Object.assign(window, { PageFindings, PageFindingDetail, PageRiskMap, PageRiskEd // ---------------- Packs ---------------- function PagePacks({ onNavigate, onOpenPack }) { + const [createOpen, setCreateOpen] = React.useState(false); return (
Import manifest - } /> + setCreateOpen(false)} /> community-stripe@0.3.1 is installed without signature diff --git a/packages/admin/test/e2e/create-pack.e2e.ts b/packages/admin/test/e2e/create-pack.e2e.ts new file mode 100644 index 0000000..dc38607 --- /dev/null +++ b/packages/admin/test/e2e/create-pack.e2e.ts @@ -0,0 +1,104 @@ +import { expect, test } from '@playwright/test'; + +/** + * v1.7 slice 3 — Admin Create-pack wizard e2e tests. + * + * Covers: + * - clicking "Create pack" on the Packs screen opens the wizard + * - slug client-side validation surfaces an error and disables submit + * - on submit, the wizard POSTs to /api/packs/scaffold and renders the + * server's pack_dir + files in a success panel + * - on server-side 409 (already exists) the wizard shows the error + * without closing + * + * The dev server has no backend wired (admin runs in mock-data mode), + * so each test installs a Playwright route mock for /api/packs/scaffold + * that fakes the server response. This proves the wizard talks to the + * documented endpoint contract; the server unit tests in @aqa/server + * prove the endpoint itself behaves correctly. Both ends together close + * the loop. + */ + +async function navigateToPacks(page: import('@playwright/test').Page): Promise { + await page.goto('/'); + await expect(page.locator('.sidebar')).toBeVisible(); + await page + .locator('.nav-item', { hasText: /^Packs/i }) + .first() + .click(); + await expect(page.locator('.page-title, h1').first()).toContainText(/Packs/i); +} + +test.describe('Create-pack wizard', () => { + test('Create-pack button opens the wizard with required fields', async ({ page }) => { + await navigateToPacks(page); + await page.getByTestId('packs-create-btn').click(); + await expect(page.getByTestId('create-pack-slug')).toBeVisible(); + await expect(page.getByTestId('create-pack-sut')).toBeVisible(); + // Submit button is disabled until a valid slug is entered. + await expect(page.getByTestId('create-pack-submit')).toBeDisabled(); + }); + + test('invalid slug surfaces an error and keeps submit disabled', async ({ page }) => { + await navigateToPacks(page); + await page.getByTestId('packs-create-btn').click(); + const slug = page.getByTestId('create-pack-slug'); + await slug.fill('Bad Name!'); + // Hint text turns into the error message. The wizard renders inside + // .modal-body, so scope the locator there to avoid colliding with + // any other .field-hint on the page (e.g. forms in modals stacked + // behind us, or hints in side panels). + await expect(page.locator('.modal-body .field-hint').first()).toContainText(/lowercase|chars/i); + await expect(page.getByTestId('create-pack-submit')).toBeDisabled(); + }); + + test('valid slug + happy-path 201 renders success panel with pack_dir', async ({ page }) => { + // Mock the server endpoint before navigating so the route is in place + // by the time the wizard's fetch fires. + await page.route('**/api/packs/scaffold', async (route) => { + const body = route.request().postDataJSON(); + expect(body.slug).toBe('pack-e2e-demo'); + expect(body.sut_type).toBe('api'); + await route.fulfill({ + status: 201, + contentType: 'application/json', + body: JSON.stringify({ + pack_dir: '/tmp/aqa-demo/packs/pack-e2e-demo', + files: ['pack.yaml', 'scenarios/starter.yaml', 'risks/starter.yaml', 'README.md'], + }), + }); + }); + await navigateToPacks(page); + await page.getByTestId('packs-create-btn').click(); + await page.getByTestId('create-pack-slug').fill('pack-e2e-demo'); + await page.getByTestId('create-pack-submit').click(); + const result = page.getByTestId('create-pack-result'); + await expect(result).toBeVisible(); + await expect(result).toContainText('/tmp/aqa-demo/packs/pack-e2e-demo'); + await expect(result).toContainText('pack.yaml'); + }); + + test('server-side 409 conflict renders error and keeps wizard open', async ({ page }) => { + await page.route('**/api/packs/scaffold', async (route) => { + await route.fulfill({ + status: 409, + contentType: 'application/json', + body: JSON.stringify({ + error: + 'pack directory /tmp/aqa-demo/packs/pack-e2e-dup already exists; pass --force to overwrite', + }), + }); + }); + await navigateToPacks(page); + await page.getByTestId('packs-create-btn').click(); + await page.getByTestId('create-pack-slug').fill('pack-e2e-dup'); + await page.getByTestId('create-pack-submit').click(); + // The error alert appears, the form stays mounted, and the result + // panel does NOT appear. + await expect(page.locator('.alert', { hasText: /already exists/i })).toBeVisible(); + await expect(page.getByTestId('create-pack-result')).toHaveCount(0); + // Toggling "Force overwrite" is the documented next step — assert it's reachable. + await page.getByTestId('create-pack-advanced-toggle').click(); + await expect(page.getByTestId('create-pack-force')).toBeVisible(); + }); +}); diff --git a/packages/kit/src/commands/pack-new.ts b/packages/kit/src/commands/pack-new.ts index 007f8a9..d9a2a4d 100644 --- a/packages/kit/src/commands/pack-new.ts +++ b/packages/kit/src/commands/pack-new.ts @@ -35,6 +35,23 @@ export interface PackNewOptions { license?: string; } +/** + * Structured error code returned alongside `error` on failure. Lets + * callers distinguish causes programmatically without regex-matching + * the human-readable error string: + * + * - `EEXIST` — the target pack directory already exists and `force` + * was not set. The right HTTP mapping is 409 Conflict. + * - `EINVAL` — the input was invalid (bad slug, unsupported + * sut-type, slug too long, malformed manifest). 400. + * - `EIO` — filesystem operation failed (permission, disk full, + * cross-device rename). 500. + * + * Always present on failure; callers can safely switch on it. The + * `error` field carries the human-readable detail. + */ +export type PackNewErrorCode = 'EEXIST' | 'EINVAL' | 'EIO'; + export interface PackNewResult { ok: boolean; /** Absolute path to the scaffolded pack directory. Present only on success. */ @@ -42,6 +59,8 @@ export interface PackNewResult { /** Files created, relative to `packDir`. Present only on success. */ files?: string[]; error?: string; + /** Structured cause; see {@link PackNewErrorCode}. Present only on failure. */ + code?: PackNewErrorCode; } const VALID_SUT_TYPES = new Set(['api', 'web', 'cli', 'lib', 'agent', 'pipeline']); @@ -52,8 +71,15 @@ const SLUG_PATTERN = /^[a-z0-9](?:-?[a-z0-9])*$/; // later reject the scaffolded files. const MAX_SLUG_LEN = 52; -function makeError(error: string): PackNewResult { - return { ok: false, error }; +/** + * Build a failure result. `code` defaults to `EINVAL` because the vast + * majority of failures are user-input or schema-validation issues; the + * two non-default sites (`EEXIST` for an existing packDir without + * --force, and `EIO` for actual filesystem operation failures) pass + * the right code explicitly. + */ +function makeError(error: string, code: PackNewErrorCode = 'EINVAL'): PackNewResult { + return { ok: false, error, code }; } /** Starter URL each SUT-type's example probe points at. Stub returns 200 either way. */ @@ -110,7 +136,10 @@ export function runPackNew(opts: PackNewOptions): PackNewResult { try { parentStat = lstatSync(packsParent); } catch (e) { - return makeError(`cannot stat ${packsParent}: ${e instanceof Error ? e.message : String(e)}`); + return makeError( + `cannot stat ${packsParent}: ${e instanceof Error ? e.message : String(e)}`, + 'EIO', + ); } if (parentStat.isSymbolicLink()) { return makeError( @@ -150,6 +179,7 @@ export function runPackNew(opts: PackNewOptions): PackNewResult { } catch (e) { return makeError( `cannot stat pack directory ${packDir}: ${e instanceof Error ? e.message : String(e)} — refusing to scaffold (cannot confirm path is not a symlink)`, + 'EIO', ); } if (isSymlink) { @@ -158,7 +188,10 @@ export function runPackNew(opts: PackNewOptions): PackNewResult { ); } if (!opts.force) { - return makeError(`pack directory ${packDir} already exists; pass --force to overwrite`); + return makeError( + `pack directory ${packDir} already exists; pass --force to overwrite`, + 'EEXIST', + ); } existingPackDirNeedsRm = true; } @@ -317,6 +350,7 @@ See the [pack authoring guide](https://github.com/padosoft/agentic-qa-kit/blob/m } catch (e) { return makeError( `cannot rename existing pack directory ${packDir} → ${backupDir} (needed to make --force non-destructive): ${e instanceof Error ? e.message : String(e)}`, + 'EIO', ); } } @@ -440,8 +474,10 @@ function rollbackAndError( // user at the backup path so they can recover manually. return makeError( `${message} (rollback FAILED — your original pack is at ${backupDir}, please restore it manually: ${restoreErr instanceof Error ? restoreErr.message : String(restoreErr)})`, + 'EIO', ); } } - return makeError(message); + // Write/round-trip phase failures are all EIO (FS or serializer issues). + return makeError(message, 'EIO'); } diff --git a/packages/kit/src/index.ts b/packages/kit/src/index.ts index 0182c32..dabe7f7 100644 --- a/packages/kit/src/index.ts +++ b/packages/kit/src/index.ts @@ -3,3 +3,9 @@ export type { ProjectProfile } from './profiler.js'; export { runInit } from './commands/init.js'; export { runDoctor } from './commands/doctor.js'; export { runValidate } from './commands/validate.js'; +export { runPackNew } from './commands/pack-new.js'; +export type { + PackNewErrorCode, + PackNewOptions, + PackNewResult, +} from './commands/pack-new.js'; diff --git a/packages/server/package.json b/packages/server/package.json index 3899ffb..70155e6 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -16,9 +16,10 @@ "clean": "node --input-type=module -e \"import { rmSync } from 'node:fs'; for (const p of ['dist','.tsbuildinfo']) { try { rmSync(p, { recursive: true, force: true }); } catch {} }\"" }, "dependencies": { + "@aqa/auth": "workspace:*", + "@aqa/kit": "workspace:*", "@aqa/schemas": "workspace:*", - "@aqa/store": "workspace:*", - "@aqa/auth": "workspace:*" + "@aqa/store": "workspace:*" }, "publishConfig": { "access": "public" } } diff --git a/packages/server/src/api.ts b/packages/server/src/api.ts index 372c932..4d9f4a4 100644 --- a/packages/server/src/api.ts +++ b/packages/server/src/api.ts @@ -1,4 +1,6 @@ import type { User, allows } from '@aqa/auth'; +import { runPackNew } from '@aqa/kit'; +import type { PackNewErrorCode } from '@aqa/kit'; import type { ApiToken, CostSummary, @@ -21,6 +23,17 @@ export interface ApiContext { queue: RunnerQueue; /** Resolve the authenticated user from the request. */ authenticate: (headers: Record) => Promise; + /** + * Absolute on-disk path of the project the server manages. Set at boot. + * Endpoints that scaffold or modify files anchor to this path and NEVER + * accept a client-supplied root — that would let an authenticated caller + * write anywhere the server process can reach. + * + * Optional so existing tests / lightweight integrations that don't touch + * the filesystem keep working; FS-touching endpoints return 400 when + * unset rather than silently writing to cwd. + */ + projectRoot?: string; } export type ApiMethod = 'GET' | 'POST' | 'PUT' | 'DELETE'; @@ -58,6 +71,22 @@ function scope(req: ApiRequest): { org?: string; project?: string } { return out; } +/** + * Map a `runPackNew` error code to an HTTP status. Stable mapping — + * unlike regex-matching the human-readable error string, this survives + * any rewording of the underlying error messages. + */ +function errorCodeToStatus(code: PackNewErrorCode | undefined): number { + switch (code) { + case 'EEXIST': + return 409; + case 'EIO': + return 500; + default: + return 400; + } +} + function requireScope(req: ApiRequest): { org: string; project: string } | ApiResponse { const s = scope(req); if (!s.org || !s.project) { @@ -254,6 +283,127 @@ export function makeApi(): ApiHandler[] { return asResponse({ ok: true }); }, }, + { + // v1.7 slice 3 — scaffold a new pack on disk for the Admin + // "Create pack" wizard. Delegates to `runPackNew` from `@aqa/kit` + // (the same code path as `aqa pack new` on the CLI) so the two + // UIs stay in lockstep on validation, atomic --force, etc. + // + // Synchronous FS work in an async handler: `runPackNew` is sync + // (mkdir / writeFile / rename of ~5 small files plus a few schema + // validations — typical wall-clock ~10ms on a developer laptop). + // For the admin's create-pack flow that's an explicit, infrequent + // user action (clicked from a wizard), so blocking the event loop + // for that brief window is an acceptable tradeoff vs the cost of + // duplicating runPackNew's logic in an async variant. If this + // endpoint ever gets called from a high-fanout path (e.g. bulk + // pack import), revisit and offload. + // + // Tenancy: this endpoint deliberately writes to a single, + // server-global `ctx.projectRoot` and ignores `x-aqa-org` / + // `x-aqa-project` headers — unlike the other pack endpoints + // which are scope-aware. The intent is single-tenant only: the + // server manages exactly one on-disk project, the wizard creates + // packs in that project's `packs/` directory, end of story. + // A multi-tenant deployment that wants per-tenant pack scaffold + // must front this endpoint with a per-tenant `projectRoot` (e.g. + // by booting a separate server process per tenant, or layering a + // routing proxy that picks the right root). Doing in-process + // per-tenant scaffolding here would require materially more + // design — at minimum, where to root the per-tenant directories, + // how `aqa run`'s default discovery interacts with that layout, + // and whether tenant isolation is enforced at the filesystem or + // at the API layer — and is intentionally out of scope for the + // v1.7 admin Create-pack wizard. + method: 'POST', + path: '/api/packs/scaffold', + requires: 'packs:install', + async handle(req, ctx) { + if (!ctx.projectRoot) { + return asResponse( + { + error: + 'server has no projectRoot configured — pack scaffolding requires the server to know which on-disk project to write into', + }, + 400, + ); + } + const body = (req.body ?? {}) as Record; + // Required fields — strict type + non-empty check. + if (typeof body.slug !== 'string' || body.slug.trim() === '') { + return asResponse({ error: 'slug is required (non-empty string)' }, 400); + } + if (typeof body.sut_type !== 'string' || body.sut_type.trim() === '') { + return asResponse({ error: 'sut_type is required (non-empty string)' }, 400); + } + // Slug length is delegated to runPackNew (MAX_SLUG_LEN=52) and + // surfaces as a 400 EINVAL. We don't duplicate the cap here so + // there's one source of truth — if MAX_SLUG_LEN ever changes, + // the API boundary follows automatically. + // Optional fields — strict type check at the API boundary so a + // client can't smuggle truthy non-booleans through `force` (which + // `runPackNew` checks via `if (!opts.force)` truthiness). A + // request like `{"force": "no"}` would be silently treated as + // force-enabled without this guard. + if (body.force !== undefined && typeof body.force !== 'boolean') { + return asResponse( + { error: 'force must be a boolean when provided (got non-boolean)' }, + 400, + ); + } + for (const k of ['description', 'author', 'license'] as const) { + const v = body[k]; + if (v !== undefined && typeof v !== 'string') { + return asResponse( + { error: `${k} must be a string when provided (got non-string)` }, + 400, + ); + } + } + // Trim the required string inputs before forwarding so a caller + // who sends `" pack-x "` gets the right validation outcome + // (it's a clean slug) rather than runPackNew's "must be + // lowercase alphanumeric" error rejecting the whitespace. + // The admin wizard already trims; this normalizes for direct + // API callers too. + // + // For optional string fields (description/author/license), trim + // AND drop empty/whitespace-only values from the forwarded + // payload — otherwise a request like `{"description": " "}` + // would write a blank `description:` line into the generated + // pack.yaml, which is worse than just falling back to the + // scaffolder's default ("Pack scaffolded by aqa pack new"). + function optStr(k: 'description' | 'author' | 'license'): string | undefined { + const v = body[k]; + if (typeof v !== 'string') return undefined; + const trimmed = v.trim(); + return trimmed === '' ? undefined : trimmed; + } + const description = optStr('description'); + const author = optStr('author'); + const license = optStr('license'); + const result = runPackNew({ + root: ctx.projectRoot, + slug: body.slug.trim(), + sutType: body.sut_type.trim(), + ...(body.force !== undefined ? { force: body.force as boolean } : {}), + ...(description !== undefined ? { description } : {}), + ...(author !== undefined ? { author } : {}), + ...(license !== undefined ? { license } : {}), + }); + if (!result.ok) { + // Map the structured `code` field to an HTTP status. This is + // stable across error-message wording changes, unlike the + // earlier regex-on-error approach. + const httpStatus = errorCodeToStatus(result.code); + return asResponse( + { error: result.error ?? 'unknown error', code: result.code ?? 'EINVAL' }, + httpStatus, + ); + } + return asResponse({ pack_dir: result.packDir, files: result.files ?? [] }, 201); + }, + }, // ============ Profiles ============ { diff --git a/packages/server/test/api.test.ts b/packages/server/test/api.test.ts index 1a464e6..4bb3093 100644 --- a/packages/server/test/api.test.ts +++ b/packages/server/test/api.test.ts @@ -1,5 +1,8 @@ import assert from 'node:assert/strict'; -import { describe, it } from 'node:test'; +import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { after, describe, it } from 'node:test'; import { MemoryStore } from '@aqa/store'; import { RunnerQueue, makeApi } from '../dist/index.js'; @@ -10,14 +13,39 @@ const FAKE_USER = { roles: ['admin' as const], }; -function ctx() { +function ctx(opts: { projectRoot?: string } = {}) { return { store: new MemoryStore(), queue: new RunnerQueue(), authenticate: async () => FAKE_USER, + // The server is configured at boot with the on-disk project root + // it manages. Endpoints that touch the filesystem (pack scaffold) + // anchor to this path — they NEVER honor a client-supplied root, + // since that would let an authenticated caller write anywhere the + // server process can reach. + projectRoot: opts.projectRoot, }; } +// Track every tmpdir created in this suite so a teardown hook can wipe +// them. Without this, every CI run leaks fully-scaffolded pack trees +// under the OS temp dir. +const TEMP_ROOTS: string[] = []; +function tmpProjectRoot(): string { + const root = mkdtempSync(join(tmpdir(), 'aqa-server-pack-')); + TEMP_ROOTS.push(root); + return root; +} +after(() => { + for (const root of TEMP_ROOTS) { + try { + rmSync(root, { recursive: true, force: true }); + } catch { + // best-effort; never fail the suite over a tmpdir we can't remove + } + } +}); + const TENANT_HEADERS = { 'x-aqa-org': 'padosoft', 'x-aqa-project': 'demo' }; describe('makeApi', () => { @@ -133,4 +161,224 @@ describe('makeApi', () => { assert.equal(res?.status, 200); assert.deepEqual((res?.body as { orgs: unknown[] }).orgs, []); }); + + // ============ v1.7 slice 3 — Pack scaffolding (Admin Create-pack wizard) ============ + + describe('POST /api/packs/scaffold', () => { + it('scaffolds a pack on disk under ctx.projectRoot/packs//', async () => { + const root = tmpProjectRoot(); + const c = ctx({ projectRoot: root }); + const route = makeApi().find((r) => r.method === 'POST' && r.path === '/api/packs/scaffold'); + assert.ok(route, 'POST /api/packs/scaffold must exist'); + const res = await route?.handle( + { headers: {}, params: {}, body: { slug: 'pack-admin-demo', sut_type: 'api' } }, + c, + ); + assert.equal( + res?.status, + 201, + `expected 201, got ${res?.status}: ${JSON.stringify(res?.body)}`, + ); + const body = res?.body as { pack_dir: string; files: string[] }; + assert.equal(body.pack_dir, join(root, 'packs', 'pack-admin-demo')); + assert.ok(body.files.includes('pack.yaml'), 'files list must include pack.yaml'); + assert.ok(existsSync(join(root, 'packs', 'pack-admin-demo', 'pack.yaml'))); + }); + + it('returns 400 when the server has no projectRoot configured', async () => { + const c = ctx(); // no projectRoot + const route = makeApi().find((r) => r.method === 'POST' && r.path === '/api/packs/scaffold'); + const res = await route?.handle( + { headers: {}, params: {}, body: { slug: 'pack-x', sut_type: 'api' } }, + c, + ); + assert.equal(res?.status, 400); + const body = res?.body as { error: string }; + assert.match(body.error, /projectRoot/i); + }); + + it('returns 400 on missing or invalid slug', async () => { + const root = tmpProjectRoot(); + const c = ctx({ projectRoot: root }); + const route = makeApi().find((r) => r.method === 'POST' && r.path === '/api/packs/scaffold'); + // missing slug + const a = await route?.handle({ headers: {}, params: {}, body: { sut_type: 'api' } }, c); + assert.equal(a?.status, 400); + // invalid slug + const b = await route?.handle( + { headers: {}, params: {}, body: { slug: 'Bad Name!', sut_type: 'api' } }, + c, + ); + assert.equal(b?.status, 400); + assert.match((b?.body as { error: string }).error, /slug/i); + }); + + it('returns 400 on unsupported sut_type', async () => { + const root = tmpProjectRoot(); + const c = ctx({ projectRoot: root }); + const route = makeApi().find((r) => r.method === 'POST' && r.path === '/api/packs/scaffold'); + const res = await route?.handle( + { headers: {}, params: {}, body: { slug: 'pack-z', sut_type: 'not-real' } }, + c, + ); + assert.equal(res?.status, 400); + assert.match((res?.body as { error: string }).error, /sut/i); + }); + + it('returns 409 when the pack already exists and force is not set', async () => { + const root = tmpProjectRoot(); + const c = ctx({ projectRoot: root }); + const route = makeApi().find((r) => r.method === 'POST' && r.path === '/api/packs/scaffold'); + const first = await route?.handle( + { headers: {}, params: {}, body: { slug: 'pack-dup', sut_type: 'api' } }, + c, + ); + assert.equal(first?.status, 201); + const second = await route?.handle( + { headers: {}, params: {}, body: { slug: 'pack-dup', sut_type: 'api' } }, + c, + ); + assert.equal(second?.status, 409); + }); + + it('overwrites with force=true', async () => { + const root = tmpProjectRoot(); + const c = ctx({ projectRoot: root }); + const route = makeApi().find((r) => r.method === 'POST' && r.path === '/api/packs/scaffold'); + await route?.handle( + { headers: {}, params: {}, body: { slug: 'pack-ow', sut_type: 'api' } }, + c, + ); + const res = await route?.handle( + { headers: {}, params: {}, body: { slug: 'pack-ow', sut_type: 'api', force: true } }, + c, + ); + assert.equal(res?.status, 201); + }); + + it('requires the packs:install permission', () => { + const route = makeApi().find((r) => r.method === 'POST' && r.path === '/api/packs/scaffold'); + assert.equal(route?.requires, 'packs:install'); + }); + + it('rejects non-boolean force with 400 (no truthy-string smuggling)', async () => { + // Regression test for PR #26 iter 1 (Codex P1 + Copilot): + // Without strict type validation, `{"force": "yes"}` would pass + // through the endpoint and `runPackNew` would treat the truthy + // string as enabled — silently overwriting an existing pack. + const root = tmpProjectRoot(); + const c = ctx({ projectRoot: root }); + const route = makeApi().find((r) => r.method === 'POST' && r.path === '/api/packs/scaffold'); + const res = await route?.handle( + { headers: {}, params: {}, body: { slug: 'pack-x', sut_type: 'api', force: 'yes' } }, + c, + ); + assert.equal(res?.status, 400); + assert.match((res?.body as { error: string }).error, /force.*boolean/i); + }); + + it('rejects non-string description/author/license with 400', async () => { + const root = tmpProjectRoot(); + const c = ctx({ projectRoot: root }); + const route = makeApi().find((r) => r.method === 'POST' && r.path === '/api/packs/scaffold'); + for (const k of ['description', 'author', 'license']) { + const res = await route?.handle( + { headers: {}, params: {}, body: { slug: 'pack-x', sut_type: 'api', [k]: 123 } }, + c, + ); + assert.equal(res?.status, 400, `${k}=number must be rejected`); + assert.match((res?.body as { error: string }).error, new RegExp(`${k}.*string`, 'i')); + } + }); + + it('treats empty/whitespace optional strings as undefined (no blank manifest fields)', async () => { + // Regression test for PR #26 iter 3 (Copilot): + // A request with `{"description": " "}` used to forward the + // whitespace string to runPackNew, which then baked a blank + // `description:` line into the generated pack.yaml. Now the + // endpoint trims and drops empty values, so runPackNew falls + // back to its own sensible default ("Pack scaffolded by aqa + // pack new"). The test asserts the request succeeds AND the + // resulting pack.yaml is NOT empty for that field. + const root = tmpProjectRoot(); + const c = ctx({ projectRoot: root }); + const route = makeApi().find((r) => r.method === 'POST' && r.path === '/api/packs/scaffold'); + const res = await route?.handle( + { + headers: {}, + params: {}, + body: { + slug: 'pack-blank', + sut_type: 'api', + description: ' ', + author: '', + license: '\t', + }, + }, + c, + ); + assert.equal(res?.status, 201); + const manifest = readFileSync(join(root, 'packs', 'pack-blank', 'pack.yaml'), 'utf8'); + // The scaffolder's fallback description must be present rather + // than a literal empty `description:` line. + assert.match( + manifest, + /description:\s+Pack scaffolded by aqa pack new/i, + 'whitespace-only description must fall back to the kit default, not write a blank line', + ); + }); + + it('trims whitespace around slug + sut_type before forwarding', async () => { + // Regression test for PR #26 iter 2 (Copilot): + // The endpoint used to check `slug.trim() !== ''` for non-empty + // but then forward the untrimmed value, so `" pack-trim "` + // would pass the boundary check and then runPackNew would reject + // it with the unrelated "must be lowercase alphanumeric…" error. + // Now the trim happens before forwarding, so a whitespace-padded + // valid slug succeeds end-to-end. + const root = tmpProjectRoot(); + const c = ctx({ projectRoot: root }); + const route = makeApi().find((r) => r.method === 'POST' && r.path === '/api/packs/scaffold'); + const res = await route?.handle( + { headers: {}, params: {}, body: { slug: ' pack-trim ', sut_type: ' api ' } }, + c, + ); + assert.equal( + res?.status, + 201, + `expected 201 after trim, got ${res?.status}: ${JSON.stringify(res?.body)}`, + ); + const body = res?.body as { pack_dir: string }; + assert.match(body.pack_dir, /pack-trim$/); + }); + + it('returns the structured code field alongside error on failure', async () => { + // Regression test for the brittle regex-on-error 409 mapping. + // The endpoint now uses runPackNew's structured `code` to pick + // the HTTP status; this test asserts the code also propagates + // to the response body so clients can act on it programmatically. + const root = tmpProjectRoot(); + const c = ctx({ projectRoot: root }); + const route = makeApi().find((r) => r.method === 'POST' && r.path === '/api/packs/scaffold'); + // First scaffold to set up the conflict. + await route?.handle( + { headers: {}, params: {}, body: { slug: 'pack-code', sut_type: 'api' } }, + c, + ); + // Second scaffold (same slug, no force) → EEXIST. + const dup = await route?.handle( + { headers: {}, params: {}, body: { slug: 'pack-code', sut_type: 'api' } }, + c, + ); + assert.equal(dup?.status, 409); + assert.equal((dup?.body as { code: string }).code, 'EEXIST'); + // EINVAL path: bad slug. + const bad = await route?.handle( + { headers: {}, params: {}, body: { slug: 'Bad!', sut_type: 'api' } }, + c, + ); + assert.equal(bad?.status, 400); + assert.equal((bad?.body as { code: string }).code, 'EINVAL'); + }); + }); });