Skip to content

feat(slice-4b): admin Import-manifest wizard + POST /api/packs/import#28

Merged
lopadova merged 3 commits into
mainfrom
task/v1.7-slice-4b-import-manifest
May 19, 2026
Merged

feat(slice-4b): admin Import-manifest wizard + POST /api/packs/import#28
lopadova merged 3 commits into
mainfrom
task/v1.7-slice-4b-import-manifest

Conversation

@lopadova
Copy link
Copy Markdown
Contributor

Summary

Second sub-slice of v1.7 slice 4. Wires the previously-silent "Import manifest" button on the Packs page to a real wizard that POSTs a YAML pack manifest to a new POST /api/packs/import endpoint. The server parses YAML, validates against @aqa/schemas/PackManifest, and installs via the store.

Complements slice 3 (PR #26, "Create pack" → scaffold) — together the two buttons on the Packs page top-bar are now both functional.

What changed

Server (@aqa/server):

  • New POST /api/packs/import (packs:install)
  • Body: { yaml: string, force?: boolean }
  • Same structured error code pattern as /api/packs/scaffold: EINVAL for bad input → 400, EEXIST for duplicate without force → 409, EIO for store failure → 500
  • Adds yaml@^2.9.0 as workspace dep

Admin (@aqa/admin):

  • New <ImportManifestWizard> modal with YAML textarea + native file picker
  • Force-overwrite checkbox for the 409 retry path
  • Success panel shows installed name + version + next-step guidance about the on-disk pack directory

Test plan

  • bun --filter @aqa/server test — 34 unit tests pass (26 existing + 8 new import endpoint tests)
  • bunx playwright test test/e2e/import-manifest.e2e.ts — 4 e2e tests pass
  • bun --filter @aqa/admin build — Vite 432 KB / 118 KB gzip
  • bun run typecheck — clean across all 19 workspaces
  • bun run lint — clean (4 pre-existing warnings, 0 errors)

Audit doc

  • Lines 6850/6853 (top-bar "Import manifest" / "Install pack") both marked DONE
  • Slice 4b checklist item marked SHIPPED

Up next

  • Slice 4c: Scenarios / Risks / Profiles CRUD (heaviest slice)
  • Slice 4d: Agents page
  • Slice 4e: Replay / Audit / Cost / Queue / Notifications
  • Slice 4f: Admin section (Users / Roles / SSO / Org / Tokens / Admin Audit)
  • Then: final v1.7.0 tag + README/docs refresh

🤖 Generated with Claude Code

v1.7 slice 4b — wires the "Import manifest" placeholder button on
the Packs page to a real wizard that POSTs a YAML pack manifest to
a new server endpoint, which parses + validates + installs into the
store.

## Server side (@aqa/server)

- New `POST /api/packs/import` endpoint (`packs:install` permission)
- Body: `{ yaml: string, force?: boolean }`
- Parses YAML server-side (the existing `POST /api/packs` accepts
  pre-parsed JSON; this endpoint accepts raw `pack.yaml` text so the
  admin doesn't need to YAML-parse client-side)
- Validates against `@aqa/schemas/PackManifest` (canonical Zod
  schema)
- On duplicate name without `force=true`, returns 409 with
  `code: 'EEXIST'` (reuses the structured-error pattern from PR #26)
- Adds `yaml` workspace dep to `@aqa/server`
- 8 new unit tests: happy-path 201, missing body.yaml, YAML parse
  error, schema-invalid manifest, 409 duplicate, force=true
  overwrite, non-boolean force rejected, permission requirement

## Admin UI (@aqa/admin)

- New `<ImportManifestWizard>` modal opened from the (previously
  silent) "Import manifest" button
- YAML textarea + native file picker for loading a `pack.yaml`
  from disk; selecting a file fills the textarea, submit still
  requires explicit click
- Force-overwrite checkbox surfaces the 409-retry path
- Result panel shows installed name + version + next-step guidance
  (matching pack directory still needs to exist on disk for
  `aqa run` to discover it)
- All errors render inline as `<Alert kind="error" role="alert">`
  (a11y carried over from slice 4a)
- 4 Playwright e2e tests: open/disabled-submit, happy-path 201,
  schema-validation 400 keeps wizard open, 409 → toggle force →
  retry succeeds

## Audit doc

- Lines 6850/6853 marked **DONE** (Install pack via PR #26 +
  Import manifest via this PR)
- Slice 4b checklist item marked **SHIPPED**

## Tests

- @aqa/server: 34 unit (was 26, +8 import tests)
- @aqa/admin: 55 Playwright (51 existing + 4 import wizard)
- Lint + typecheck clean

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an Admin “Import manifest” wizard and a corresponding server endpoint to import a pack by submitting a raw pack.yaml manifest. This extends the Packs page workflow introduced in slice 3 by wiring the previously-placeholder import path end-to-end.

Changes:

  • Server: add POST /api/packs/import that parses YAML, validates against @aqa/schemas/PackManifest, and installs via the store with structured error codes.
  • Admin: add <ImportManifestWizard> modal (paste YAML / load file, force overwrite retry path) and wire it to the Packs page button.
  • Tests/docs: add server unit tests + Playwright e2e coverage; update internal audit checklist and lockfiles.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/server/src/api.ts Implements POST /api/packs/import (YAML parse + schema validation + store install + error mapping).
packages/server/test/api.test.ts Adds unit tests for import endpoint success and error paths (400/409/force).
packages/server/package.json Adds yaml dependency for server-side parsing.
packages/admin/src/app.tsx Adds ImportManifestWizard UI and wires Packs page “Import manifest” button to open it.
packages/admin/test/e2e/import-manifest.e2e.ts New Playwright tests validating the wizard’s UX + request/response handling.
docs/internal/admin-placeholder-audit.md Marks slice 4b Packs page placeholders as shipped and documents coverage.
bun.lock Updates lockfile for the new yaml dependency.

Comment on lines +4731 to +4735
const text = await res.text();
let parsed = null;
try {
parsed = text ? JSON.parse(text) : null;
} catch {
Comment on lines +4713 to +4716
setYamlText(text);
} catch (err) {
const msg = err instanceof Error ? err.message : String(err);
setError(`Could not read file: ${msg}`);
…owing

PR #28 iter 1 review fixes (Copilot — 2 real items + CI typecheck):

## Workspace TS narrowing (CI)

`let posted: T | null = null` inside a `page.route` closure narrows
to `null` in the outer scope under the workspace TS config, so
`posted?.yaml` becomes a `never` property access. Same pattern + fix
as PR #27 iter 2: use a `const captured = { value: ... }` wrapper —
TS doesn't narrow object properties through closures. Applied to
both `happy-path` and `409 duplicate` tests.

## Real iter-1 fixes

1. **2xx + empty/non-JSON body treated as success** (Copilot
   app.tsx:4735): a 200 response with empty body left
   `parsed = null` (falsy), so `setResult(null)` kept the wizard
   in form-state while the success toast fired. The user saw a
   success notification but the modal didn't transition to the
   result panel — confusing.

   Now the wizard explicitly checks for the documented
   `{pack: {...}}` shape in the response. If it's missing
   (empty body, non-JSON, or unexpected shape) we treat it as an
   integration error and surface a clear message. New regression
   test: a 200 with empty body shows the error alert and does NOT
   show the result panel.

2. **File-input UX** (Copilot app.tsx:4716): after successfully
   reading a file into the textarea, the wizard didn't clear a
   stale "could not read file" error from a prior selection.
   Additionally, the `<input type="file">` value persisted, so
   re-selecting the same file didn't fire `onChange` in
   Chrome/Edge. Both fixed: `setError(null)` on success, and
   `fileInput.value = ''` in a `finally` block so re-selection
   works reliably.

Tests: @aqa/admin 56 Playwright (was 55, +1 for 2xx-without-pack).
Lint + typecheck clean (locally + workspace-wide).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lopadova lopadova requested a review from Copilot May 19, 2026 07:54
@lopadova
Copy link
Copy Markdown
Contributor Author

iter 2 on c8fede7: 2 real iter-1 items + workspace TS narrowing addressed — (1) 2xx with empty/non-JSON body now treated as integration failure with explicit error (regression test); (2) file input clears stale errors + resets value to allow re-selection of same file; (3) wrapper-object pattern for closure-mutated test state (same as PR #27). 56 e2e tests, lint+typecheck clean.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.

### Packs (lines ~6850-7040)

- ~~`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 `<CreatePackWizard>` component that POSTs to `/api/packs/scaffold`. "Import manifest" remains a placeholder for slice 4b.
- ~~`6850`, `6853` — top-bar "Import manifest" / "Install pack"~~ **DONE:** "Install pack" → "Create pack" wizard (slice 3, PR #26) wired to `POST /api/packs/scaffold`; "Import manifest" wired in slice 4b (PR #28) to a new `<ImportManifestWizard>` that POSTs to `POST /api/packs/import` — server parses YAML, validates against `@aqa/schemas/PackManifest`, installs via store. 4 Playwright e2e tests cover open/disabled, happy-path 201, schema-validation 400, 409-duplicate-with-force-retry path. 8 server unit tests cover the endpoint contract.
Comment on lines +462 to +466
const manifest = validated.data;
const existing = await ctx.store.loadPack(manifest.name);
if (existing && body.force !== true) {
return asResponse(
{
Comment thread packages/server/src/api.ts Outdated
Comment on lines +454 to +456
return asResponse(
{
error: `manifest failed schema validation: ${validated.error.message}`,
Comment on lines +4735 to +4739
const reqUrl = apiUrl('/api/packs/import');
try {
const res = await fetch(reqUrl, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
Comment thread packages/server/test/api.test.ts Outdated
Comment on lines +210 to +214
it('returns 400 on YAML that does not parse', async () => {
const c = ctx();
const route = makeApi().find((r) => r.method === 'POST' && r.path === '/api/packs/import');
const res = await route?.handle(
{ headers: {}, params: {}, body: { yaml: 'this is: not\n valid: [yaml' } },
Comment thread packages/server/test/api.test.ts Outdated
const yaml = `schema_version: "1"\nversion: 0.1.0\ndescription: missing name\nauthor: X\nlicense: Apache-2.0\napplies_when: { sut_type: [api] }\ntemplates: []\nscenarios: []\nrisks: []\noracles: []\nprobes: []\n`;
const res = await route?.handle({ headers: {}, params: {}, body: { yaml } }, c);
assert.equal(res?.status, 400);
assert.match((res?.body as { error: string }).error, /schema|name|required/i);
…pack, audit doc

PR #28 iter 2 review fixes (Copilot — 6 real new items + 2 stale re-flags):

## Real fixes

1. **Verbose Zod error.message** (Copilot api.ts:456): the
   schema-invalid response surfaced `validated.error.message`, which
   is a multi-line JSON-ish dump (often 5KB+) and unreadable in the
   admin's inline alert. New `formatZodError(err)` helper walks
   `err.issues` and produces a concise `path: message; path: message`
   list ("applies_when.sut_type: Required" etc). Falls back to a
   truncated `message` if `issues` is missing. Test asserts the
   error is not a multi-line dump.

2. **Test code-field assertions** (Copilot api.test.ts:214 + 228):
   the YAML-parse-failure and schema-invalid-manifest tests asserted
   only the status + error string, not the structured `code` field.
   Both now assert `code === 'EINVAL'` to lock the contract.

3. **CreatePackWizard didn't use apiUrl()** (Copilot app.tsx:4739):
   ImportManifestWizard correctly used `apiUrl(...)` to honor
   VITE_AQA_SERVER_URL, but CreatePackWizard still hard-coded the
   relative path. Now both pack wizards go through `apiUrl()`,
   keeping the deployment story consistent.

4. **`POST POST` in audit doc** (Copilot audit-doc:57): the wizard
   description double-prefixed "POSTs to `POST /api/packs/import`".
   Reworded to "calls `POST /api/packs/import`".

5. **Endpoint-consolidation note** (Copilot api.ts:466): the
   pre-existing `POST /api/packs` route doesn't validate or
   conflict-check (MemoryStore.installPack silently overwrites),
   while `/api/packs/import` does both. Consolidating onto a
   shared validate-then-install helper is the right architectural
   move, but it'd change long-standing behavior in the JSON route
   that callers may depend on — out of scope for this slice. Added
   an inline NOTE on the `POST /api/packs` handler tracking it as
   a v1.7.x follow-up and steering callers toward `/api/packs/import`
   for safety guarantees.

## Stale re-flags (no action)

`app.tsx:4746` (2xx-without-pack as error) and `app.tsx:4721`
(file-input clear-error + reset-value) were both addressed in
iter 2's c8fede7 push — Copilot's iter-2 review re-flagged
them based on stale line numbers.

Tests: @aqa/server 34 (unchanged count; 2 tests upgraded to assert
`code`). @aqa/admin 56 Playwright. Lint + typecheck clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lopadova lopadova requested a review from Copilot May 19, 2026 08:08
@lopadova
Copy link
Copy Markdown
Contributor Author

iter 3 on 866867e: 6 real iter-2 items addressed — (1) Zod errors now formatted as concise path: message list via formatZodError helper; (2+3) 2 tests upgraded to assert code === 'EINVAL'; (4) CreatePackWizard now uses apiUrl() for parity with ImportManifestWizard; (5) audit-doc POST POST typo fixed; (6) inline NOTE on POST /api/packs documenting consolidation follow-up.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.

@lopadova lopadova merged commit 94cac3d into main May 19, 2026
16 checks passed
@lopadova lopadova deleted the task/v1.7-slice-4b-import-manifest branch May 19, 2026 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants