command and help ux pass
This commit is contained in:
@@ -0,0 +1,346 @@
|
||||
# Crucible Command Surface Cleanup Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use
|
||||
> `superpowers:subagent-driven-development` or
|
||||
> `superpowers:executing-plans` to implement this plan task-by-task. Steps use
|
||||
> checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Replace repetitive Crucible command names with a descriptive,
|
||||
registry-driven public surface, retain long names as hidden aliases, and remove
|
||||
the unused music conversion pipeline without affecting authored `.bmu` assets.
|
||||
|
||||
**Architecture:** `internal/dispatch.Registry` gains explicit command records
|
||||
that drive lookup, help, and the interactive menu while translating to unchanged
|
||||
`internal/app` implementation names. The unused music subsystem is removed
|
||||
through its app, pipeline, project-config, validator, packaging, and consumer
|
||||
boundaries. The two active `sow-assets-manifest` call sites are decoupled in the
|
||||
same worktree session.
|
||||
|
||||
**Tech Stack:** Go 1.26, Go tests, Nix flakes, Dockerfile, Bash/Bats.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- Never commit or push.
|
||||
- Do not touch secrets or `.sops` data.
|
||||
- Keep `internal/dispatch.Registry` synchronized with
|
||||
`docs/command-surface.md`.
|
||||
- Preserve deterministic artifact behavior.
|
||||
- Preserve every existing long command as a hidden alias except all music
|
||||
commands, which are removed.
|
||||
- Preserve `.bmu` as an ordinary HAK asset.
|
||||
- Do not retain generated binaries or build output.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Registry-driven canonical commands and hidden aliases
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `internal/dispatch/dispatch.go`
|
||||
- Modify: `internal/dispatch/dispatch_test.go`
|
||||
|
||||
**Interfaces:**
|
||||
|
||||
- Produces a `Command` record owned by each `Builder`.
|
||||
- Produces lookup that returns the command record and exact app target.
|
||||
- Preserves `Builder.Legacy` only if required by migration tests; otherwise
|
||||
replaces it with command-derived compatibility metadata.
|
||||
|
||||
- [ ] Add failing tests covering the visible command table:
|
||||
`hak build/manifest`, `module build/extract/validate/compare/manifest`,
|
||||
`topdata validate/build/package/compare/convert`, and `wiki build/deploy`.
|
||||
- [ ] Add failing tests proving long aliases resolve to the same app target and
|
||||
that `module build-module` resolves to `build-module`, not `build`.
|
||||
- [ ] Add a failing test proving all music commands are rejected.
|
||||
- [ ] Run:
|
||||
|
||||
```bash
|
||||
nix develop --command go test ./internal/dispatch -run 'TestCanonical|TestHidden|TestMusic' -v
|
||||
```
|
||||
|
||||
Expected: failures caused by the old registry shape and exposed music
|
||||
commands.
|
||||
|
||||
- [ ] Introduce command records with fields equivalent to:
|
||||
|
||||
```go
|
||||
type Command struct {
|
||||
Name string
|
||||
Summary string
|
||||
AppCommand string
|
||||
Usage string
|
||||
Options []string
|
||||
HiddenAlias []string
|
||||
}
|
||||
```
|
||||
|
||||
Use the minimum final field names that keep lookup, menu, and help readable.
|
||||
|
||||
- [ ] Replace builder-level subcommand acceptance with command lookup. Translate
|
||||
canonical and hidden aliases to `AppCommand` before calling `app.Run`.
|
||||
- [ ] Define the approved command table and remove dispatcher music entries.
|
||||
- [ ] Re-run the focused dispatch tests and the full dispatch package:
|
||||
|
||||
```bash
|
||||
nix develop --command go test ./internal/dispatch -v
|
||||
```
|
||||
|
||||
Expected: pass.
|
||||
|
||||
### Task 2: Descriptive menu and command-specific help
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `internal/menu/menu.go`
|
||||
- Modify: `internal/menu/menu_test.go`
|
||||
- Modify: `internal/dispatch/dispatch.go`
|
||||
- Modify: `internal/dispatch/dispatch_test.go`
|
||||
|
||||
**Interfaces:**
|
||||
|
||||
- `menu.Item` carries display label, summary, dispatcher args, usage, and option
|
||||
lines.
|
||||
- `menu.Select` returns selected dispatcher args plus entered arguments.
|
||||
- Dispatcher command help uses the same registry guidance as the menu.
|
||||
|
||||
- [ ] Add failing menu tests proving descriptions start at one aligned column
|
||||
for labels of different lengths and hidden aliases never enter `menuItems()`.
|
||||
- [ ] Add a failing test selecting a command and asserting output contains its
|
||||
usage, option guidance, and
|
||||
`arguments (press Enter to use defaults):`.
|
||||
- [ ] Add failing dispatcher tests proving
|
||||
`crucible topdata build --help` returns `0`, prints canonical usage/options,
|
||||
and does not delegate into project loading.
|
||||
- [ ] Run:
|
||||
|
||||
```bash
|
||||
nix develop --command go test ./internal/menu ./internal/dispatch -run 'Test.*Menu|Test.*Help' -v
|
||||
```
|
||||
|
||||
Expected: fail against the current fixed-width menu and builder-only help.
|
||||
|
||||
- [ ] Compute menu label width from visible items and render descriptions with
|
||||
that width.
|
||||
- [ ] Print command guidance immediately after a selection and before reading
|
||||
optional arguments.
|
||||
- [ ] Add command-level help handling before app delegation.
|
||||
- [ ] Ensure builder help lists only canonical names and their summaries.
|
||||
- [ ] Run:
|
||||
|
||||
```bash
|
||||
nix develop --command go test ./internal/menu ./internal/dispatch -v
|
||||
```
|
||||
|
||||
Expected: pass.
|
||||
|
||||
### Task 3: Remove music from HAK app and pipeline
|
||||
|
||||
**Files:**
|
||||
|
||||
- Delete: `internal/pipeline/music.go`
|
||||
- Modify: `internal/pipeline/build.go`
|
||||
- Modify: `internal/pipeline/pipeline_test.go`
|
||||
- Modify: `internal/app/app.go`
|
||||
- Modify: `internal/app/app_test.go`
|
||||
- Delete: `internal/music/config.go`
|
||||
- Delete: `internal/music/credits.go`
|
||||
- Delete: `internal/music/metadata.go`
|
||||
- Delete: `internal/music/music.go`
|
||||
- Delete: `internal/music/music_test.go`
|
||||
- Delete: `internal/music/naming.go`
|
||||
|
||||
**Interfaces:**
|
||||
|
||||
- `pipeline.BuildHAKOptions` retains only HAK selection, source-manifest,
|
||||
content-addressed-root, and progress controls.
|
||||
- `pipeline.BuildResult` retains artifact fields unrelated to generated music
|
||||
credits.
|
||||
- `build-haks` accepts no music-specific flags.
|
||||
|
||||
- [ ] Replace music pipeline tests with a failing contract test that builds a
|
||||
HAK containing a pre-authored `.bmu` and proves the file appears in the
|
||||
manifest/archive without conversion setup.
|
||||
- [ ] Add failing parser tests proving `--skip-music` and `--music-dataset`
|
||||
return unknown-argument errors and help does not mention them.
|
||||
- [ ] Run:
|
||||
|
||||
```bash
|
||||
nix develop --command go test ./internal/app ./internal/pipeline -run 'Test.*Music|Test.*BMU|TestParseBuildHAK' -v
|
||||
```
|
||||
|
||||
Expected: at least the removed-flag assertions fail.
|
||||
|
||||
- [ ] Remove the app `music` command, argument parser, emitters, music HAK
|
||||
options, and credits console output.
|
||||
- [ ] Remove music fields from `BuildHAKOptions` and `BuildResult`.
|
||||
- [ ] Remove preparation/cleanup calls from HAK builds so asset collection reads
|
||||
authored resources directly.
|
||||
- [ ] Delete the music implementation packages and obsolete conversion tests.
|
||||
- [ ] Keep `.bmu` handling in ordinary ERF/resource and asset collection paths.
|
||||
- [ ] Run:
|
||||
|
||||
```bash
|
||||
nix develop --command go test ./internal/app ./internal/pipeline -v
|
||||
```
|
||||
|
||||
Expected: pass.
|
||||
|
||||
### Task 4: Remove music configuration and validation
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `internal/project/project.go`
|
||||
- Modify: `internal/project/effective.go`
|
||||
- Modify: `internal/project/project_test.go`
|
||||
- Modify: `internal/validator/validator.go`
|
||||
- Modify: `internal/validator/validator_test.go`
|
||||
|
||||
**Interfaces:**
|
||||
|
||||
- `project.Config` and `project.EffectiveConfig` contain no music section.
|
||||
- Project scanning uses only configured inventory extensions.
|
||||
- `.bmu` and `.wav` remain default asset extensions; `.mp3` and `.ogg` do not.
|
||||
|
||||
- [ ] Add or adjust failing project tests asserting default extensions include
|
||||
`.bmu` and `.wav` but exclude `.mp3` and `.ogg`.
|
||||
- [ ] Add a failing strict-decoder test proving a top-level `music:` field is
|
||||
rejected as unknown.
|
||||
- [ ] Run:
|
||||
|
||||
```bash
|
||||
nix develop --command go test ./internal/project ./internal/validator -run 'Test.*Music|Test.*AssetExtension|Test.*BMU' -v
|
||||
```
|
||||
|
||||
Expected: fail while music config is still accepted.
|
||||
|
||||
- [ ] Remove music config types, defaults, normalization, validation, accessors,
|
||||
effective config, provenance defaults, and environment overrides.
|
||||
- [ ] Simplify `Project.Scan` to accept assets solely from
|
||||
`effective.Inventory.AssetExtensions`.
|
||||
- [ ] Remove validator logic that exempts conversion source files.
|
||||
- [ ] Remove conversion-only `.mp3` and `.ogg` from default asset extensions;
|
||||
retain `.bmu` and `.wav`.
|
||||
- [ ] Remove obsolete music-focused project and validator tests.
|
||||
- [ ] Run:
|
||||
|
||||
```bash
|
||||
nix develop --command go test ./internal/project ./internal/validator -v
|
||||
```
|
||||
|
||||
Expected: pass.
|
||||
|
||||
### Task 5: Remove ffmpeg packaging and stale documentation
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `flake.nix`
|
||||
- Modify: `docker/Dockerfile`
|
||||
- Modify: `wrappers/crucible.sh`
|
||||
- Modify: `README.md`
|
||||
- Modify: `AGENTS.md`
|
||||
- Modify: `docs/command-surface.md`
|
||||
- Modify: `docs/migration-from-nwn-tool.md`
|
||||
|
||||
**Interfaces:**
|
||||
|
||||
- Nix development shell and image no longer contain ffmpeg.
|
||||
- Docker runtime contains only certificates and the non-root runtime support
|
||||
needed by current builders.
|
||||
- User docs advertise canonical short commands.
|
||||
|
||||
- [ ] Add a shell verification that fails while active packaging still mentions
|
||||
ffmpeg or removed command names:
|
||||
|
||||
```bash
|
||||
! rg -n 'ffmpeg|ffprobe|skip-music|music-dataset|module music|hak music' \
|
||||
flake.nix docker wrappers README.md AGENTS.md docs/command-surface.md
|
||||
```
|
||||
|
||||
- [ ] Remove `pkgs.ffmpeg-headless` and `ffmpeg` from Nix image/dev-shell
|
||||
contents and update comments.
|
||||
- [ ] Return the Docker runtime to a minimal base without ffmpeg while retaining
|
||||
CA certificates and non-root execution.
|
||||
- [ ] Remove the wrapper's `ffmpeg-free` wording.
|
||||
- [ ] Rewrite command-surface documentation around canonical names and a
|
||||
concise hidden-alias compatibility section.
|
||||
- [ ] Update README examples and ownership/status prose.
|
||||
- [ ] Re-run the search. Expected: no matches in active files.
|
||||
|
||||
### Task 6: Decouple active consumer scripts
|
||||
|
||||
**Files in `../sow-assets-manifest`:**
|
||||
|
||||
- Modify: `scripts/pack-haks.sh`
|
||||
- Modify: `scripts/build-local-haks.sh`
|
||||
- Modify: `tests/pack-haks.bats`
|
||||
- Modify only directly related active comments/tests if a focused test proves
|
||||
they require adjustment.
|
||||
|
||||
**Interfaces:**
|
||||
|
||||
- Both scripts call the hidden-compatible `hak build-haks` or canonical
|
||||
`hak build` without `--skip-music`.
|
||||
|
||||
- [ ] Read `../sow-assets-manifest/AGENTS.md` before editing.
|
||||
- [ ] Add or adjust a focused test to reject `--skip-music` in generated
|
||||
Crucible invocations.
|
||||
- [ ] Run the focused test and confirm it fails while the flag is present.
|
||||
- [ ] Remove `--skip-music` and its lockstep comments from both scripts.
|
||||
- [ ] Update the HAK parser test invocation to omit the removed flag.
|
||||
- [ ] Run:
|
||||
|
||||
```bash
|
||||
cd ../sow-assets-manifest
|
||||
nix develop --command bats tests/pack-haks.bats
|
||||
```
|
||||
|
||||
Expected: pass.
|
||||
|
||||
### Task 7: Full verification and scratchpad cleanup
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `../SCRATCHPAD.md`
|
||||
- Do not retain: `bin/`, `.cache/go-build/`, result symlinks, image tarballs, or
|
||||
other generated outputs.
|
||||
|
||||
- [ ] Run formatting:
|
||||
|
||||
```bash
|
||||
nix develop --command gofmt -w internal/app internal/dispatch internal/menu internal/pipeline internal/project internal/validator
|
||||
```
|
||||
|
||||
- [ ] Run full repository checks:
|
||||
|
||||
```bash
|
||||
nix develop --command make check
|
||||
make build
|
||||
make smoke
|
||||
```
|
||||
|
||||
- [ ] Run focused consumer verification from Task 6.
|
||||
- [ ] Verify deleted pipeline identifiers are absent from active code:
|
||||
|
||||
```bash
|
||||
rg -n 'BuildMusic|CreditsSummary|MusicDataset|skip-music|music-dataset|ffmpeg|ffprobe' \
|
||||
--glob '!docs/superpowers/**' .
|
||||
```
|
||||
|
||||
Expected: no matches.
|
||||
|
||||
- [ ] Verify active command docs/examples do not advertise long names:
|
||||
|
||||
```bash
|
||||
rg -n 'topdata (validate-topdata|build-topdata|build-top-package|compare-topdata|convert-topdata)|wiki (build-wiki|deploy-wiki)|hak (build-haks|apply-hak-manifest)' \
|
||||
README.md docs/command-surface.md
|
||||
```
|
||||
|
||||
Expected: matches only inside the explicitly labeled hidden-alias
|
||||
compatibility section.
|
||||
|
||||
- [ ] Inspect `git diff --check`, `git status --short`, and both repository
|
||||
diffs. Remove only generated files created by this work.
|
||||
- [ ] Remove the completed “Simplify crucible commands” to-do and the full
|
||||
“Crucible commands issue” section from `../SCRATCHPAD.md`; do not archive it.
|
||||
- [ ] Record exact verification commands and outcomes in the final handoff.
|
||||
|
||||
@@ -0,0 +1,147 @@
|
||||
# Crucible Command Surface Cleanup Design
|
||||
|
||||
**Date:** 2026-06-25
|
||||
**Status:** Approved
|
||||
**Scope:** `sow-tools`, with the two active `sow-assets-manifest` call sites
|
||||
updated in lockstep
|
||||
|
||||
## Problem
|
||||
|
||||
Crucible exposes migrated `nwn-tool` implementation names directly. Several
|
||||
public commands repeat their builder name (`topdata build-topdata`,
|
||||
`wiki build-wiki`), the interactive menu repeats builder-level descriptions
|
||||
instead of explaining each action, and selected commands only prompt for
|
||||
unexplained "extra args".
|
||||
|
||||
The registry also exposes a migrated music-conversion subsystem that is not part
|
||||
of the current project workflow. Live HAK builds explicitly bypass it with
|
||||
`--skip-music`; authored `.bmu` files are already packed as ordinary assets.
|
||||
|
||||
## Decisions
|
||||
|
||||
### Canonical command names
|
||||
|
||||
The visible command surface is:
|
||||
|
||||
| Builder | Visible commands |
|
||||
| --- | --- |
|
||||
| `hak` | `build`, `manifest` |
|
||||
| `module` | `build`, `extract`, `validate`, `compare`, `manifest` |
|
||||
| `topdata` | `validate`, `build`, `package`, `compare`, `convert` |
|
||||
| `wiki` | `build`, `deploy` |
|
||||
|
||||
The dispatcher translates these names to the existing `internal/app`
|
||||
implementation commands. The app implementation names do not need to be
|
||||
renamed.
|
||||
|
||||
### Hidden compatibility aliases
|
||||
|
||||
Existing long names remain callable indefinitely but are omitted from the
|
||||
interactive menu and routine builder help:
|
||||
|
||||
- `hak build-haks`
|
||||
- `hak apply-hak-manifest`
|
||||
- `module build-module`
|
||||
- `module apply-hak-manifest`
|
||||
- `topdata validate-topdata`
|
||||
- `topdata build-topdata`
|
||||
- `topdata build-top-package`
|
||||
- `topdata compare-topdata`
|
||||
- `topdata convert-topdata`
|
||||
- `wiki build-wiki`
|
||||
- `wiki deploy-wiki`
|
||||
|
||||
Aliases preserve their existing implementation target. In particular,
|
||||
`module build-module` continues to run the module-only implementation; it must
|
||||
not silently become the broader `module build`.
|
||||
|
||||
### Registry model
|
||||
|
||||
`internal/dispatch.Registry` remains the single source of truth. Each wired
|
||||
builder owns command records containing:
|
||||
|
||||
- visible command name;
|
||||
- action-oriented summary;
|
||||
- underlying `internal/app` command;
|
||||
- command usage and option guidance;
|
||||
- hidden compatibility aliases, where applicable.
|
||||
|
||||
Execution, menu items, and builder help are generated from these records.
|
||||
Unknown commands continue to fail with exit `64`.
|
||||
|
||||
### Interactive behavior
|
||||
|
||||
The main menu:
|
||||
|
||||
- lists only visible commands;
|
||||
- aligns the description column from the longest visible label;
|
||||
- shows a distinct description for every command.
|
||||
|
||||
After selection, it prints the selected command's usage and available options,
|
||||
then prompts:
|
||||
|
||||
```text
|
||||
arguments (press Enter to use defaults):
|
||||
```
|
||||
|
||||
Arguments continue to use shell-like whitespace splitting, matching current
|
||||
behavior. Quoted argument parsing is out of scope.
|
||||
|
||||
`crucible <builder> <command> --help` prints the same command-specific guidance,
|
||||
returns success, and performs no project loading or build work.
|
||||
|
||||
### Remove the unused music pipeline
|
||||
|
||||
The music conversion subsystem is deleted rather than hidden:
|
||||
|
||||
- remove the `music` app and dispatcher commands;
|
||||
- remove `internal/music` and `internal/pipeline/music.go`;
|
||||
- remove music config, effective config, validation, environment overrides, and
|
||||
project accessors;
|
||||
- remove HAK music preparation, conversion, credits, generated-manifest result
|
||||
fields, `--skip-music`, and `--music-dataset`;
|
||||
- remove ffmpeg/ffprobe runtime and development dependencies;
|
||||
- remove music-pipeline documentation and wrapper wording.
|
||||
|
||||
`.bmu` remains a supported ordinary asset extension and continues through the
|
||||
same content-addressed HAK packing path as other authored files. Actual game
|
||||
content, category names, or prose that happens to use the word "music" is not
|
||||
part of this deletion.
|
||||
|
||||
The default asset-extension list drops conversion-only `.mp3` and `.ogg`.
|
||||
`.wav` remains because it is an NWN resource type independent of the deleted
|
||||
conversion pipeline.
|
||||
|
||||
Because `sow-assets-manifest` currently passes `--skip-music`, its
|
||||
`scripts/pack-haks.sh` and `scripts/build-local-haks.sh` call sites are updated
|
||||
in the same change before verification.
|
||||
|
||||
## Error handling
|
||||
|
||||
- Canonical and hidden alias lookup is deterministic within one builder.
|
||||
- Registry tests reject duplicate visible names or aliases.
|
||||
- Command help never delegates to `internal/app`.
|
||||
- Removed music commands and flags fail as unknown usage.
|
||||
- Existing project files containing a top-level `music:` configuration become
|
||||
invalid under strict YAML decoding. No active consumer configuration contains
|
||||
that field.
|
||||
|
||||
## Testing
|
||||
|
||||
Tests assert public contracts rather than exact incidental formatting:
|
||||
|
||||
- visible command names and hidden alias translation;
|
||||
- preservation of the distinct `module build-module` target;
|
||||
- hidden aliases absent from menu and normal help;
|
||||
- command-specific help exits successfully without project setup;
|
||||
- menu columns align and selected-command guidance is displayed;
|
||||
- `.bmu` remains accepted as an asset;
|
||||
- `.mp3`/`.ogg` are no longer default packable assets;
|
||||
- HAK builds no longer expose or execute music preparation;
|
||||
- ffmpeg is absent from package/image definitions;
|
||||
- both consumer scripts no longer pass `--skip-music`.
|
||||
|
||||
Final verification is `nix develop --command make check`, `make build`,
|
||||
`make smoke`, focused consumer tests, and repository-wide searches for deleted
|
||||
pipeline identifiers.
|
||||
|
||||
Reference in New Issue
Block a user