87 lines
5.8 KiB
Markdown
87 lines
5.8 KiB
Markdown
# Issue #804 — fail closed on unknown installer arguments
|
|
|
|
## Objective
|
|
|
|
Implement Part 1 of Gitea issue #804 only: `tools/install.sh` must reject every unrecognized flag or argument with an actionable STDERR error and nonzero exit before installation starts.
|
|
|
|
## Scope and constraints
|
|
|
|
- Preserve all currently recognized options and behavior, including `-y` and `--ref <branch>`.
|
|
- No positional arguments are currently accepted by the parser.
|
|
- Do not add `--next`, `MOSAIC_NEXT`, prerelease routing, or any Part 2 behavior.
|
|
- TDD is mandatory: add and observe a failing process-level regression test before changing `tools/install.sh`.
|
|
- Worker lifecycle ends after branch push, PR creation, and coordinator notification; do not merge or close #804.
|
|
- Existing launcher-owned changes in `.mosaic/orchestrator/mission.json` and `.mosaic/orchestrator/session.lock` are out of scope and must not be committed.
|
|
|
|
## Requirements and acceptance criteria
|
|
|
|
- Unknown input names the offending argument on STDERR.
|
|
- STDERR includes a short installer usage hint.
|
|
- Exit status is nonzero.
|
|
- The installer does not invoke npm or otherwise proceed into installation.
|
|
- Existing recognized flags remain unchanged.
|
|
|
|
## Plan
|
|
|
|
1. Add a process-level Vitest regression using the installer test location under `packages/mosaic/src/commands/`.
|
|
2. Run the focused test and record the expected RED failure.
|
|
3. Commit the RED test as `test(#804): ...`.
|
|
4. Replace the parser catch-all with a fail-closed STDERR error and usage hint.
|
|
5. Update concise installer-facing documentation without introducing prerelease behavior.
|
|
6. Run focused tests, shell syntax validation, package tests, lint, typecheck, and format checks.
|
|
7. Run independent review tooling and remediate findings.
|
|
8. Commit as `fix(#804): ...`, queue-guard, push, open a PR containing `Closes #804.`, notify the coordinator, and exit.
|
|
|
|
## Budget
|
|
|
|
- No explicit token cap supplied.
|
|
- Working estimate: 8K tokens; narrow two-file behavior/test change plus concise docs and delivery gates.
|
|
|
|
## Progress
|
|
|
|
- 2026-07-17: Loaded mission state, issue #804, delivery/QA/documentation rails, and relevant TDD/Vitest/pnpm/Gitea skills.
|
|
- 2026-07-17: Confirmed the parser has no legitimate positional arguments and currently drops all unmatched input via `*) shift ;;`.
|
|
- 2026-07-17: Installed locked workspace dependencies with a worktree-local pnpm store; no lockfile changes.
|
|
- 2026-07-17: Added the process-level unknown-argument regression with an isolated `$HOME` and npm shim.
|
|
- 2026-07-17: Replaced the silent catch-all with STDERR error + usage output and exit 2 before preflight or installation.
|
|
- 2026-07-17: Initial Codex code review found an unknown option could still be consumed as the `--ref` value. Added a second RED reproducer, then rejected option-shaped/missing `--ref` values without changing valid `--ref <branch>` behavior. The review's launcher-state note is handled by excluding both `.mosaic/orchestrator/` files from commits.
|
|
- 2026-07-17: Updated README, user guide, and packaged framework README with the fail-closed argument contract. No API, auth, admin, sitemap/navigation, or publishing surface changed.
|
|
|
|
## Verification
|
|
|
|
- RED: `pnpm --filter @mosaicstack/mosaic exec vitest run src/commands/install-arguments.spec.ts` — expected failure: installer exited `0` instead of nonzero at the exit-status assertion; confirms the test reproduces the silent-drop defect before production changes.
|
|
- Remediation RED: the added `--cli --ref --bogus` case exited `0`, proving `--ref` could swallow an unknown option before the guard was added.
|
|
- GREEN: `pnpm --filter @mosaicstack/mosaic exec vitest run src/commands/install-arguments.spec.ts src/commands/install-heading.spec.ts` — 2 files, 3 tests passed.
|
|
- Situational process check: unknown positional input exited 2, named the input on STDERR, printed usage, and did not call the npm shim.
|
|
- `bash -n tools/install.sh` — passed.
|
|
- Bare `--ref` process check — exited 2 with `Missing value for --ref` and usage.
|
|
- `pnpm --filter @mosaicstack/mosaic test` — 69 files, 1,287 tests passed; framework shell checks passed. The first attempt lacked generated `dist/cli.js`; `pnpm --filter @mosaicstack/mosaic build` restored the required test precondition and the full rerun passed.
|
|
- `pnpm lint` — 23/23 tasks passed.
|
|
- `pnpm typecheck` — 42/42 tasks passed.
|
|
- `pnpm format:check` — passed.
|
|
- Codex code re-review against `origin/main` — `approve`, 0 blockers/should-fix/suggestions.
|
|
- Codex security re-review against `origin/main` — risk `none`, 0 findings.
|
|
|
|
## Acceptance evidence
|
|
|
|
| Criterion | Evidence |
|
|
| --- | --- |
|
|
| Unknown input is named on STDERR | Process-level Vitest assertions for `--bogus`, including after `--ref` |
|
|
| Short usage hint is printed on STDERR | Vitest usage regex + manual process output |
|
|
| Exit is nonzero | Vitest status assertions and manual exit 2 |
|
|
| Installation does not proceed | Isolated npm shim marker remains absent |
|
|
| Recognized behavior is preserved | Parser cases are unchanged except validation of malformed `--ref`; full Mosaic package suite passed |
|
|
| Part 2 is excluded | No `--next`, `MOSAIC_NEXT`, dist-tag, or prerelease routing changes |
|
|
|
|
## Documentation checklist
|
|
|
|
- Current canonical `docs/PRD.md` remains unchanged; issue #804 and the coordinator brief supply this bounded defect's acceptance contract.
|
|
- Updated installer behavior in root README, user guide, and packaged framework README in the same logical change set.
|
|
- API/OpenAPI, auth/permissions, admin operations, developer architecture, sitemap/navigation, and external publishing are not affected.
|
|
- Scratchpad remains under `docs/scratchpads/`; no root-hygiene changes.
|
|
|
|
## Risks and blockers
|
|
|
|
- Part 2 remains owner-gated under #805 and is intentionally excluded.
|
|
- No implementation blocker remains. Independent coordinator RoR, CI, merge, and issue closure remain pending after worker handoff.
|