DRAFT: adds ux bits, spec review skill

[markturansky]

No description provided.

[jsell-rh]

🤖 Amber Analysis — PR #351

This is a DRAFT, so I'm reviewing with advisory intent rather than blocking. Overall quality is high — the spec authoring standards are well-structured, the skill designs are solid, and the runner gap analysis is genuinely useful (especially linking runtime pod failure logs to untested code paths).

CI: All relevant gates passing (Lint, Build, Unit Tests, CodeQL, SDD boundary, lab-markdown-e2e). Clean.

Confidence: 90%

---

What's here

File	Change
Makefile	New kind-stop-openshell-cli target
scripts/setup-gateway-cli.sh	Refactors blocking wait → detached PID-file model
skills/review/spec-gap-analysis/	New skill + evals
skills/review/spec-review/	New skill + evals
specs/standards/specs/specs.spec.md	New spec authoring standards
specs/index.spec.md	Registry entry for above
specs/platform/runner.spec-gaps.md	Gap analysis output (exemplar run)

Title says "ux bits" — I see none in the diff. I assume those changes are still pending in this DRAFT.

---

Issues

1. Minor · skills/review/spec-review/evals/evals.json — inconsistent NL eval args

Eval #2 ("review the new credential rotation spec for standards compliance") passes the full natural-language string as args. The spec-gap-analysis evals correctly resolve NL inputs to file paths (e.g., "what gaps exist in the runner spec?" → args: "specs/platform/runner.spec.md"). spec-review should do the same — if no path is inferrable, use "" or null so the skill triggers its "ask user which spec" branch rather than receiving an unparseable path.

2. Minor · scripts/setup-gateway-cli.sh — SIGINT during multi-namespace setup leaves orphans

The old trap cleanup EXIT ensured port-forwards started before a Ctrl-C were killed. Now, if the script is interrupted mid-loop (e.g., between two namespaces), earlier port-forwards have their PIDs written but the signal was delivered before completing the loop — only make kind-stop-openshell-cli will clean them up. This is fine as the intended behavior, but worth a # Note: interrupt during setup leaves partial port-forwards; use make kind-stop-openshell-cli to clean up comment near the loop.

3. Minor · Commit message not conventional

adds ux bits, spec review skill → should be feat(skills): add spec-review, spec-gap-analysis skills and openshell gateway port-forward persistence. Squash before merge.

---

What's good

• setup-gateway-cli.sh refactor is clean. KIND_PF_DIR in Makefile already points to /tmp/ambient-code, so the PID files are co-located correctly. The kind-stop-openshell-cli target correctly extracts the namespace from the filename pattern.
• specs/standards/specs/specs.spec.md is excellent. RFC 2119 keyword table, the "quick test" for implementation-detail smell, and the reconcilability validation section (R1–R7) are exactly what agents need to evaluate specs reliably. The G/T/D/E gap taxonomy is clear and maps well to the skill output.
• specs/platform/runner.spec-gaps.md is a model output. Anchoring R1–R3 to actual pod failure timestamps (2026-07-11T11:11:45Z) shows this wasn't a mechanical scan — it's operationally grounded. G1 (zero tests on AGUI_TOKEN auth middleware) is a real CRITICAL that should become a follow-up issue.
• spec-gap-analysis skill phases are well-ordered. Parse → search → classify → propose tests → write report is exactly the right sequence.

---

— Amber

[jsell-rh]

The args here is the full NL sentence, not a resolved spec path. Compare to spec-gap-analysis eval #2 where "what gaps exist in the runner spec?" correctly maps to args: "specs/platform/runner.spec.md".

For this natural-language case where no spec path is inferable, prefer args: "" (or null) so the skill enters its "ask user which spec" branch. Passing the full sentence as args would cause the skill to try to Read a non-existent path.

— Amber

[jsell-rh]

Adding amber/approved — no blockers. Advisory items noted in review comments above.

— Amber