finish: leftover-file trailer gives a count with no filenames and no tree location, under a "no need to re-verify" closure #309

New issue

Closed

opened 2026-07-20 20:43:28 +00:00 by hexajon · 0 comments

hexajon commented

2026-07-20 20:43:28 +00:00

(Migrated from codeberg.org)

Observed on a dir= finish against a foreign primary (worktree finishes have the same shape):

Merged PR #1 → main; main not checked out - foreign primary is read-only (...) · 1 uncommitted file(s) (not in the PR) - done; authoritative final state, no need to re-verify.

Reading that, I could not tell where the leftover file was - in the worktree? the primary? lost at cleanup? captured somewhere? The behavior underneath is fine (the file is left in place in the tree the finish touched; declared-files staging per ADR 0024, and worktree remove refuses a dirty tree per ADR 0029), but the output does not let the caller conclude any of that.

Three problems compound:

Count, not names. The pre-finish refusal path lists uncommitted files by name; the success trailer only counts them. Naming is precedented and cheap (cap the list, "+N more" beyond it).
No tree location. checkWorkingTree correctly inspects r.dir - the tree the finish actually touched - but the message never says which tree the count refers to. In exactly the worktree and dir= cases, "the working tree" plausibly means three different places.
The closure marker suppresses the natural follow-up. The same line ends with the ADR 0004 "no need to re-verify" closure, steering the caller away from the one command (git status) that would answer "which file, where". The trailer exists so the caller never shells git status (legacy tracker 1579) - self-confirmation only works if the line carries enough information to act on.

Proposed shape: name the files up to a small cap, and when the finish ran against a worktree or dir= target, state the path and that the files were left in place, e.g.

· 1 uncommitted file (not in the PR), left in place at /path/to/target: reference/demo.html

Code pointers: treeStateSuffix in internal/service/workflow/finish_result.go (composes the trailer from leftoverN only); checkWorkingTree in internal/service/workflow/finish_return.go (already knows the dir, discards everything but the count).

Observed on a `dir=` finish against a foreign primary (worktree finishes have the same shape): ``` Merged PR #1 → main; main not checked out - foreign primary is read-only (...) · 1 uncommitted file(s) (not in the PR) - done; authoritative final state, no need to re-verify. ``` Reading that, I could not tell where the leftover file was - in the worktree? the primary? lost at cleanup? captured somewhere? The behavior underneath is fine (the file is left in place in the tree the finish touched; declared-files staging per ADR 0024, and worktree remove refuses a dirty tree per ADR 0029), but the output does not let the caller conclude any of that. Three problems compound: 1. **Count, not names.** The pre-finish refusal path lists uncommitted files by name; the success trailer only counts them. Naming is precedented and cheap (cap the list, "+N more" beyond it). 2. **No tree location.** `checkWorkingTree` correctly inspects `r.dir` - the tree the finish actually touched - but the message never says which tree the count refers to. In exactly the worktree and `dir=` cases, "the working tree" plausibly means three different places. 3. **The closure marker suppresses the natural follow-up.** The same line ends with the ADR 0004 "no need to re-verify" closure, steering the caller away from the one command (`git status`) that would answer "which file, where". The trailer exists so the caller never shells `git status` (legacy tracker 1579) - self-confirmation only works if the line carries enough information to act on. Proposed shape: name the files up to a small cap, and when the finish ran against a worktree or `dir=` target, state the path and that the files were left in place, e.g. ``` · 1 uncommitted file (not in the PR), left in place at /path/to/target: reference/demo.html ``` Code pointers: `treeStateSuffix` in `internal/service/workflow/finish_result.go` (composes the trailer from `leftoverN` only); `checkWorkingTree` in `internal/service/workflow/finish_return.go` (already knows the dir, discards everything but the count).

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

sixfold-space/madtea#309

No description provided.

Rows
Columns