docs(19.8): gap-closure plans for the code review findings and v0.4.2
This commit is contained in:
Jakub Zych
@@ -1470,7 +1470,7 @@ Plans:
|
||||
4. A sweep of all existing commands (framework modules and shared plugins) checks them against an Artisan feature checklist and fixes the gaps found, without changing command logic or breaking existing names and flags. The checklist covers confirmation with `--force` on destructive actions, `--pretend`/dry-run where it applies, progress bars and tables for bulk work, and `--json` output for scripting. Long-running commands such as `serve` and `queue:work` print a startup summary of what booted (environment, plugins, database and migrations state, queues, scheduler, mail, cache and realtime drivers, URLs) and report what they did on shutdown. One-shot commands end with a result line and a meaningful exit code.
|
||||
5. The `diary` and `bonfire` READMEs, the docs console and logging pages, the summercms.io landing site and its roadmap presentation reflect the new console. The new code has unit tests, delivered in the phase's last plan.
|
||||
|
||||
**Plans:** 4/4 plans executed
|
||||
**Plans:** 4/6 plans executed
|
||||
|
||||
Plans:
|
||||
**Wave 1**
|
||||
@@ -1484,6 +1484,8 @@ Plans:
|
||||
|
||||
**Wave 4** *(blocked on Wave 3 completion)*
|
||||
- [x] 19.8-04-PLAN.md — unit and golden tests for all new code, `scripts/check-phase19.8.sh` gate across repos, post-release fixes go-ahead
|
||||
- [ ] 19.8-05-PLAN.md — gap closure: fixes for the code review's critical and warning findings (live-view output order, summer delegates' exit codes and terminal, Ctrl+C cursor, dev shutdown grace, diary file retry, escape stripping, migration sets), each with a pinning test
|
||||
- [ ] 19.8-06-PLAN.md — gap closure: v0.4.2 release go-ahead, tag and changelog, demo roadmap sync, full gate
|
||||
|
||||
### Phase 20: Demo instance with every shared plugin
|
||||
|
||||
@@ -1588,7 +1590,7 @@ Phases execute in numeric order: 1 → 2 → 3 → 4 → 5 → 6 → 7 → 8 →
|
||||
| 19.6. Two-factor auth and device flow in sm-user-plugin | 0/TBD | Not started | - |
|
||||
| 19.6.1. Backend Administrators screen (INSERTED) | 0/TBD | Not started | - |
|
||||
| 19.7. Generic MCP module | 0/TBD | Not started | - |
|
||||
| 19.8. Console commands overhaul (INSERTED) | 4/4 | In Progress| |
|
||||
| 19.8. Console commands overhaul (INSERTED) | 4/6 | In Progress| |
|
||||
| 20. Demo instance with every shared plugin | 0/TBD | Not started | - |
|
||||
| 30. Płytarium cutover | 0/TBD | Not started | - |
|
||||
| 31. Chat polish against WavePath clients | 0/TBD | Not started | - |
|
||||
|
||||
456
.planning/phases/19.8-console-commands-overhaul/19.8-05-PLAN.md
Normal file
456
.planning/phases/19.8-console-commands-overhaul/19.8-05-PLAN.md
Normal file
@@ -0,0 +1,456 @@
|
||||
---
|
||||
phase: 19.8-console-commands-overhaul
|
||||
plan: 05
|
||||
type: execute
|
||||
wave: 1
|
||||
depends_on: []
|
||||
gap_closure: true
|
||||
files_modified:
|
||||
- modules/bonfire/live.go
|
||||
- modules/bonfire/output.go
|
||||
- modules/bonfire/widgets.go
|
||||
- modules/bonfire/helpers.go
|
||||
- modules/bonfire/errors.go
|
||||
- modules/bonfire/execute.go
|
||||
- modules/bonfire/README.md
|
||||
- modules/bonfire/live_test.go
|
||||
- modules/bonfire/live_signal_unix_test.go
|
||||
- modules/bonfire/widgets_test.go
|
||||
- modules/bonfire/errors_test.go
|
||||
- modules/diary/daily.go
|
||||
- modules/diary/diary.go
|
||||
- modules/diary/console.go
|
||||
- modules/diary/README.md
|
||||
- modules/diary/daily_test.go
|
||||
- modules/diary/diary_test.go
|
||||
- modules/lagoon/migrations.go
|
||||
- modules/lagoon/commands.go
|
||||
- modules/lagoon/README.md
|
||||
- modules/lagoon/pending_test.go
|
||||
- cmd/summer/runtime.go
|
||||
- cmd/summer/runtime_test.go
|
||||
- internal/dev/watch.go
|
||||
- internal/dev/watch_test.go
|
||||
- docs/console/introduction.md
|
||||
- docs/console/writing-commands.md
|
||||
- docs/console/scaffolding.md
|
||||
- docs/services/logging.md
|
||||
- docs/setup/upgrading.md
|
||||
- scripts/check-phase19.8.sh
|
||||
- .planning/phases/19.8-console-commands-overhaul/19.8-REVIEW-DISPOSITION.md
|
||||
- .planning/todos/pending/2026-10-22-console-review-info-findings.md
|
||||
- .planning/phases/19.8-console-commands-overhaul/19.8-05-SUMMARY.md
|
||||
autonomous: true
|
||||
requirements: [D-03, D-04, D-10, D-12, D-18, D-19, D-21, D-25, D-27, D-28, D-29]
|
||||
user_setup: []
|
||||
|
||||
estimate:
|
||||
tokens: 200000
|
||||
raw_tokens: 200000
|
||||
tasks: 7
|
||||
confidence: low
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "CR-01 (D-03, D-27): lines printed while a Spinner, Progress or Process view runs (Output.Println and Printf, status lines, Progress Fail lines, bonfire.PrintAbove from diary) appear in the order they were printed and none is lost when the view ends; a line printed from another goroutine while the view finishes appears once, after the final frame (TestPrintAboveOrderedNoLoss)."
|
||||
- "WR-02 (D-03): only the console that started a live view sends its Println, Printf and status lines above it; every other console (a bonfire.Call buffer, a nested command, a console on a file) writes to its own writer. At most one view registers at a time (check and set under one lock; a widget that loses falls back to plain lines), and a panic in the work function still unregisters, finishes and waits for the view (TestPrintlnStaysWithItsConsole, TestLiveViewSingleOwner, TestLiveViewPanicUnregisters)."
|
||||
- "WR-01 (D-03): SIGINT or SIGTERM during a live view kills the view (cursor and terminal restored) and then still ends the process with the signal's default action, or reaches the command's own signal handler when it has one. The generated main and bonfire.Execute's context are unchanged (TestLiveViewRestoresTerminalOnSignal)."
|
||||
- "CR-02 (D-18): `summer <delegate>` exits with the app binary's exit code (an exit 2 from the binary stays 2) and prints no second error block; a binary killed by a signal still gets summer's error block and exit 1. The carrier is the additive bonfire.WithExitCode, honoured by bonfire.Execute (TestDelegateExitCodePassthrough, TestWithExitCode)."
|
||||
- "CR-03 (D-27, D-28, D-04): on a terminal the delegated binary gets summer's own stdout and stderr (so its production question, styled summaries and widgets work); off a terminal its stdout goes through summer's Output and its stderr to summer's stderr, never into stdout (TestDelegateStreams, TestAppStreams)."
|
||||
- "WR-03 (D-25, D-29): summer dev sends serve SIGTERM and waits the same grace as the delegates (longer than serve's 10-second drain) before it kills it (TestDevStopWaitsForGracefulShutdown)."
|
||||
- "WR-04 (D-10): a daily file that fails to open is skipped and retried one minute later by the writer's clock, or at the first record of a new date, whichever comes first; the warning is logged once per date (TestDailyRetriesAfterFailure; TestDailyUnwritable and TestDailyDegradesSmoke stay green)."
|
||||
- "WR-06 (D-10): daily writers replaced by Setup, Bootstrap, ApplyGlobals or Reset are retired: a late record through them is appended and the file closed again, so no descriptor stays open; diary.Close keeps its documented reopen-on-next-record behaviour for the installed writers. ApplyGlobals holds the diary lock for its whole run, so concurrent calls rebuild the channels once (TestRetiredDailyWriterKeepsNoFile, TestApplyGlobalsRebuildsOnce)."
|
||||
- "WR-05 (D-12, D-18): error blocks (message, explanation, Try lines, -v chain), process-view item names and errors, and table cells strip control characters other than tab before styling, through the additive bonfire.Sanitize, which diary's console handler also uses (TestRenderErrorStripsControl, TestProcessViewStripsControl, TestTableStripsControl, TestSanitize; TestConsoleStripsControlChars stays green)."
|
||||
- "WR-07 (D-21, D-04): Migrate applies exactly the sets migrationSets lists (one list for Migrate, PendingMigrations, --pretend, the production question, the startup summary and migrate:status --json), keeping each set's error wording; migrate's 'N applied' lines and its result line count what Migrate actually applied, also when it fails partway (TestMigrateAppliedLines, Docker)."
|
||||
- "WR-08 (D-19): the wrong-directory Try line keeps the command and its other arguments but replaces the value of every flag whose name contains password, passwd, secret, token or key with ***, in both the --flag value and --flag=value forms (TestWrongDirectoryMasksSecrets)."
|
||||
- "Each CR and WR finding is fixed in its own commit that contains its pinning test, and replaying the commit without its non-test, non-doc changes makes the test fail (RED replay in Task 7). 19.8-REVIEW-DISPOSITION.md marks CR-01..CR-03 and WR-01..WR-08 fixed with their commit; IN-01..IN-09 are deferred to one pending todo (severity minor, area framework)."
|
||||
- "No exported identifier of the framework is removed or changes its signature; the only API additions are bonfire.WithExitCode and bonfire.Sanitize. The generated main is unchanged, so no shared plugin needs a require bump and no application a regenerate commit unless the SUMMARY's Release impact table says otherwise."
|
||||
artifacts:
|
||||
- path: "modules/bonfire/live.go"
|
||||
provides: "ordered, drained printing above live views; owner-checked registration; signal-safe teardown"
|
||||
contains: "func runLive("
|
||||
- path: "modules/bonfire/live_test.go"
|
||||
provides: "CR-01 and WR-02 pinning tests"
|
||||
contains: "TestPrintAboveOrderedNoLoss"
|
||||
- path: "modules/bonfire/live_signal_unix_test.go"
|
||||
provides: "WR-01 pinning test through a re-executed helper process"
|
||||
contains: "TestLiveViewRestoresTerminalOnSignal"
|
||||
- path: "modules/bonfire/errors.go"
|
||||
provides: "bonfire.WithExitCode, bonfire.Sanitize in the error block, masked wrong-directory hint"
|
||||
contains: "func WithExitCode("
|
||||
- path: "cmd/summer/runtime.go"
|
||||
provides: "delegates with pass-through exit codes and separate streams"
|
||||
contains: "WithExitCode"
|
||||
- path: "modules/diary/daily.go"
|
||||
provides: "retry after a failed open; retired writers"
|
||||
contains: "retire"
|
||||
- path: "modules/lagoon/migrations.go"
|
||||
provides: "Migrate over migrationSets"
|
||||
contains: "migrationSets(gdb, plugins)"
|
||||
- path: ".planning/todos/pending/2026-10-22-console-review-info-findings.md"
|
||||
provides: "IN-01..IN-09 captured"
|
||||
contains: "IN-09"
|
||||
key_links:
|
||||
- from: "modules/bonfire/output.go"
|
||||
to: "modules/bonfire/live.go"
|
||||
via: "Println, Printf and emit reroute above a view only when this console owns it"
|
||||
pattern: "owner"
|
||||
- from: "modules/diary/console.go"
|
||||
to: "modules/bonfire/errors.go"
|
||||
via: "diary's console sanitizer delegates to bonfire.Sanitize"
|
||||
pattern: "bonfire.Sanitize"
|
||||
- from: "cmd/summer/runtime.go"
|
||||
to: "modules/bonfire/execute.go"
|
||||
via: "runAppBinary wraps a child exit in Reported and WithExitCode; exitCode honours the carrier"
|
||||
pattern: "WithExitCode"
|
||||
- from: "modules/lagoon/migrations.go"
|
||||
to: "modules/lagoon/commands.go"
|
||||
via: "migrate reports pending-before minus pending-after as its applied lines"
|
||||
pattern: "PendingMigrations"
|
||||
- from: "scripts/check-phase19.8.sh"
|
||||
to: "modules/bonfire/live_test.go"
|
||||
via: "the go stage requires the pinning tests by name"
|
||||
pattern: "TestPrintAboveOrderedNoLoss"
|
||||
prohibitions:
|
||||
- statement: "No exported framework identifier is removed, renamed or changes its signature (core shared plugins and applications build against them); additions only."
|
||||
status: unresolved
|
||||
verification: judgment
|
||||
- statement: "WR-01 is fixed inside bonfire's live views: the generated main (internal/build), bonfire.Execute's context and Ctrl+C semantics of commands without a live view do not change."
|
||||
status: unresolved
|
||||
verification: judgment
|
||||
- statement: "IN-01..IN-09 are not fixed in this plan; they go to the pending todo."
|
||||
status: unresolved
|
||||
verification: judgment
|
||||
- statement: "No new Go module dependency; nothing is pushed or tagged by this plan (plan 06 owns the release)."
|
||||
status: unresolved
|
||||
verification: judgment
|
||||
---
|
||||
|
||||
## Phase Goal
|
||||
|
||||
ROADMAP Goal (not in user-story format; plans 01-04 recorded the decision not to invent one): console commands in `summer` and in application binaries reach the Artisan bar; a user sees what a command did, can ask for more or less of it, and learns from `--help` how to use it.
|
||||
|
||||
This plan (gap closure 1 of 2, wave 1) closes the code review of the shipped console (19.8-REVIEW.md, v0.3.1..v0.4.1): the three critical findings CR-01..CR-03 and the eight warnings WR-01..WR-08. The nine info findings IN-01..IN-09 become one pending todo. Plan 06 releases the fixes as v0.4.2 under a go-ahead.
|
||||
|
||||
## Spec-less probe fallback
|
||||
|
||||
Spec-less probe fallback skipped: the phase has no REQUIREMENTS.md IDs (`Requirements: TBD`) and no SPEC.md, as in plans 01-04. `requirements` lists the CONTEXT decisions each finding breaks; `must_haves` derive from the review's findings.
|
||||
|
||||
## Decisions taken in this plan (Claude's discretion inside the review's suggested fixes)
|
||||
|
||||
- **Order of work.** The tracer (Task 1) is CR-01 alone: one finding taken through the whole loop that every later task repeats (pinning test red, fix, green, one commit). The review's "bonfire live view" group (CR-01, WR-01, WR-02) therefore spans Tasks 1 and 2.
|
||||
- **CR-01 mechanism.** Each PrintAbove call becomes one print message placed straight on the program's message queue by the calling goroutine (the bubbletea event loop prints such a message itself, in queue order), instead of a model reply that bubbletea runs as a command on its own goroutine. Unregistering, sending the finish message and waiting for the program happen as one step under the registry lock, so no line reaches a stopped program and no line races the final frame. The review's alternative (a FIFO in the model drained with `tea.Sequence` before quitting) is the fallback if bubbletea v2.1.0's source does not support the first approach.
|
||||
- **WR-01 mechanism.** The review offers a process-wide `signal.NotifyContext` around Execute or a handler inside `runLive`. This plan takes the handler inside `runLive`: a process-wide context would change Ctrl+C for every plugin command that ignores its context (the process would no longer stop) and would change the generated main, forcing regenerate commits in every application.
|
||||
- **CR-02 carrier.** An explicit, additive `bonfire.WithExitCode(err, code)`; bonfire does not honour every error that happens to have an `ExitCode()` method, so plugin commands that return an `*exec.ExitError` keep exiting 1.
|
||||
- **WR-05 home.** `bonfire.Sanitize`, as the review suggests; diary's console sanitizer delegates to it so one implementation exists. IN-08 (raw C1 bytes) stays in the todo and now points at `bonfire.Sanitize`.
|
||||
- **WR-04 retry.** Retry after one minute (writer's clock) or on a new date, whichever comes first; one warning per date. The existing warning message `daily log file disabled` is kept so the pinned tests and any log alerts keep matching.
|
||||
- **WR-06 retired writers.** A retired writer appends a late record and closes the file again (open, write, close), so neither a record nor a descriptor is lost. `diary.Close` keeps its documented behaviour (installed writers reopen on the next record).
|
||||
- **WR-07 applied lines.** migrate computes pending migrations before and after Migrate and prints the difference, also when Migrate fails partway (then it prints what was applied and returns the error).
|
||||
- **Review line numbers.** The review's line numbers for `modules/diary/daily.go` and `modules/bonfire/execute.go` do not match the source (the code is unchanged since v0.4.1); every task names the symbols to locate.
|
||||
|
||||
## Artifacts this plan produces
|
||||
|
||||
- Fix commits (one per finding, each with its pinning test) in bonfire, cmd/summer, internal/dev, diary and lagoon, with README and docs updates in the same commit where documented behaviour or exported API changes.
|
||||
- `scripts/check-phase19.8.sh` requiring the new tests by name.
|
||||
- `19.8-REVIEW-DISPOSITION.md` updated; `.planning/todos/pending/2026-10-22-console-review-info-findings.md`.
|
||||
- `19.8-05-SUMMARY.md` with the RED evidence per finding, the Release impact table and `plan_head_before`/`plan_head_after`.
|
||||
|
||||
<objective>
|
||||
Fix the eleven critical and warning findings of the 19.8 code review, each in its own commit with a test that fails without the fix, without breaking any exported API, and capture the nine info findings as a todo.
|
||||
|
||||
Purpose: the console overhaul shipped in v0.4.0/v0.4.1 loses and reorders output above live views, hides the terminal from summer's delegates, misreports their exit codes, leaves the cursor hidden after Ctrl+C, drops the daily log trail after one failed open and prints untrusted escape sequences; D-03, D-10, D-12, D-18, D-19, D-21, D-25, D-27, D-28 and D-29 promise otherwise.
|
||||
Output: eleven fix commits, one gate commit, one planning-docs commit, the SUMMARY with a Release impact table for plan 06.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@~/.claude/msd-core/workflows/execute-plan.md
|
||||
@~/.claude/msd-core/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/phases/19.8-console-commands-overhaul/19.8-REVIEW.md
|
||||
@.planning/phases/19.8-console-commands-overhaul/19.8-REVIEW-DISPOSITION.md
|
||||
@.planning/phases/19.8-console-commands-overhaul/19.8-CONTEXT.md
|
||||
@.planning/phases/19.8-console-commands-overhaul/19.8-04-SUMMARY.md
|
||||
@.planning/phases/19.8-console-commands-overhaul/deferred-items.md
|
||||
|
||||
Repos and concurrency: summercms.go only. Several sessions work in this repository: before each task run `git status --porcelain` on its paths, stage only those paths by name (never `git add -A`), never touch `.planning/STATE.md` or `.planning/state.json` by hand, no co-author tags, planning docs and code in separate commits. Record `git rev-parse HEAD` as `plan_head_before` in the SUMMARY before the first commit. `go vet ./...` and `go test -short ./...` green at every commit; `go -C examples/hello test ./...` green before the last commit.
|
||||
|
||||
Fix loop for every finding (the tracer proves it once): write the pinning test, run it and keep the failing output (RED) for the SUMMARY, implement the fix, run the test and its package green (with `-race` for bonfire and diary), update README and docs when exported API or documented behaviour changes, commit the test, the fix and the docs together as one `fix(<module>): ...` commit. The RED replay in Task 7 re-proves each RED from the commits.
|
||||
|
||||
Docker: Docker-backed tests need `TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1`; if the socket is denied although the user is in the docker group, run the tests through `sudo -u golem`. A Docker test that could not run is NOT RUN in the SUMMARY, never PASS.
|
||||
|
||||
<interfaces>
|
||||
- bonfire console seam: `newConsole(in, out, errW, lookup, tty, interactive *bool)` and `setGlobals(Globals)` (modules/bonfire/output.go); `boolPtr`, `noEnv` (widgets_test.go); a terminal console on a bytes.Buffer runs the live path and the buffer receives the PrintAbove lines (TestPrintAbove).
|
||||
- bonfire live path: `runLive(ctx, out, c, model, work)`, `printAboveLines(s)`, `liveActive()`, `liveMu`, `activeProg`, `liveModel`, `printAboveMsg`, `finishMsg` (live.go); `console.live()`, `emit`, `Println`, `Printf` (output.go); `Spinner`, `Progress`, `liveProgress.Fail`, `Table` (widgets.go); `Process`, `PrintAbove` (helpers.go).
|
||||
- bubbletea v2.1.0 (path: `go list -m -f '{{.Dir}}' charm.land/bubbletea/v2`): tea.go `handleCommands` (each command on its own goroutine), the event loop's `case printLineMessage` (prints in the loop), `Program.Send` (returns once the program has stopped), `Program.Kill` (restores the terminal), `execSequenceMsg`; renderer.go `Println`.
|
||||
- bonfire errors: `RenderError`, `wrongDirectory(name, err, args, dir)`, `shellQuote`, `Reported`, `ErrReported`, `isUsage` (errors.go); `Execute`, `exitCode`, `Fail` (execute.go).
|
||||
- summer delegates: `appRunner`, `delegateCommand`, `runAppBinary`, `newAppCmd`, `runChild`, `appShutdownGrace` (cmd/summer/runtime.go); test helpers `captureAppRunner`, `runSummer`, `TestSignalHelperProcess`, `helperArgv`, `helperEnv`, `syncBuffer`, `waitForOutput` (cmd/summer/runtime_test.go).
|
||||
- summer dev: `Options{Build, Start, Debounce}`, `watch`, `child.stop`, `startBin` (internal/dev/watch.go); serve's drain timeout is the 10-second `context.WithTimeout` in modules/surf/serve.go.
|
||||
- diary: `dailyWriter{mu, disabled, file, date, now, warn}`, `Write`, `open`, `Close`, `newDailyWriter` (daily.go); `install`, `SetVerbosity`, `ApplyGlobals`, `Close`, `closeChannels`, `Reset`, `mu`, `current`, `gens` (diary.go); `channelBuilder.warn` with message `daily log file disabled` (channels.go); `sanitize` (console.go); `Options.Now`.
|
||||
- lagoon: `Migrate`, `migrator`, `migrationSet`, `migrationSets`, `PendingMigrations` (migrations.go); the `migrate` command's Run (commands.go); Docker helpers `pendingPlugins`, `migPlugin`, `runLagoon` (pending_test.go).
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="tracer" tdd="true">
|
||||
<name>Task 1: Tracer: CR-01 lines printed above a live view keep their order and none is lost when the view ends (one fix through the whole loop: red test, fix, green, one commit)</name>
|
||||
<files>modules/bonfire/live.go, modules/bonfire/widgets.go, modules/bonfire/output.go, modules/bonfire/helpers.go, modules/bonfire/live_test.go</files>
|
||||
<behavior>
|
||||
- TestPrintAboveOrderedNoLoss, subtests spinner, progress and process: on a terminal console over a bytes.Buffer (newConsole with tty and interactive true), 20 runs per widget; in each run the work function prints 100 numbered lines (`line-000` .. `line-099`, mixing Output.Println, Output.Printf with a trailing newline, Output.Info and bonfire.PrintAbove), the progress subtest also records two failures through bonfire.Failed between them, and the last line is printed immediately before the work function returns. After the widget returns, the ANSI-stripped buffer holds every numbered line exactly once and in order, and both failure lines.
|
||||
- Subtest concurrent_finish: a goroutine started inside the work function keeps calling bonfire.PrintAbove after work returns; each of its lines appears exactly once in the buffer (above the view or after its final frame), none is lost.
|
||||
- TestPrintAbove, TestWidgetMatrix and TestLiveModels stay green.
|
||||
</behavior>
|
||||
<read_first>modules/bonfire/live.go (whole file), modules/bonfire/widgets.go (Spinner, Progress, liveProgress), modules/bonfire/output.go (emit, Println, Printf, live), modules/bonfire/helpers.go (Process, PrintAbove), modules/bonfire/widgets_test.go (TestPrintAbove and the console helpers), modules/bonfire/live_test.go, the bubbletea v2.1.0 sources named in the interfaces block, 19.8-REVIEW.md (CR-01)</read_first>
|
||||
<action>Per CR-01 (D-03, D-27). Verify the review's claim against bubbletea v2.1.0's source before changing anything: `liveModel.Update` answers each `printAboveMsg` with `tea.Println(...)`, a command that `handleCommands` runs on its own goroutine, so successive lines race back into the event loop and a line still in flight when `finishMsg` quits the program is dropped.
|
||||
|
||||
Write TestPrintAboveOrderedNoLoss in modules/bonfire/live_test.go and run it on the unchanged code; keep the failing output for the SUMMARY (out-of-order or missing lines). If it does not fail in three runs, raise the runs or the line count until it does, and record the numbers.
|
||||
|
||||
Fix in live.go (and where the callers send): every PrintAbove call (printAboveLines, and liveProgress.Fail's failure line) becomes ONE print message for the whole block, placed directly on the program's message queue by the calling goroutine in call order, so the event loop prints it itself without a command goroutine (for example send the message value that `tea.Println(block)` returns, through `Program.Send`, which also returns instead of blocking once the program has stopped; never call `Program.Println`, which blocks forever on a stopped program). Then make the end of runLive one step under `liveMu`: clear the registration, send `finishMsg` and wait for the program to finish while holding the lock, so a printer that arrives meanwhile waits and then writes directly after the final frame. Remove `printAboveMsg` handling from `liveModel` if nothing sends it any more. If the source shows the direct-queue approach does not hold, implement the review's alternative instead (a FIFO of pending lines in the model, drained with `tea.Sequence(<prints>..., tea.Quit)` on `finishMsg`) and record why.
|
||||
|
||||
No exported signature changes (bonfire.PrintAbove keeps its signature and meaning). The README's PrintAbove row already describes the intended behaviour; no docs change unless a sentence there contradicts the fix. Commit test and fix together: `fix(bonfire): keep the order of lines printed above a live view and drain them before it ends`.</action>
|
||||
<verify>
|
||||
<automated>go test -count=1 -race -short -run '^(TestPrintAboveOrderedNoLoss|TestPrintAbove|TestWidgetMatrix|TestLiveModels)$' -v ./modules/bonfire 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 4 && go vet ./... && go test -count=1 -short ./...</automated>
|
||||
<fails_when>CR-01: fewer than 4 top-level "--- PASS: Test" lines (a numbered line missing, duplicated or out of order in any of the 60 widget runs, a failure line missing, a concurrent_finish line lost or printed twice, or a race report), or vet or the short suite fail.</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- CR-01: `go test -count=3 -race -short -run '^TestPrintAboveOrderedNoLoss$' ./modules/bonfire` exits 0 (three clean passes in a row).
|
||||
- CR-01: the SUMMARY records the RED output of TestPrintAboveOrderedNoLoss on the code before the fix (the first out-of-order or missing line).
|
||||
- The CR-01 fix commit (found by its subject in `git log --format='%h %s' <plan_head_before>..HEAD`) lists modules/bonfire/live_test.go together with the changed non-test files in `git show --stat`.
|
||||
</acceptance_criteria>
|
||||
<done>Output printed during any live view arrives complete and in order, proven by a deterministic buffer test that fails on the shipped code, in one commit.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: WR-02 then WR-01: a live view takes lines only from the console that owns it, registers by check and set and unregisters on panic; Ctrl+C or SIGTERM during a view restores the terminal and keeps the signal's effect</name>
|
||||
<files>modules/bonfire/live.go, modules/bonfire/output.go, modules/bonfire/widgets.go, modules/bonfire/helpers.go, modules/bonfire/live_test.go, modules/bonfire/live_signal_unix_test.go, modules/bonfire/README.md</files>
|
||||
<behavior>
|
||||
- TestPrintlnStaysWithItsConsole: while a Spinner runs on terminal console A (buffer A), console B (a non-terminal console over buffer B) calls Println, Printf and Info; buffer B holds all three and buffer A none of them; console A's own Println still prints above its view.
|
||||
- TestLiveViewSingleOwner: two terminal consoles start a Spinner at the same moment (a barrier, 200 rounds); in every round exactly one runs as a live view and the other prints its plain `[...] <label>` line; after each round bonfire.PrintAbove reports false.
|
||||
- TestLiveViewPanicUnregisters: a Spinner whose work panics; the test recovers the panic; afterwards bonfire.PrintAbove reports false and a new Spinner runs as a live view.
|
||||
- TestLiveViewRestoresTerminalOnSignal (unix build tag, the test binary re-executed as a helper with an environment switch, like cmd/summer's TestSignalHelperProcess): the helper runs a Spinner on a terminal console over its stdout pipe, prints `ready` on stderr and blocks; the test sends SIGINT. Subtest default: the helper dies by SIGINT (default action kept) and its stdout holds the show-cursor sequence after the last hide-cursor sequence. Subtest own_handler: the helper also holds its own signal.NotifyContext, exits 0 by itself after its context ends, and its stdout holds the show-cursor sequence after the last hide-cursor sequence.
|
||||
</behavior>
|
||||
<read_first>modules/bonfire/live.go and output.go after Task 1, modules/bonfire/widgets.go, modules/bonfire/helpers.go (Process passes its ctx), cmd/summer/runtime_test.go (TestSignalHelperProcess, helperArgv, helperEnv, waitForOutput: the re-exec helper pattern), modules/lagoon/pty_linux_test.go (the syscall-only pseudo-terminal helper, only if a pipe does not show the cursor sequences), internal/build/build.go (the generated main: must stay unchanged), 19.8-REVIEW.md (WR-01, WR-02)</read_first>
|
||||
<action>Two findings, two commits, WR-02 first because WR-01 builds on the registration it reworks.
|
||||
|
||||
WR-02 (D-03): verify against the source that `console.Println`, `console.Printf` and `emit` reroute through the single process-wide `activeProg` without checking which console started it, that `console.live()`'s `liveActive()` check and runLive's registration are separate steps, and that runLive unregisters without `defer`. Write the three WR-02 tests and record their RED output (the single-owner test may need more rounds to fail; if it does not fail in three runs, add an unexported test hook called between the check and the registration, nil in production, to force the interleaving, and record that). Fix: keep the owning console with the registered program; Println, Printf and emit reroute only when the calling console is the owner (bonfire.PrintAbove, used by diary, keeps rerouting to whichever view runs, as documented). Replace the separate check with a check-and-set claim under `liveMu` taken before the program starts; a widget that cannot claim prints its plain lines as nested widgets do today. Unregister, finish and wait in a deferred step (keeping Task 1's single locked step), so a panic in work leaves no orphaned registration and still re-panics. Commit: `fix(bonfire): reroute only the owning console's lines to a live view`.
|
||||
|
||||
WR-01 (D-03): verify that the live views run with bubbletea's own signal handling off, that Spinner and Progress pass a background context and that neither summer nor the generated main installs a signal handler, so SIGINT's default action ends the process with the cursor hidden. Write TestLiveViewRestoresTerminalOnSignal and record its RED output (no show-cursor after the last hide-cursor). First confirm on the unchanged code that the helper's stdout pipe carries the hide-cursor sequence; if a pipe never carries the cursor sequences, open a pseudo-terminal for the helper's stdout with a copy of the syscall-only helper from modules/lagoon/pty_linux_test.go (linux build tag, no new dependency). Fix inside runLive only: for the life of the view, relay SIGINT and SIGTERM to a channel with signal.Notify (skip a signal that signal.Ignored reports); on a signal, kill the program (Program.Kill restores the cursor and the terminal), wait for it to stop, clear the registration if it is still this view, stop relaying (signal.Stop), then re-raise the same signal on the own process (os.FindProcess(os.Getpid()) then Signal; if that fails, exit with 128 plus the signal number) so the default action, or the command's own registered handler, still applies. When the view ends normally, stop relaying before runLive returns. Do not change internal/build's generated main, bonfire.Execute's context or the summer main. README: the Spinner, Progress and Process rows or the widgets paragraph say that Ctrl+C during a view restores the terminal and then stops the command as before. Commit: `fix(bonfire): restore the terminal when a live view is interrupted`.</action>
|
||||
<verify>
|
||||
<automated>go test -count=1 -race -short -run '^(TestPrintlnStaysWithItsConsole|TestLiveViewSingleOwner|TestLiveViewPanicUnregisters|TestLiveViewRestoresTerminalOnSignal|TestPrintAboveOrderedNoLoss|TestPrintAbove|TestWidgetMatrix)$' -v ./modules/bonfire 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 7</automated>
|
||||
<fails_when>WR-02: TestPrintlnStaysWithItsConsole, TestLiveViewSingleOwner or TestLiveViewPanicUnregisters does not pass (a line lands in another console's view, two views run at once, or a panic leaves a registration behind). WR-01: TestLiveViewRestoresTerminalOnSignal does not pass (the cursor is left hidden, the default signal action is lost, or the command's own handler no longer receives the signal). Any race report, or Task 1's tests regress.</fails_when>
|
||||
<automated>go vet ./... && go test -count=1 -short ./... && F=$(git log --format= --name-only --grep='^fix(bonfire): restore the terminal when a live view is interrupted$' --grep='^fix(bonfire): reroute only the owning console' -- internal/build cmd/summer/main.go) && test -z "$F"</automated>
|
||||
<fails_when>vet or the short suite fail, or either fix commit touches the generated main (internal/build) or summer's main.</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- WR-02: `go test -count=3 -race -short -run '^(TestPrintlnStaysWithItsConsole|TestLiveViewSingleOwner|TestLiveViewPanicUnregisters)$' ./modules/bonfire` exits 0, and the SUMMARY records each test's RED output.
|
||||
- WR-01: `go test -count=1 -short -run '^TestLiveViewRestoresTerminalOnSignal$' -v ./modules/bonfire 2>&1 | grep -cE -- '--- PASS: TestLiveViewRestoresTerminalOnSignal/(default|own_handler)'` prints 2, and the SUMMARY records the RED output.
|
||||
- `git log --format=%s <plan_head_before>..HEAD` lists the WR-02 and WR-01 fix subjects as two separate commits (other sessions' commits may sit between them), and `git show --stat` of each lists its test file.
|
||||
</acceptance_criteria>
|
||||
<done>A live view belongs to one console, cannot be registered twice or left registered, and an interrupted view gives the terminal back without changing how the signal ends the command.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 3: summer delegates and dev: CR-02 exit codes pass through without a second error block, CR-03 the binary gets the terminal and a separate stderr, WR-03 dev waits serve's full drain, WR-08 the wrong-directory hint masks secret flag values</name>
|
||||
<files>cmd/summer/runtime.go, cmd/summer/runtime_test.go, modules/bonfire/execute.go, modules/bonfire/errors.go, modules/bonfire/errors_test.go, modules/bonfire/README.md, internal/dev/watch.go, internal/dev/watch_test.go, docs/console/introduction.md, docs/console/writing-commands.md, docs/console/scaffolding.md, docs/setup/upgrading.md</files>
|
||||
<behavior>
|
||||
- TestWithExitCode (bonfire): bonfire.Execute returns 2 for a command returning Reported(WithExitCode(err, 2)) and prints no error block; WithExitCode(err, 3) without Reported prints the block and returns 3; codes below 1 or above 255 return ExitFailure; WithExitCode(nil, 2) is nil; errors.Is and errors.As still see the wrapped error.
|
||||
- TestDelegateExitCodePassthrough (cmd/summer): a summer root runs a delegate whose child (the test binary re-executed as a helper) writes a line on stderr and exits 2; Execute returns 2 and summer's own output and error streams hold no ERROR block and no `summer: migrate` text. A child killed by SIGKILL gives exit 1 and one summer error block naming the signal.
|
||||
- TestAppStreams (cmd/summer): the stream choice for a terminal Output returns os.Stdout and os.Stderr; for a non-terminal Output it returns that Output and os.Stderr.
|
||||
- TestDelegateStreams (cmd/summer): with a non-terminal buffer console, a helper child writing `data-line` to stdout and `log-line` to stderr leaves `data-line` in the buffer and no `log-line` in it.
|
||||
- TestDevStopWaitsForGracefulShutdown (internal/dev): a helper child that traps SIGTERM, sleeps 2.5 seconds and exits 0 is stopped through child.stop and reports exit 0 (not killed); dev's stop grace equals the delegates' grace and exceeds 10 seconds.
|
||||
- TestWrongDirectoryMasksSecrets (bonfire): from a temp tree (application root with summer.yaml, cwd without config/), args `admin:create --email a@b.c --password hunter2`, `--password=hunter2`, `--client-secret s3cr3t`, `--api-token=t0k3n` and `--key k3y` give a Try line without hunter2, s3cr3t, t0k3n or k3y, with `--password ***` or `--password=***` in their place and `--email a@b.c` unchanged; TestWrongDirectoryHint stays green.
|
||||
</behavior>
|
||||
<read_first>cmd/summer/runtime.go (delegateCommand, runAppBinary, newAppCmd, runChild, appShutdownGrace), cmd/summer/runtime_test.go (captureAppRunner, runSummer, the signal helper), cmd/summer/main.go (how the root is built), modules/bonfire/execute.go (Execute, exitCode, Fail), modules/bonfire/errors.go (Reported, RenderError, wrongDirectory, shellQuote), modules/bonfire/errors_test.go and wrongdir_smoke_test.go (TestWrongDirectoryHint), internal/dev/watch.go (watch's stream choice, child.stop, startBin), internal/dev/watch_test.go (TestStartBinServe, TestDevGracefulNil), modules/surf/serve.go (the 10-second drain), docs/console/introduction.md ("Exit codes", "Commands that summer delegates"), docs/console/scaffolding.md (summer dev), docs/console/writing-commands.md (wrong-directory example), docs/setup/upgrading.md ("Upgrading to 0.4"), modules/bonfire/README.md (error block paragraph, API table), 19.8-REVIEW.md (CR-02, CR-03, WR-03, WR-08)</read_first>
|
||||
<action>Four findings, four commits, in this order. Verify each claim against the source first and record the pinning test's RED output.
|
||||
|
||||
CR-02 (D-18): runAppBinary returns `summer: <argv>: exit status N` wrapping the child's *exec.ExitError, so Execute renders a second error block under the one the binary printed and maps it to exit 1. Add to bonfire an exported carrier `WithExitCode(err error, code int) error` (unexported type with Error, Unwrap; nil stays nil) and make `exitCode` honour it before the usage check (a code outside 1..255 maps to ExitFailure). In runAppBinary, when the child exited with a code (`ExitError.Exited()`), return `bonfire.Reported(bonfire.WithExitCode(<the wrapped error>, code))`; a child ended by a signal keeps today's error block and exit 1. Factor the part of runAppBinary after locating the binary into a function the tests can call with the test binary as the child. README: an API row for bonfire.WithExitCode and the Reported row amended ("exit code 1, or the code of bonfire.WithExitCode"); docs/console/introduction.md: the delegates section says summer exits with the binary's exit code; docs/setup/upgrading.md: one bullet under "Upgrading to 0.4" behaviour changes. Commit: `fix(summer): pass the app binary's exit code through and print its error once`.
|
||||
|
||||
CR-03 (D-27, D-28, D-04): newAppCmd sets the child's stdout and stderr to summer's Output, so the child never sees a terminal (no production question, no styled summary, no widgets) and its stderr lands in summer's stdout. Add a stream choice used by the delegates: on a terminal Output (bonfire.IsTerminal) the child gets os.Stdout and os.Stderr; otherwise stdout goes to the Output (keeps tests and Call roots capturing) and stderr to os.Stderr. newAppCmd takes the two writers (unexported; adjust its existing tests). The child keeps os.Stdin. Docs: the delegates section of docs/console/introduction.md says the binary writes to the terminal directly and its stderr stays separate, so `summer route:list --json > routes.json` captures only the JSON; extend the upgrading bullet. Commit: `fix(summer): give delegated commands the terminal and keep their stderr apart`.
|
||||
|
||||
WR-03 (D-25, D-29): child.stop in internal/dev/watch.go kills serve two seconds after SIGTERM while serve drains for up to ten. Move the grace into one constant shared by the delegates and dev: `ShutdownGrace` in internal/dev (15 seconds, with a comment naming serve's 10-second drain; internal, so not public API), used by child.stop, with cmd/summer's appShutdownGrace defined as `dev.ShutdownGrace`. The helper child for the test may be the test binary re-executed with an environment switch. docs/console/scaffolding.md: the summer dev paragraph says Ctrl+C waits for serve's shutdown report (up to its drain time) before stopping it. Commit: `fix(dev): give serve the full shutdown grace before killing it`.
|
||||
|
||||
WR-08 (D-19): wrongDirectory builds the Try line from every argument, so a `--password` value reaches stderr and CI logs. When building the Try line, replace the value of every long flag whose name (case-insensitive) contains password, passwd, secret, token or key with `***`, in the `--flag value` form (the next argument when it does not start with `-`) and the `--flag=value` form; write the mask without shell quoting; leave every other argument as today. README (error block paragraph) and docs/console/writing-commands.md (the wrong-directory paragraph) say secret-looking flag values are masked. Commit: `fix(bonfire): mask secret flag values in the wrong-directory hint`.
|
||||
|
||||
No other exported signature changes; bonfire.Reported, bonfire.Fail and bonfire.Execute keep their signatures.</action>
|
||||
<verify>
|
||||
<automated>go test -count=1 -race -short -run '^(TestWithExitCode|TestWrongDirectoryMasksSecrets|TestWrongDirectoryHint|TestExitCodes|TestReportedSilent|TestErrorGolden)$' -v ./modules/bonfire 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 6</automated>
|
||||
<fails_when>CR-02 (bonfire side): TestWithExitCode fails (the carrier code is ignored, a block is printed for a Reported carrier, or an out-of-range code escapes). WR-08: TestWrongDirectoryMasksSecrets fails (a secret value appears in the Try line or another argument changed). Or TestWrongDirectoryHint, TestExitCodes, TestReportedSilent or the error goldens regress.</fails_when>
|
||||
<automated>go test -count=1 -short -run '^(TestDelegateExitCodePassthrough|TestAppStreams|TestDelegateStreams|TestRunChildForwardsSignals|TestNewAppCmdCancelSendsSIGTERM|TestDelegateExamples)$' -v ./cmd/summer 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 6</automated>
|
||||
<fails_when>CR-02: TestDelegateExitCodePassthrough fails (summer exits 1 for a child's 2, prints a second error block, or a signalled child is silent). CR-03: TestAppStreams or TestDelegateStreams fails (the child's stderr reaches summer's stdout, or a terminal Output does not hand the child the process's own streams). Or the existing delegate signal tests regress.</fails_when>
|
||||
<automated>go test -count=1 -short -run '^(TestDevStopWaitsForGracefulShutdown|TestStartBinServe|TestDevReport|TestDevGracefulNil)$' -v ./internal/dev 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 4 && go vet ./... && go test -count=1 -short ./... && go run ./cmd/summer docs:build --check</automated>
|
||||
<fails_when>WR-03: TestDevStopWaitsForGracefulShutdown fails (serve is killed before its drain ends, or dev's grace differs from the delegates'). Or the dev tests, vet, the short suite or the docs checker (a README or docs identifier that does not exist) fail.</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- CR-02: `grep -c '^func WithExitCode(' modules/bonfire/errors.go modules/bonfire/execute.go | awk -F: '{s+=$2} END {print s}'` prints 1 and `grep -c 'bonfire.WithExitCode' modules/bonfire/README.md` prints at least 1.
|
||||
- CR-03: `grep -c 'os.Stderr' cmd/summer/runtime.go` prints at least 1 and TestDelegateStreams passes.
|
||||
- WR-03: `grep -c 'dev.ShutdownGrace' cmd/summer/runtime.go` prints at least 1 and `grep -c 'ShutdownGrace' internal/dev/watch.go` prints at least 2 (declaration and use in child.stop); TestDevStopWaitsForGracefulShutdown passes.
|
||||
- WR-08: TestWrongDirectoryMasksSecrets passes and `grep -ci 'masked\|\*\*\*' docs/console/writing-commands.md` prints at least 1.
|
||||
- `git log --format=%s <plan_head_before>..HEAD` lists the four fix subjects as four separate commits; the SUMMARY records each pinning test's RED output.
|
||||
</acceptance_criteria>
|
||||
<done>summer's delegates behave like running the binary directly (terminal, separate streams, same exit code, one error block), summer dev lets serve finish its shutdown, and the wrong-directory hint never repeats a secret.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 4: diary: WR-04 a failed daily open is retried after a minute or on the next date, WR-06 replaced daily writers keep no descriptor and ApplyGlobals rebuilds once</name>
|
||||
<files>modules/diary/daily.go, modules/diary/diary.go, modules/diary/daily_test.go, modules/diary/diary_test.go, modules/diary/README.md, docs/services/logging.md</files>
|
||||
<behavior>
|
||||
- TestDailyRetriesAfterFailure: with an injected clock at 2026-10-21 10:00:00 and a regular file where the log directory should be, the first record warns once (`daily log file disabled`); the test removes the blocking file; a record at 10:00:30 writes nothing and does not warn again; a record at 10:01:01 creates summer-2026-10-21.log holding it. A second scenario: the failure on 2026-10-21 23:59:59, blocker removed, a record at 2026-10-22 00:00:01 creates summer-2026-10-22.log; if the new date fails as well, exactly one more warning appears for it.
|
||||
- TestRetiredDailyWriterKeepsNoFile: a daily writer that has written once is retired (as install and Reset do with the previous channels); a later record still lands in the day's file and afterwards the writer holds no open file; diary.Close on the installed writers still lets the next record reopen the file.
|
||||
- TestApplyGlobalsRebuildsOnce: after Setup with color auto, 50 goroutines released by a barrier call ApplyGlobals with color never; the channels are rebuilt exactly once (the generation counter advances by one) and every call's verbosity is applied; run under -race.
|
||||
- TestDailyUnwritable, TestDailyDegradesSmoke, TestDailyRotation, TestConcurrentWrites and TestResetRestoresDefault stay green.
|
||||
</behavior>
|
||||
<read_first>modules/diary/daily.go (dailyWriter, Write, open, Close), modules/diary/diary.go (install, SetVerbosity, ApplyGlobals, Close, closeChannels, Reset), modules/diary/channels.go (buildChannels, channelBuilder.warn), modules/diary/daily_test.go (TestDailyUnwritable, the setup helper, lockedBuffer, dailyOnly), modules/diary/diary_test.go, modules/diary/README.md (Daily files), docs/services/logging.md (the paragraph about a file that cannot be opened), 19.8-REVIEW.md (WR-04, WR-06), 19.8-01-PLAN.md threat T-19.8-06</read_first>
|
||||
<action>Two findings, two commits. Locate the code by symbol (the review's daily.go line numbers do not match the file). Verify each claim and record the pinning tests' RED output.
|
||||
|
||||
WR-04 (D-10): dailyWriter.Write sets `disabled` on the first failed open and never clears it, so one transient failure (ENOSPC, EMFILE, a directory briefly removed at rotation) stops file logging until restart. Replace the permanent flag with a retry point: after a failed open, skip writes (still returning len(p), nil) until one minute has passed by the writer's clock or the local date changes, whichever comes first, then try to open again; warn through the existing warn function once per failed date (keep the message `daily log file disabled` so the pinned tests and existing alerts match). T-19.8-06's rule stands: boot never fails on a log file. Update the dailyWriter doc comment, modules/diary/README.md (Daily files bullet) and docs/services/logging.md (the cannot-be-opened paragraph: the channel skips records and tries again a minute later or on the next day, warning once per day). Commit: `fix(diary): retry a daily file that failed to open`.
|
||||
|
||||
WR-06 (D-10): install and Reset swap the state and then close the old channels' daily writers, but a goroutine holding the old state reopens the file through the closed writer and nothing closes it again; ApplyGlobals reads the state outside `mu`, so two concurrent calls can both rebuild. Add a retire step for replaced channels, separate from Close: a retired daily writer closes its file, and every later write opens the day's file, appends the record and closes it again (no record lost, no descriptor kept). install and Reset retire the old channels; the public diary.Close keeps closing the installed writers with reopen on the next record, as documented. Make ApplyGlobals hold `mu` for its whole run, using lock-free internal variants of install and SetVerbosity so nothing locks twice. Commit: `fix(diary): retire replaced daily writers and serialise ApplyGlobals`.
|
||||
|
||||
No exported signature changes in diary.</action>
|
||||
<verify>
|
||||
<automated>go test -count=1 -race -short -run '^(TestDailyRetriesAfterFailure|TestRetiredDailyWriterKeepsNoFile|TestApplyGlobalsRebuildsOnce|TestDailyUnwritable|TestDailyDegradesSmoke|TestDailyRotation|TestConcurrentWrites|TestResetRestoresDefault)$' -v ./modules/diary 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 8</automated>
|
||||
<fails_when>WR-04: TestDailyRetriesAfterFailure fails (no retry after a minute or on a new date, a retry before the minute, or more than one warning per date). WR-06: TestRetiredDailyWriterKeepsNoFile or TestApplyGlobalsRebuildsOnce fails (a retired writer keeps a file open or loses the record, diary.Close no longer reopens, or concurrent ApplyGlobals calls rebuild more than once). Or a race report, or the existing daily, rotation, concurrency or reset tests regress.</fails_when>
|
||||
<automated>go vet ./... && go test -count=1 -short ./... && go run ./cmd/summer docs:build --check</automated>
|
||||
<fails_when>vet, the short suite or the docs checker fail.</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- WR-04: `grep -ci 'next day\|a minute\|one minute' docs/services/logging.md` prints at least 1 and TestDailyRetriesAfterFailure passes.
|
||||
- WR-06: `go test -count=3 -race -short -run '^(TestRetiredDailyWriterKeepsNoFile|TestApplyGlobalsRebuildsOnce)$' ./modules/diary` exits 0.
|
||||
- `git log --format=%s <plan_head_before>..HEAD` lists the WR-04 and WR-06 fix subjects as two separate commits; the SUMMARY records each pinning test's RED output.
|
||||
</acceptance_criteria>
|
||||
<done>A long-running server keeps its file log trail through transient failures and setup changes, without leaking descriptors.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 5: WR-05 error blocks, process views and tables strip control characters through the additive bonfire.Sanitize, which diary's console handler also uses</name>
|
||||
<files>modules/bonfire/errors.go, modules/bonfire/live.go, modules/bonfire/widgets.go, modules/bonfire/helpers.go, modules/bonfire/errors_test.go, modules/bonfire/live_test.go, modules/bonfire/widgets_test.go, modules/bonfire/README.md, modules/diary/console.go, docs/console/writing-commands.md</files>
|
||||
<behavior>
|
||||
- TestSanitize: tab kept, CR and LF become spaces, ESC, BEL, other C0 controls and DEL removed, clean strings returned unchanged; the same table as diary's TestConsoleStripsControlChars.
|
||||
- TestRenderErrorStripsControl: with color forced on (Globals Color always), an error whose message, HintError explanation and Try text and wrapped cause (at -v) carry `\x1b]0;pwned\x07` and `\x1b[2J` renders a block that contains neither injected sequence (the palette's own styling sequences may remain) and still shows the visible text.
|
||||
- TestProcessViewStripsControl: the process model's View, with an item name and an item error carrying the same sequences, contains neither.
|
||||
- TestTableStripsControl: Table on a terminal console with color forced, and on a non-terminal console, with a cell and a header carrying the sequences, prints neither.
|
||||
- TestConsoleStripsControlChars (diary) and the bonfire goldens stay green.
|
||||
</behavior>
|
||||
<read_first>modules/diary/console.go (sanitize and its callers), modules/bonfire/errors.go (RenderError, writeStyled, causes), modules/bonfire/live.go (processModel.View), modules/bonfire/widgets.go (Table, failureLine), modules/bonfire/helpers.go (Process, failure), modules/bonfire/errors_test.go, modules/bonfire/live_test.go, modules/bonfire/widgets_test.go, modules/diary/console_test.go (TestConsoleStripsControlChars), modules/bonfire/README.md, docs/console/writing-commands.md, 19.8-REVIEW.md (WR-05, IN-08), 19.8-01-PLAN.md threat T-19.8-03</read_first>
|
||||
<action>Per WR-05 (D-12, D-18; extends threat T-19.8-03 from diary's console to bonfire's output). Verify against the source that RenderError passes text through sunscreen.Scrub only (which keeps ESC and other control characters), that processModel.View prints item names and `it.err.Error()` unchanged and that Table prints cells unchanged, and that the colorprofile writer passes such sequences through when color is on. Write the four tests with color forced on (the NoTTY profile strips all escape sequences and would hide the defect) and record their RED output.
|
||||
|
||||
Move diary's sanitizer into bonfire as the exported `Sanitize(s string) string` with the same rules (tab kept, CR and LF to a space, every other control character dropped) and make diary's console handler call it, deleting diary's copy. Apply it before styling to: every error-block line (message, explanation, Try lines and the -v chain, after splitting into lines and alongside sunscreen.Scrub), process-view item names and item errors, the failure lines of Progress and Process, and table headers and cells in both the terminal and the plain layout. Do not add the IN-08 change (invalid UTF-8 and raw C1 bytes); the todo records it against bonfire.Sanitize.
|
||||
|
||||
README: an API row for bonfire.Sanitize and one sentence in the error block and widgets text that printed text is stripped of control characters; docs/console/writing-commands.md: one sentence where error output and tables are described. Commit: `fix(bonfire): strip control characters from error blocks, process views and tables`.</action>
|
||||
<verify>
|
||||
<automated>go test -count=1 -race -short -run '^(TestSanitize|TestRenderErrorStripsControl|TestProcessViewStripsControl|TestTableStripsControl|TestErrorGolden|TestListGolden|TestWidgetMatrix|TestLiveModels)$' -v ./modules/bonfire 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 8 && go test -count=1 -race -short -run '^(TestConsoleStripsControlChars|TestSlogtestConsoleHandler)$' ./modules/diary && go vet ./... && go test -count=1 -short ./... && go run ./cmd/summer docs:build --check</automated>
|
||||
<fails_when>WR-05: an injected OSC or CSI sequence survives in an error block, process view or table (TestRenderErrorStripsControl, TestProcessViewStripsControl, TestTableStripsControl), TestSanitize disagrees with diary's rules, diary's console tests regress, a golden changes, or vet, the short suite or the docs checker fail.</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- WR-05: `grep -c '^func Sanitize(' modules/bonfire/*.go | awk -F: '{s+=$2} END {print s}'` prints 1, `grep -c 'bonfire.Sanitize' modules/diary/console.go` prints at least 1 and `grep -c 'bonfire.Sanitize' modules/bonfire/README.md` prints at least 1.
|
||||
- `git log --format=%h <plan_head_before>..HEAD -- modules/bonfire/testdata` prints nothing (no golden changed by this plan).
|
||||
- The SUMMARY records the RED output of the three WR-05 output tests.
|
||||
</acceptance_criteria>
|
||||
<done>Text from databases, remote services or admins can no longer move the cursor or retitle the operator's terminal through error blocks, process views or tables.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 6: WR-07 Migrate and PendingMigrations read one migration set list, and migrate's applied lines count what Migrate actually applied</name>
|
||||
<files>modules/lagoon/migrations.go, modules/lagoon/commands.go, modules/lagoon/pending_test.go, modules/lagoon/README.md</files>
|
||||
<precondition>A Docker daemon answers (`docker info`, directly or through `sudo -u golem`); TestMigrateAppliedLines is a testcontainers test. Without one, stop the task and report it.</precondition>
|
||||
<behavior>
|
||||
- TestMigrateAppliedLines (Docker, skipped under -short through the package's TestMain): two fixture plugins, alpha with one migration that succeeds and beta with one whose SQL fails; `migrate` on a fresh database prints `alpha: 1 applied` and the framework sets' applied lines, returns beta's error and prints no success line; after replacing beta's migration with a valid one, a second `migrate` prints `beta: 1 applied` and `Applied 1 migrations`, and a third prints `Nothing to migrate`.
|
||||
- TestPendingMigrations, TestMigratePretend, TestMigrateStatusJSON, TestMigrateRunsSystemFilesFirst and TestTwoPluginMigrationSetsIsolated stay green.
|
||||
</behavior>
|
||||
<read_first>modules/lagoon/migrations.go (Migrate, migrator, migrationSet, migrationSets, PendingMigrations), modules/lagoon/commands.go (the migrate command's Run), modules/lagoon/pending_test.go (pendingPlugins, migPlugin, runLagoon, TestMain), modules/lagoon/migrations_test.go (TestMigrateRunsSystemFilesFirst), modules/lagoon/README.md, 19.8-REVIEW.md (WR-07)</read_first>
|
||||
<action>Per WR-07 (D-21, D-04). Verify that Migrate lists the framework sets and the plugin loop by hand while PendingMigrations reads migrationSets, and that migrate prints its applied lines from the list computed before Migrate ran (so a failure partway prints none). Write TestMigrateAppliedLines and record its RED output (no applied lines when beta fails).
|
||||
|
||||
Make Migrate iterate `migrationSets(gdb, plugins)`, so the startup summary, --pretend, the production question's count, the applied lines and migrate:status --json read the same list. Give each set a label so Migrate's error wording stays as today (`lagoon: migrate system_files: ...`, `backend admin`, `deferred bindings`, `queue`, `<plugin id>`). In the migrate command, compute PendingMigrations before and after Migrate and print the per-set applied lines and the result line from the difference; when Migrate fails, print the lines for what was applied and then return its error. Migrate's signature and its order of sets do not change. README: the migrate description says the applied lines count what was applied, also when a migration fails. Commit: `fix(lagoon): one migration set list for Migrate and PendingMigrations`.</action>
|
||||
<verify>
|
||||
<automated>TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1 go test -count=1 -timeout 10m -run '^(TestMigrateAppliedLines|TestPendingMigrations|TestMigratePretend|TestMigrateStatusJSON|TestMigrateRunsSystemFilesFirst|TestTwoPluginMigrationSetsIsolated)$' -v ./modules/lagoon 2>&1 | grep -c -- '^--- PASS: Test' | grep -qx 6 && go vet ./... && go test -count=1 -short ./...</automated>
|
||||
<fails_when>WR-07: TestMigrateAppliedLines fails (no applied lines on a partial failure, wrong counts, or a success line after a failure), any of the five existing migration tests regress, a test was skipped (fewer than 6 passes means Docker did not run: report NOT RUN, do not count it as passed), or vet or the short suite fail.</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- WR-07: `grep -v '^\s*//' modules/lagoon/migrations.go | grep -c 'attach.Migrations'` prints 1 and `grep -v '^\s*//' modules/lagoon/migrations.go | grep -c 'QueueMigrations(sqlDB)'` prints 1 (the framework sets are listed once, in migrationSets).
|
||||
- WR-07: `grep -c 'migrationSets(gdb, plugins)' modules/lagoon/migrations.go` prints at least 2 (Migrate and PendingMigrations).
|
||||
- `git log --format=%s <plan_head_before>..HEAD` lists the WR-07 fix subject; the SUMMARY records the RED output.
|
||||
</acceptance_criteria>
|
||||
<done>A migration set added once is applied, counted, pretended and reported everywhere, and migrate reports what it really applied.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 7: The gate requires every pinning test, each fix is replayed red without its code, the full suites are green with Docker, the disposition record is updated and IN-01..IN-09 become one pending todo</name>
|
||||
<files>scripts/check-phase19.8.sh, .planning/phases/19.8-console-commands-overhaul/19.8-REVIEW-DISPOSITION.md, .planning/todos/pending/2026-10-22-console-review-info-findings.md, .planning/phases/19.8-console-commands-overhaul/19.8-05-SUMMARY.md</files>
|
||||
<read_first>scripts/check-phase19.8.sh (the required-name arrays and run_go, run_docker), 19.8-REVIEW-DISPOSITION.md (frontmatter and table format, the note on setting dispositions by hand), 19.8-REVIEW.md (IN-01..IN-09), .planning/todos/pending/2026-10-21-quiet-plugin-boot-logs-on-console-commands.md (todo format), this plan's commits (git log plan_head_before..HEAD)</read_first>
|
||||
<action>(1) Gate. Add every new test name to the required arrays in scripts/check-phase19.8.sh with a `# Plan 05 (review fixes)` comment: BONFIRE_TESTS gets TestPrintAboveOrderedNoLoss, TestPrintlnStaysWithItsConsole, TestLiveViewSingleOwner, TestLiveViewPanicUnregisters, TestLiveViewRestoresTerminalOnSignal, TestWithExitCode, TestWrongDirectoryMasksSecrets, TestSanitize, TestRenderErrorStripsControl, TestProcessViewStripsControl, TestTableStripsControl; DIARY_TESTS gets TestDailyRetriesAfterFailure, TestRetiredDailyWriterKeepsNoFile, TestApplyGlobalsRebuildsOnce; SUMMER_TESTS gets TestDelegateExitCodePassthrough, TestAppStreams, TestDelegateStreams; DEV_TESTS gets TestDevStopWaitsForGracefulShutdown; DOCKER_LAGOON_TESTS gets TestMigrateAppliedLines. Use the names the tasks actually committed (record any rename). Leave IN-09's naming of consuming applications alone (it is in the todo). Commit: `test(19.8): gate requires the review fixes' tests`.
|
||||
|
||||
(2) RED replay. For each of the eleven fix commits C, in a temporary detached worktree under the session scratchpad (`git worktree add --detach <dir> C`), reverse-apply C's changes outside test files and Markdown (`git -C <dir> show C -- . ':!*_test.go' ':!*.md' > <dir>.patch && git -C <dir> apply -R <dir>.patch`), run C's pinning test by name and expect it to FAIL (a compile failure counts for the tests of the two new exported identifiers; say so in the table), then `git worktree remove --force <dir>`. The lagoon replay needs Docker. Record per finding: commit, pinning test, RED kind (assertion or compile), result. A pinning test that passes on the reverted code means the test does not pin the fix: stop and fix the test in its own `test(...)` commit before continuing.
|
||||
|
||||
(3) Full verification: `go vet ./...`; `TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1 go test -count=1 ./...` (through `sudo -u golem` if the socket is denied; the known wristband flake TestOIDCSubjectIsRandomSmoke from deferred-items.md may be rerun once and recorded); `go -C examples/hello test -count=1 ./...`; `bash scripts/check-phase19.8.sh go`; `bash scripts/check-phase19.8.sh docs`; `TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1 bash scripts/check-phase19.8.sh docker`. API check: no exported func, method or type declaration line removed between plan_head_before and HEAD in non-test Go files under modules/ unless the identical line is re-added (`comm -23` of the removed and added `func`/`type` lines, filtered to exported names, prints nothing).
|
||||
|
||||
(4) Records. Edit 19.8-REVIEW-DISPOSITION.md by hand: CR-01..CR-03 and WR-01..WR-08 `disposition: fixed` in the frontmatter and `fixed` in the table, with Source `fix <short hash> (19.8-05)`; IN-01..IN-09 `deferred` with Source `todo .planning/todos/pending/2026-10-22-console-review-info-findings.md`; `open:` set to the number of findings still open (0). Create the todo in the existing format: frontmatter `created` (ISO time), `title: Console review info findings (IN-01..IN-09)`, `area: framework`, `severity: minor`, `g15office: pending` (the orchestrator mirrors it; no g15office tool here), `files:` the files the nine findings name; body: one section per finding with its file, the problem and the review's suggested fix, noting that IN-08's fix now belongs in bonfire.Sanitize (Task 5) and that IN-07 should keep diary's new use of bonfire.Sanitize in mind. Commit both planning files together: `docs(19.8): review fixes recorded, info findings captured as a todo`.
|
||||
|
||||
(5) SUMMARY: plan_head_before and plan_head_after, the commits per finding, the RED replay table, the verification results (Docker run or NOT RUN), the API check result, and a Release impact table (finding, commit, module, exported API added, needs require in plugins yes/no, regenerate in apps yes/no). Expected: API added bonfire.WithExitCode and bonfire.Sanitize, no plugin uses them, the generated main is unchanged, so no require bump and no regenerate; any row that differs says why. Add a suggested g15office comment for the orchestrator ("Plan 19.8-05: 11 review findings fixed with pinning tests; info findings tracked in a todo; release v0.4.2 next").</action>
|
||||
<verify>
|
||||
<automated>go vet ./... && go -C examples/hello test -count=1 ./... && bash scripts/check-phase19.8.sh go && bash scripts/check-phase19.8.sh docs && TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1 bash scripts/check-phase19.8.sh docker</automated>
|
||||
<fails_when>vet, the example application's tests, or the gate's go, docs or docker stage fail (a pinning test missing, skipped or red by name, a stale docs identifier, or a Docker test red; with no Docker daemon the docker stage refuses, which is a failure here).</fails_when>
|
||||
<automated>TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1 go test -count=1 ./... && grep -c 'disposition: fixed' .planning/phases/19.8-console-commands-overhaul/19.8-REVIEW-DISPOSITION.md | grep -qx 11 && grep -c 'disposition: deferred' .planning/phases/19.8-console-commands-overhaul/19.8-REVIEW-DISPOSITION.md | grep -qx 9 && test -f .planning/todos/pending/2026-10-22-console-review-info-findings.md</automated>
|
||||
<fails_when>The full suite with Docker fails, the disposition record does not show 11 fixed and 9 deferred findings, or the todo file is missing.</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -cE 'TestPrintAboveOrderedNoLoss|TestDelegateExitCodePassthrough|TestDailyRetriesAfterFailure|TestDevStopWaitsForGracefulShutdown|TestMigrateAppliedLines|TestRenderErrorStripsControl|TestWrongDirectoryMasksSecrets' scripts/check-phase19.8.sh` prints at least 7.
|
||||
- The SUMMARY's RED replay table has eleven rows, each with result "failed on reverted code".
|
||||
- `grep -cE '^(severity: minor|area: framework|g15office: pending)$' .planning/todos/pending/2026-10-22-console-review-info-findings.md` prints 3 and `grep -cE '^## IN-0[1-9]' .planning/todos/pending/2026-10-22-console-review-info-findings.md` prints 9.
|
||||
- The SUMMARY holds the Release impact table and the API check result (no exported declaration removed).
|
||||
- None of this plan's commits (the hashes the SUMMARY lists) includes `.planning/STATE.md` or `.planning/state.json`: for each listed hash, `git show --name-only --format= <hash>` names neither file. Other sessions' edits and commits to them are left alone.
|
||||
</acceptance_criteria>
|
||||
<done>Every review fix is pinned by a gated test that is proven to fail without it, the whole framework suite is green with Docker, the review record says what happened to each finding, and plan 06 has the table it needs to release.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| database, remote services and admin input to the operator's terminal | error text, item names and table cells printed by commands |
|
||||
| command arguments to stderr and CI logs | the wrong-directory hint repeats arguments |
|
||||
| summer to the delegated app binary | streams, terminal, exit code and signals |
|
||||
| long-running processes to the filesystem | daily log files over days of uptime |
|
||||
| framework to shared core plugins and applications | exported APIs and the generated main |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|----------|-------------|-----------------|
|
||||
| T-19.8-26 | Tampering | bonfire error block, process view, table (WR-05) | medium | mitigate | bonfire.Sanitize applied to every printed line, item, error and cell before styling; tests with color forced on prove OSC and CSI sequences are gone |
|
||||
| T-19.8-27 | Information disclosure | wrong-directory hint (WR-08) | medium | mitigate | values of flags named like password, passwd, secret, token or key replaced by *** in the Try line; TestWrongDirectoryMasksSecrets |
|
||||
| T-19.8-28 | Elevation of privilege | summer delegates skip the production question (CR-03) | high | mitigate | the child gets summer's own streams on a terminal, so bonfire.Interactive and ConfirmProceed see a terminal; TestAppStreams pins the choice |
|
||||
| T-19.8-29 | Tampering | --json and -q data captured through summer (CR-03) | medium | mitigate | the child's stderr goes to stderr, never into stdout; TestDelegateStreams |
|
||||
| T-19.8-30 | Repudiation | daily log trail lost after one failed open (WR-04) | medium | mitigate | retry after a minute or on the next date, one warning per date; TestDailyRetriesAfterFailure |
|
||||
| T-19.8-31 | Denial of service | descriptors leaked by replaced daily writers (WR-06) | low | mitigate | retired writers open, append and close per late record; TestRetiredDailyWriterKeepsNoFile |
|
||||
| T-19.8-32 | Denial of service | signal handling in live views (WR-01) | medium | mitigate | the view relays SIGINT and SIGTERM only while it runs, restores the terminal, then re-raises so the default action or the command's own handler applies; both paths tested in a helper process |
|
||||
| T-19.8-33 | Tampering | shared plugin and application contracts | medium | mitigate | additions only (WithExitCode, Sanitize); Task 7's API check finds no removed exported declaration; the generated main is untouched (Task 2 verify) |
|
||||
| T-19.8-SC | Tampering | dependencies | low | accept | this plan adds no module; tests use the standard library, the existing charm modules and testcontainers |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- Task 1 (tracer): CR-01 red then green, -race, three clean runs.
|
||||
- Task 2: WR-02 and WR-01 tests, generated main untouched.
|
||||
- Task 3: bonfire, cmd/summer and internal/dev tests for CR-02, CR-03, WR-03, WR-08; docs checker.
|
||||
- Task 4: diary tests for WR-04 and WR-06 with -race; docs checker.
|
||||
- Task 5: WR-05 tests with color forced on; goldens unchanged; docs checker.
|
||||
- Task 6: WR-07 Docker test plus the existing migration tests.
|
||||
- Task 7: gate names, RED replay of all eleven commits, `go vet ./...`, `go test ./...` with Docker, examples/hello, gate stages go, docs and docker, API check, disposition and todo.
|
||||
- Feedback sampling: after every commit `go vet ./...` and `go test -short ./...`.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- CR-01..CR-03 and WR-01..WR-08 fixed, one commit each, each with a gated pinning test proven red without the fix.
|
||||
- No exported API removed or changed; two additive identifiers documented in the bonfire README and docs.
|
||||
- `go vet ./...`, `go test ./...` with Docker, examples/hello and the gate's go, docs and docker stages green.
|
||||
- 19.8-REVIEW-DISPOSITION.md: 11 fixed, 9 deferred; the IN todo exists with `g15office: pending` for the orchestrator.
|
||||
- The SUMMARY's Release impact table tells plan 06 whether any plugin or application needs a commit.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/19.8-console-commands-overhaul/19.8-05-SUMMARY.md` when done, with plan_head_before and plan_head_after, the commit per finding with its pinning test and RED output, the RED replay table, the verification results (Docker run or NOT RUN), the API check, the Release impact table, the todo path, and the suggested g15office comment for the orchestrator.
|
||||
</output>
|
||||
211
.planning/phases/19.8-console-commands-overhaul/19.8-06-PLAN.md
Normal file
211
.planning/phases/19.8-console-commands-overhaul/19.8-06-PLAN.md
Normal file
@@ -0,0 +1,211 @@
|
||||
---
|
||||
phase: 19.8-console-commands-overhaul
|
||||
plan: 06
|
||||
type: execute
|
||||
wave: 2
|
||||
depends_on: ["19.8-05"]
|
||||
gap_closure: true
|
||||
files_modified:
|
||||
- .planning/phases/19.8-console-commands-overhaul/19.8-06-SUMMARY.md
|
||||
- ../sm-demo-app/plugins/demo/content/ROADMAP.md
|
||||
- ../sm-demo-app/plugins/demo/roadmap/names.go
|
||||
autonomous: false
|
||||
requirements: [D-26]
|
||||
user_setup: []
|
||||
|
||||
estimate:
|
||||
tokens: 60000
|
||||
raw_tokens: 60000
|
||||
tasks: 3
|
||||
confidence: low
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "D-26: nothing is tagged or pushed before the Task 1 reply, which the SUMMARY records verbatim; before the reply no tag points at HEAD."
|
||||
- "On 'release vX.Y.Z' (proposed v0.4.2): after git fetch, origin/master..master holds only plan 05's code commits and planning documents (this phase's and other sessions'); vet, the short suite and the docs checker are green at the commit that gets tagged; an annotated vX.Y.Z tag listing the fixes in user words points at that commit; master is pushed as exactly that commit, then the tag; nothing is force-pushed; the same-day changelog post with user-facing notes under ## Fixed exists and its URL is in the SUMMARY; the token is never printed."
|
||||
- "On 'push-only': the same range check and checks, then master pushed as the tested commit; no tag, no changelog post. On 'hold': nothing leaves this machine."
|
||||
- "Shared plugins get a require bump and applications a regenerate commit only for rows of plan 05's Release impact table that say so (expected: none, because the two API additions are used by no plugin and the generated main did not change)."
|
||||
- "sm-demo-app's roadmap snapshot equals origin/master's .planning/ROADMAP.md, every phase in it has a Polish title and every open phase a Polish summary, in separate sm-demo-app commits (Polish names first, then the snapshot) that are pushed only under 'release' or 'push-only'; when the snapshot already matches, sm-demo-app is not touched."
|
||||
- "`PHASE198_REQUIRE_DOCKER=1 TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1 bash scripts/check-phase19.8.sh --all` ends with 'Phase 19.8 gate passed' (TestTerminalCommands may print NOT RUN)."
|
||||
- "Nothing is deployed in any option."
|
||||
artifacts:
|
||||
- path: ".planning/phases/19.8-console-commands-overhaul/19.8-06-SUMMARY.md"
|
||||
provides: "the release proposal, the reply, the tag, pushed ranges, the changelog URL and the --all stage summary"
|
||||
contains: "## Task 1 reply"
|
||||
key_links:
|
||||
- from: "framework tag vX.Y.Z"
|
||||
to: "https://summercms.io/changelog"
|
||||
via: "scripts/changelog-post.py source and publish in ../sm-summercmsio-app"
|
||||
pattern: "changelog-post.py"
|
||||
- from: "../sm-demo-app/plugins/demo/content/ROADMAP.md"
|
||||
to: "origin/master:.planning/ROADMAP.md"
|
||||
via: "scripts/sync-roadmap.sh with ROADMAP_REF=origin/master; the gate's landing stage checks it"
|
||||
pattern: "sync-roadmap"
|
||||
prohibitions:
|
||||
- statement: "No push or tag before the Task 1 reply; never a force-push; never a deploy."
|
||||
status: unresolved
|
||||
verification: judgment
|
||||
- statement: "Another session's code commit in origin/master..master stops the release: nothing is pushed until the user decides."
|
||||
status: unresolved
|
||||
verification: judgment
|
||||
- statement: "The changelog token is never printed, copied or minted; a missing or refused token stops and asks the user."
|
||||
status: unresolved
|
||||
verification: judgment
|
||||
---
|
||||
|
||||
## Phase Goal
|
||||
|
||||
ROADMAP Goal (not in user-story format; plans 01-04 recorded the decision not to invent one): console commands reach the Artisan bar. This plan (gap closure 2 of 2, wave 2) publishes plan 05's review fixes to the applications' build path: a patch tag with its changelog post, under a go-ahead, then proves the whole phase again with the gate.
|
||||
|
||||
## Spec-less probe fallback
|
||||
|
||||
Spec-less probe fallback skipped: no REQUIREMENTS.md IDs and no SPEC.md (as in plans 01-05). The release follows D-26 (nothing pushed or tagged without the go-ahead) and the procedure of plan 03 Task 7 step (1) and plan 04 Tasks 5-6.
|
||||
|
||||
## Decisions taken in this plan
|
||||
|
||||
- **Same shape as v0.4.1.** The checkpoint and the release copy plan 04 Tasks 5-6: range check after a fetch, annotated tag, push of the exact tagged commit to master, then the tag, then the changelog post. The proposed tag is v0.4.2 unless another tag appeared after v0.4.1, in which case the proposal is the next patch after the newest tag.
|
||||
- **Plugins and applications.** Only rows of plan 05's Release impact table that say "needs require" or "regenerate" lead to commits there; plan 05 expects none. A consumer picks the fixes up with its next framework require bump.
|
||||
- **Demo roadmap snapshot.** The gate's landing stage compares sm-demo-app's `plugins/demo/content/ROADMAP.md` with origin/master's `.planning/ROADMAP.md`. The release push publishes this phase's and other sessions' roadmap changes, which turned the landing stage red after v0.4.1. Task 3 re-syncs it when the check fails, adding Polish names first for any new phase (TestRoadmapPolishTitles), so every sm-demo-app commit is green.
|
||||
|
||||
<objective>
|
||||
Release plan 05's fixes as v0.4.2 under the user's go-ahead, with the same-day changelog post, keep the demo roadmap snapshot in step with what was published, and finish with a green phase gate.
|
||||
|
||||
Purpose: D-26 (nothing published without the go-ahead) and the changelog rule in CLAUDE.md; the fixes only help applications once they are tagged.
|
||||
Output: on release, tag vX.Y.Z on origin and a changelog post; on push-only, master on origin; in every option, a green `--all` run in the SUMMARY.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@~/.claude/msd-core/workflows/execute-plan.md
|
||||
@~/.claude/msd-core/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/phases/19.8-console-commands-overhaul/19.8-05-SUMMARY.md
|
||||
@.planning/phases/19.8-console-commands-overhaul/19.8-04-SUMMARY.md
|
||||
|
||||
Release procedure to repeat: 19.8-03-PLAN.md Task 7 step (1) and 19.8-04-PLAN.md Tasks 5 and 6 (and the "Task 6: published" section of 19.8-04-SUMMARY.md). Changelog: ../sm-summercmsio-app/DEPLOY.md "Posting to the changelog" and the usage header of ../sm-summercmsio-app/scripts/changelog-post.py.
|
||||
|
||||
Concurrency: other sessions commit planning documents (and sometimes code) to summercms.go and may push. Stage by explicit path, never `git add -A`, never touch `.planning/STATE.md` or `.planning/state.json` by hand, no co-author tags. In sm-demo-app commit only the two roadmap paths and check `git status --porcelain` and `git log origin/master..master` there first. Docker tests need `TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1`; if the socket is denied, run through `sudo -u golem`.
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="checkpoint:decision" gate="blocking">
|
||||
<name>Task 1: Go-ahead to publish the review fixes: release v0.4.2, push-only, or hold</name>
|
||||
<read_first>19.8-05-SUMMARY.md (fix commits, RED replay, verification, Release impact table), 19.8-04-SUMMARY.md ("Task 6: published": how v0.4.1 went out), 19.8-04-PLAN.md Task 5 (the checkpoint this one copies), CLAUDE.md (Commits and concurrency, the changelog rule)</read_first>
|
||||
<acceptance_criteria>
|
||||
- Before the checkpoint returns, a draft 19.8-06-SUMMARY.md holds a `## Release proposal` section: the output of `git fetch origin` and of `git log --format='%h %s' origin/master..master`, each commit classified as plan 05 code, planning documents (this phase or another session) or other code (with its paths from `git show --name-only --format= <hash>`); the newest tag (`git tag --sort=-v:refname`, first line) and the proposed tag; plan 05's Release impact table; the result of `ROADMAP_REF=origin/master ../sm-demo-app/scripts/sync-roadmap.sh --check`; and plan 05's verification status (Docker run or NOT RUN).
|
||||
- Before the reply, `T=$(git tag --points-at HEAD) && test -z "$T"` exits 0.
|
||||
- After the reply, the SUMMARY records it verbatim under `## Task 1 reply`, with the chosen tag on a release.
|
||||
</acceptance_criteria>
|
||||
<decision>Publish plan 05's fixes now, and with which patch tag?</decision>
|
||||
<context>Plan 05 fixed the eleven critical and warning findings of the code review, each in its own commit with a gated pinning test, and recorded a Release impact table. Releasing means an annotated patch tag (proposed v0.4.2) on the tested master commit, pushing master as exactly that commit and then the tag, and a same-day changelog post with the fixes under ## Fixed. Pushing master also publishes every unpushed planning-document commit of other sessions listed in the proposal; another session's code commit in the range blocks the release until you decide. Plugins and applications get commits only where the Release impact table says so (expected: none). After the push, sm-demo-app's roadmap snapshot is re-synced if it no longer matches origin/master (Task 3). Nothing is deployed in any option. If the proposal shows another session's code in the range, a red check or a Docker NOT RUN in plan 05, the recommended answer is hold.</context>
|
||||
<options>
|
||||
<option id="release">
|
||||
<name>release <tag> (for example "release v0.4.2")</name>
|
||||
<pros>Applications and plugins can pin the fixes; the changelog shows them the same day; the demo roadmap snapshot follows the published roadmap.</pros>
|
||||
<cons>Another tag and changelog post one day after v0.4.1; publishes other sessions' planning commits.</cons>
|
||||
</option>
|
||||
<option id="push-only">
|
||||
<name>push-only</name>
|
||||
<pros>The fixes are on origin/master for the next release; no tag or post today.</pros>
|
||||
<cons>Tagged consumers keep v0.4.1 and its defects until the next tag.</cons>
|
||||
</option>
|
||||
<option id="hold">
|
||||
<name>hold</name>
|
||||
<pros>Nothing leaves this machine.</pros>
|
||||
<cons>The fixes stay local; verification sees them unpublished.</cons>
|
||||
</option>
|
||||
</options>
|
||||
<resume-signal>Reply "release vX.Y.Z", "push-only" or "hold". "release" and "push-only" also authorise pushing sm-demo-app's Polish-name and roadmap-snapshot commits from Task 3 when they are its only unpushed commits. A release also authorises the plugin require bumps and application regenerate commits that plan 05's Release impact table names (expected none). Nothing is ever force-pushed or deployed.</resume-signal>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Under the Task 1 reply, publish: range check, checks, annotated tag on the tested commit, push master then the tag, same-day changelog post; plugin and application commits only where plan 05's table asks</name>
|
||||
<precondition>19.8-06-SUMMARY.md (draft) records the Task 1 reply under `## Task 1 reply`. On "hold" this task records "release held, nothing published" and stops.</precondition>
|
||||
<files>.planning/phases/19.8-console-commands-overhaul/19.8-06-SUMMARY.md</files>
|
||||
<read_first>19.8-06-SUMMARY.md draft (Release proposal, Task 1 reply), 19.8-05-SUMMARY.md (fix commits and Release impact table), 19.8-03-PLAN.md Task 7 step (1), 19.8-04-SUMMARY.md ("Task 6: published"), ../sm-summercmsio-app/DEPLOY.md ("Posting to the changelog"), ../sm-summercmsio-app/scripts/changelog-post.py (usage header only)</read_first>
|
||||
<action>Per D-26, under the recorded reply. Never force-push, never deploy, never print the changelog token.
|
||||
|
||||
On "release vX.Y.Z":
|
||||
(1) `git fetch origin`; list `git log --format='%h %s' origin/master..master` and classify each commit with `git show --name-only --format= <hash>`: a commit touching only `.planning/` is a planning document and may go out; a commit touching anything else must be one of plan 05's commits listed in its SUMMARY. Any other code commit stops the task: record it in the SUMMARY and ask the user (nothing pushed). Confirm the newest tag is still the one the proposal named; if another tag appeared since, stop and ask (the reply's tag may now be wrong).
|
||||
(2) At master HEAD with a clean tree for this phase's paths (`git status --porcelain`; other sessions' STATE.md and state.json edits are left alone): `go vet ./...`, `go test -count=1 -short ./...` and `go run ./cmd/summer docs:build --check` green. Record the tested commit's hash.
|
||||
(3) `git tag -a vX.Y.Z <tested hash>` with a message: first line `SummerCMS vX.Y.Z`, a blank line, one sentence ("Fixes from the review of the 0.4 console."), then one bullet per fix in user words (no CR/WR IDs, no planning or quick codes): lines printed during spinners and progress bars keep their order and none is lost; Ctrl+C during a spinner or progress bar restores the cursor; a command's output never lands in another command's live view; summer's delegated commands get the terminal (production question, styled output), keep their stderr apart from data and exit with the binary's exit code; summer dev lets serve finish its shutdown; a daily log file that failed to open is retried; replaced log files are closed; error blocks, process views and tables strip terminal control characters; the wrong-directory hint masks password, secret, token and key values; migrate reports what it actually applied (adjust to what plan 05 actually shipped).
|
||||
(4) `git push origin <tested hash>:refs/heads/master` (a fast-forward of exactly the tagged commit, so commits landing meanwhile are not published), then `git push origin vX.Y.Z`; `git ls-remote --exit-code --tags origin vX.Y.Z` exits 0.
|
||||
(5) Changelog, the same day, from ../sm-summercmsio-app: `scripts/changelog-post.py source vX.Y.Z`; write user-facing Markdown notes to a file in the session scratchpad: no H1, a `## Fixed` section with the bullets above (more readable sentences are fine; no planning IDs, no quick codes); then `scripts/changelog-post.py publish vX.Y.Z <notes file> --excerpt "<one line: console fixes for 0.4>"`. If the token file is missing or the token is refused, stop and ask the user. Record the post URL.
|
||||
(6) For each plan 05 Release impact row marked "needs require": in that plugin repository `go get git.golem15.com/golem15/summercms@vX.Y.Z`, `go mod tidy`, vet and short tests, one commit `chore: require SummerCMS vX.Y.Z`, push. For each row marked "regenerate": regenerate main.go, build, `bin/<app> list`, one commit `build: regenerate main for SummerCMS vX.Y.Z`, push. With no such rows record "plugins and applications: nothing to publish".
|
||||
|
||||
On "push-only": steps (1) and (2), then `git push origin <tested hash>:refs/heads/master`; no tag, no changelog post.
|
||||
|
||||
Record in the SUMMARY: the range classification after the fetch, the tested hash, the tag object, the pushed range, the changelog URL, and every plugin or application commit (or "none").</action>
|
||||
<verify>
|
||||
<automated>T="${RELEASE_TAG:?set RELEASE_TAG to the tag named in the Task 1 reply}" && git ls-remote --exit-code --tags origin "$T" && git fetch origin && git merge-base --is-ancestor "$T^{commit}" origin/master && test "$(git cat-file -t "$T")" = tag</automated>
|
||||
<fails_when>On release: the newest reachable tag is not on origin, the tagged commit is not on origin/master, or the tag is lightweight instead of annotated. On "push-only" or "hold" this command is not run; the SUMMARY records the pushed range or "nothing published".</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- On release: `git ls-remote --exit-code --tags origin vX.Y.Z` exits 0, `git cat-file -t vX.Y.Z` prints `tag`, and the SUMMARY holds the changelog post URL (https://summercms.io/changelog/...).
|
||||
- The SUMMARY lists every published repository with its pushed range, or "nothing published" on hold.
|
||||
- The SUMMARY's range classification lists no "other code" commit, or records that the task stopped because of one.
|
||||
</acceptance_criteria>
|
||||
<done>The fixes are published exactly as the user chose, with a tag and changelog post on release, and nothing else of another session's code went out.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 3: The demo roadmap snapshot matches the published roadmap (Polish names for new phases first, then the snapshot, in sm-demo-app, only when the check fails), and the whole phase gate is green with Docker required</name>
|
||||
<files>../sm-demo-app/plugins/demo/roadmap/names.go, ../sm-demo-app/plugins/demo/content/ROADMAP.md, .planning/phases/19.8-console-commands-overhaul/19.8-06-SUMMARY.md</files>
|
||||
<read_first>../sm-demo-app/scripts/sync-roadmap.sh (header: ROADMAP_REF, --check), ../sm-demo-app/plugins/demo/roadmap/names.go (phaseText entries, the client-name denylist), ../sm-demo-app/plugins/demo/roadmap/*_test.go (TestRoadmapPolishTitles, TestRoadmapNoClientNames), `git -C ../sm-demo-app log --oneline -5` (the earlier "Polish name for roadmap phase" and "sync the ROADMAP.md snapshot" commits as the pattern), scripts/check-phase19.8.sh (run_landing), 19.8-04-SUMMARY.md ("Gate after the release": the red landing stage after v0.4.1)</read_first>
|
||||
<action>(1) Conditional roadmap sync. `git fetch origin` in summercms.go, then `ROADMAP_REF=origin/master ../sm-demo-app/scripts/sync-roadmap.sh --check`. If it prints ok, record "snapshot in step, sm-demo-app untouched" and go to (2). Otherwise, in sm-demo-app: check `git status --porcelain` and `git log --oneline origin/master..master` (another session's uncommitted roadmap edits or unpushed commits there: stop and report); run `ROADMAP_REF=origin/master scripts/sync-roadmap.sh` to write the snapshot, then `go test -count=1 -short -run '^(TestRoadmapParse|TestRoadmapDeckOrder|TestRoadmapNoClientNames|TestRoadmapPolishTitles)$' ./plugins/demo/roadmap`. If TestRoadmapPolishTitles names a phase without a Polish title (or an open phase without a Polish summary), add its `phaseText` entry to plugins/demo/roadmap/names.go in the style of the existing entries: a short Polish title and, for open phases, a one-line Polish summary of the roadmap goal, naming no client application (TestRoadmapNoClientNames enforces the denylist). Commit so that every commit is green: first names.go alone (`feat(demo): Polish name for roadmap phase <N>`, one commit naming all new phases if several), then the snapshot alone (`chore: sync the ROADMAP.md snapshot (summercms <short hash of origin/master>)`), staged by path. Re-run the four roadmap tests. Push sm-demo-app only when the Task 1 reply was "release" or "push-only" and `git log origin/master..master` there lists only these commits; on "hold" leave them local and say so. Never deploy.
|
||||
|
||||
(2) Gate. Run `PHASE198_REQUIRE_DOCKER=1 TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1 bash scripts/check-phase19.8.sh --all` (through `sudo -u golem` if the Docker socket is denied) and paste its stage summary into the SUMMARY. A red stage is diagnosed and fixed through the owning repository's normal flow (a framework defect goes back to a plan 05 style fix commit and blocks completion; a sibling repository's unrelated pre-existing failure is recorded with evidence that it also fails without this plan's commits) and the gate re-run until green.
|
||||
|
||||
(3) Record in the SUMMARY a suggested g15office comment for the orchestrator (no g15office tool here): on release "Phase 19.8 review fixes released as SummerCMS vX.Y.Z (changelog: <URL>); phase gate green; ready for /msd-verify-work", otherwise the matching hold or push-only wording.</action>
|
||||
<verify>
|
||||
<automated>ROADMAP_REF=origin/master ../sm-demo-app/scripts/sync-roadmap.sh --check && LOG="${TMPDIR:?set TMPDIR to the session scratchpad}/check-phase19.8-06.log" && PHASE198_REQUIRE_DOCKER=1 TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1 bash scripts/check-phase19.8.sh --all >"$LOG" 2>&1 && tail -20 "$LOG" && grep -q '^Phase 19.8 gate passed$' "$LOG" && ! grep -qE '^(FAIL|NOT RUN docker)' "$LOG"</automated>
|
||||
<fails_when>The demo snapshot differs from origin/master's ROADMAP.md, the gate exits non-zero or lacks "Phase 19.8 gate passed", a stage line starts with FAIL, or docker did not run (PHASE198_REQUIRE_DOCKER=1 turns a missing daemon into a failure).</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- The SUMMARY holds the --all stage summary with every stage PASS (TestTerminalCommands may be NOT RUN).
|
||||
- When sm-demo-app was changed: `git -C ../sm-demo-app log --format=%s -2` shows the Polish-name commit (when one was needed) before the snapshot commit, `git -C ../sm-demo-app show --name-only --format= HEAD` names only plugins/demo/content/ROADMAP.md, and on release or push-only `git -C ../sm-demo-app log --oneline origin/master..master` prints nothing.
|
||||
- When sm-demo-app was not changed, the SUMMARY says the snapshot was already in step.
|
||||
</acceptance_criteria>
|
||||
<done>The published roadmap and the demo deck agree, and one green gate run proves the whole phase after the release.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| this machine to git.golem15.com | pushes and tags publish framework code, other sessions' planning commits and sm-demo-app commits |
|
||||
| this machine to summercms.io | the changelog post, authenticated by a token file |
|
||||
| roadmap to the public demo deck | phase titles and summaries shown to prospective clients |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|----------|-------------|-----------------|
|
||||
| T-19.8-34 | Repudiation | release publication | high | mitigate | blocking checkpoint before any push; after a fetch every commit in origin/master..master is classified by its paths, and any code commit not listed by plan 05 stops the release |
|
||||
| T-19.8-35 | Tampering | pushed master | medium | mitigate | push exactly the tested, tagged commit as a fast-forward (`<hash>:refs/heads/master`); never force-push; vet, short tests and the docs checker green at that commit |
|
||||
| T-19.8-36 | Information disclosure | changelog token | medium | mitigate | changelog-post.py reads the token file itself; the token is never printed, copied or minted; a missing or refused token stops and asks the user |
|
||||
| T-19.8-37 | Information disclosure | demo roadmap deck | medium | mitigate | new Polish names follow the existing entries and TestRoadmapNoClientNames rejects client application names before the snapshot commit |
|
||||
| T-19.8-SC | Tampering | dependencies | low | accept | no module is added; plugin requires move only to this framework's own tag, and only for rows of plan 05's table |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- Task 1: proposal written before the checkpoint; no tag on HEAD before the reply; reply recorded verbatim.
|
||||
- Task 2: on release, tag annotated and on origin, its commit on origin/master, changelog URL recorded; on push-only, pushed range recorded; on hold, nothing published.
|
||||
- Task 3: demo snapshot equals origin/master's roadmap; --all green with Docker required.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- The user's choice is executed exactly: release (tag, push, changelog), push-only or hold; nothing deployed or force-pushed.
|
||||
- No other session's code went out with the release.
|
||||
- The demo roadmap snapshot and Polish names are current, in separate sm-demo-app commits, only when needed.
|
||||
- `bash scripts/check-phase19.8.sh --all` is green with Docker required.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/19.8-console-commands-overhaul/19.8-06-SUMMARY.md` when done, with the release proposal, the Task 1 reply verbatim, the range classification, the tested hash, the tag and tag object, the pushed ranges, the changelog post URL, plugin and application commits (or "none"), the sm-demo-app commits (or "snapshot already in step"), the --all stage summary, and the suggested g15office comment for the orchestrator.
|
||||
</output>
|
||||
Reference in New Issue
Block a user