golem15/summercms
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(19.8): re-review fixes recorded, IN-10..IN-13 and the exit-130 suggestion deferred

Browse Source
This commit is contained in:
Jakub Zych
2026-10-22 04:10:42 +02:00
parent 0691d26f1f
commit a48afbd11e
2 changed files with 76 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

56
.planning/phases/19.8-console-commands-overhaul/19.8-REVIEW-DISPOSITION.md
View File

@@ -5,51 +5,51 @@ titles: json
findings:
- id: WR-09
severity: warning
disposition: open
disposition: fixed
title: "Lines printed through a live view after its context is cancelled are silently dropped"
- id: WR-10
severity: warning
disposition: open
disposition: fixed
title: "A delegate killed by a signal prints its full argv, including a --password value, and exits 1"
- id: WR-11
severity: warning
disposition: open
disposition: fixed
title: "The wrong-directory hint leaves secret values that start with \"-\" unmasked"
- id: WR-12
severity: warning
disposition: open
disposition: fixed
title: "migrate:status --json still drops the framework sets that migrate applies"
- id: WR-13
severity: warning
disposition: open
disposition: fixed
title: "migrate reports failure after a successful Migrate when the follow-up read fails"
- id: WR-14
severity: warning
disposition: open
disposition: fixed
title: "Process's plain path prints item names without Sanitize"
- id: IN-10
severity: info
disposition: open
disposition: deferred
title: "console.Error during a live view writes straight to the terminal and tears the view"
- id: IN-11
severity: info
disposition: open
disposition: deferred
title: "A blank line printed during a live view is dropped"
- id: IN-12
severity: info
disposition: open
disposition: deferred
title: "Signal relay edge cases: a signal during view start-up, and Windows"
- id: IN-13
severity: info
disposition: open
disposition: deferred
title: "A delegate's stderr always goes to os.Stderr, bypassing the root's error writer"
- id: IN-14
severity: info
disposition: open
disposition: fixed
title: "The ErrReported doc comment still says the exit code is 1"
- id: IN-15
severity: info
disposition: open
disposition: fixed
title: "runLive builds the bubbletea program before it knows it may run"
- id: CR-01
severity: critical
@@ -131,7 +131,7 @@ findings:
severity: info
disposition: deferred
title: "The phase gate names consuming applications"
open: 12
open: 0
total: 32
recorded: 2026-10-22T01:19:33.337Z
---
@@ -140,18 +140,18 @@ recorded: 2026-10-22T01:19:33.337Z
| Finding | Severity | Disposition | Source |
|---------|----------|-------------|--------|
| WR-09 | warning | open | - |
| WR-10 | warning | open | - |
| WR-11 | warning | open | - |
| WR-12 | warning | open | - |
| WR-13 | warning | open | - |
| WR-14 | warning | open | - |
| IN-10 | info | open | - |
| IN-11 | info | open | - |
| IN-12 | info | open | - |
| IN-13 | info | open | - |
| IN-14 | info | open | - |
| IN-15 | info | open | - |
| WR-09 | warning | fixed | fix fce232d (quick 261022-4z8) |
| WR-10 | warning | fixed | fix 17c722d (quick 261022-4z8); the exit-130 suggestion is deferred to todo .planning/todos/pending/2026-10-22-console-review-info-findings.md |
| WR-11 | warning | fixed | fix 707372b (quick 261022-4z8) |
| WR-12 | warning | fixed | fix 57a849d (quick 261022-4z8) |
| WR-13 | warning | fixed | fix 0691d26 (quick 261022-4z8) |
| WR-14 | warning | fixed | fix 48e13f9 (quick 261022-4z8) |
| IN-10 | info | deferred | todo .planning/todos/pending/2026-10-22-console-review-info-findings.md |
| IN-11 | info | deferred | todo .planning/todos/pending/2026-10-22-console-review-info-findings.md |
| IN-12 | info | deferred | todo .planning/todos/pending/2026-10-22-console-review-info-findings.md |
| IN-13 | info | deferred | todo .planning/todos/pending/2026-10-22-console-review-info-findings.md |
| IN-14 | info | fixed | docs 7c15095 (quick 261022-4z8) |
| IN-15 | info | fixed | fix 1ac75ad (quick 261022-4z8) |
| CR-01 | critical | fixed | fix b077dc3 (19.8-05) (not in the current review) |
| CR-02 | critical | fixed | fix 323413c (19.8-05) (not in the current review) |
| CR-03 | critical | fixed | fix 9c89705 (19.8-05) (not in the current review) |
@@ -159,10 +159,10 @@ recorded: 2026-10-22T01:19:33.337Z
| WR-02 | warning | fixed | fix 86e4e27 (19.8-05) (not in the current review) |
| WR-03 | warning | fixed | fix fc7706d (19.8-05) (not in the current review) |
| WR-04 | warning | fixed | fix c586c6f (19.8-05) (not in the current review) |
| WR-05 | warning | fixed | fix a9025c0 (19.8-05) (not in the current review) |
| WR-05 | warning | fixed | fix a9025c0 (19.8-05) (not in the current review); remainder WR-14 (fix 48e13f9) |
| WR-06 | warning | fixed | fix 39677f1 (19.8-05) (not in the current review) |
| WR-07 | warning | fixed | fix 020fe83 (19.8-05) (not in the current review) |
| WR-08 | warning | fixed | fix 9b62285 (19.8-05) (not in the current review) |
| WR-07 | warning | fixed | fix 020fe83 (19.8-05) (not in the current review); remainder WR-12 (fix 57a849d) |
| WR-08 | warning | fixed | fix 9b62285 (19.8-05) (not in the current review); remainder WR-11 (fix 707372b) |
| IN-01 | info | deferred | todo .planning/todos/pending/2026-10-22-console-review-info-findings.md (not in the current review) |
| IN-02 | info | deferred | todo .planning/todos/pending/2026-10-22-console-review-info-findings.md (not in the current review) |
| IN-03 | info | deferred | todo .planning/todos/pending/2026-10-22-console-review-info-findings.md (not in the current review) |

50
.planning/todos/pending/2026-10-22-console-review-info-findings.md
View File

@@ -1,6 +1,6 @@
---
created: 2026-10-22T00:43:48.000Z
title: Console review info findings (IN-01..IN-09)
title: Console review info findings (IN-01..IN-13) and the delegate signal exit code
area: framework
severity: minor
g15office: pending
@@ -19,11 +19,14 @@ files:
- modules/bouncer/turnstile.go
- modules/wristband/oidc_token.go
- scripts/check-phase19.8.sh
- modules/bonfire/live.go
- modules/bonfire/output.go
- cmd/summer/runtime.go
---
## Context
The 19.8 code review (`.planning/phases/19.8-console-commands-overhaul/19.8-REVIEW.md`, v0.3.1..v0.4.1) found 3 critical issues, 8 warnings and 9 info findings. Plan 19.8-05 fixed the critical issues and the warnings. The nine info findings below are deferred to this todo. Line numbers are the review's; locate the code by symbol.
The 19.8 code review (`.planning/phases/19.8-console-commands-overhaul/19.8-REVIEW.md`, v0.3.1..v0.4.1) found 3 critical issues, 8 warnings and 9 info findings. Plan 19.8-05 fixed the critical issues and the warnings. The nine info findings below are deferred to this todo. Line numbers are the review's; locate the code by symbol. The re-review of those fixes (same file, gap-closure range dd12382..0ebed68) found six warnings and six info findings; quick task 261022-4z8 fixed the warnings, IN-14 and IN-15, and deferred IN-10..IN-13 and WR-10's exit-code suggestion to this todo (the sections after IN-09).
## IN-01: A forced-color console drops HideOnTTY records even when stderr is not a terminal
@@ -96,3 +99,46 @@ The 19.8 code review (`.planning/phases/19.8-console-commands-overhaul/19.8-REVI
**Problem:** The script hard-codes `sm-demo-app`, `sm-bm-app`, `sm-grzybyfunkcjonalne-app` and `sm-summercmsio-app`, and splits a consumer's name into two strings to get past the name check. CLAUDE.md says framework code never names a consuming application. Earlier phase gates do the same, but this framework repository is published.
**Suggested fix:** Read the sibling paths from environment variables only, with no defaults, or move the cross-repository stages to a gate outside the framework repository.
## IN-10: console.Error during a live view writes straight to the terminal and tears the view
**File:** `modules/bonfire/output.go` (`console.Error`, `emit`)
**Problem:** `Error` writes to `c.err`, and `emit` reroutes only writes to `c.out`. When stderr and stdout are the same terminal (the usual case), an `out.Error(...)` from inside `Spinner`, `Progress` or `Process` lands mid-frame. A pty run showed the spinner glyph left on the error line (`⠹ ✗ stderr-error-line`), with the renderer continuing on the next row. diary's console records on stderr do go above the view through `PrintAbove`, so the two stderr paths behave differently.
**Suggested fix:** In `emit`, also reroute `w == c.err` when the console owns the running view and `c.err` is a terminal (`printAbove(c, s)`).
## IN-11: A blank line printed during a live view is dropped
**File:** `modules/bonfire/live.go` (`printAbove`, the `strings.TrimSuffix` of the block)
**Problem:** `out.Println()` gives `"\n"`. `TrimSuffix` turns that into `""`, and bubbletea's `insertAbove` returns early for an empty string (cursed_renderer.go in bubbletea v2.1.0). `printAbove` still reports `true`, so the blank separator line is lost. It is printed when no view runs.
**Suggested fix:** Send `" "` (or `"\u200b"`) for an empty block, or skip the trim when the block is only a newline.
## IN-12: Signal relay edge cases: a signal during view start-up, and Windows
**File:** `modules/bonfire/live.go` (`runLive`, `watchSignals`, `raise`)
**Problem:** This comes from reading bubbletea v2.1.0; the reviewer did not reproduce it.
1. **Start-up race.** `prog.Run` starts on its own goroutine just before the relay is registered. If a signal arrives before `Run` has created and started its renderer, `Kill` runs `shutdown(true)` while `p.renderer` is still nil. `shutdownOnce` then makes `Run`'s own `shutdown(killed)` a no-op after it has started the renderer and hidden the cursor, so the cursor stays hidden.
2. **Windows.** On Windows, `p.Signal(os.Interrupt)` fails, so `raise` calls `os.Exit(130)`. That skips the command's own handler (for example, `summer dev` stopping its serve child) and its deferred cleanup.
**Suggested fix:** Before killing, wait until the program reports it has started (for example, have the wrapped model's `Init` close a `ready` channel). On platforms where `raise` cannot deliver the signal, return without exiting when another handler is registered.
## IN-13: A delegate's stderr always goes to os.Stderr, bypassing the root's error writer
**File:** `cmd/summer/runtime.go` (`appStreams`)
**Problem:** Off a terminal, the child's stdout goes through `out`, but its stderr goes to the real `os.Stderr`, not to the error stream of the root that ran the command. For a summer root built with `NewRootIO(…, errW)` (tests, embedding), the app binary's error block cannot be captured. `TestDelegateExitCodePassthrough` therefore cannot check that the binary's own block was printed.
**Suggested fix:** Pass the root's error writer through (for example, a `bonfire.ErrWriter(out)` accessor), defaulting to `os.Stderr`.
## WR-10 follow-up: exit code 128+n for a delegate killed by a signal
**File:** `cmd/summer/runtime.go` (`runAppCmd`)
**Problem:** Plan 19.8-05 specified exit code 1 for an app binary that dies by a signal, and quick task 261022-4z8 kept it while masking the argv in summer's error block (WR-10). The re-review points out that Ctrl+C is the normal way to stop `summer serve` or `summer queue:work`: since the WR-01 relay re-raises SIGINT, a child that dies by the signal produces an ERROR block and exit 1, where shells and CI expect 130 (128+SIGINT).
**Suggested fix:** Map signal deaths to 128+n: treat SIGINT and SIGTERM as already reported (`bonfire.Reported(bonfire.WithExitCode(err, 128+n))`, no block), and give other signals exit 128+n with summer's masked block (`argmask.Join(argv)`). `docs/console/introduction.md` documents exit code 1 for a signal death and must change with it, as must `TestDelegateExitCodePassthrough`'s signal subtests.