fix(ship): looksComplete no longer flags wrapped list-item continuations as truncated - #46
Merged
Merged
Conversation
…ons as truncated
Root cause (dogfooding on ai-marketing-platfrom, forge ship --agent-mode,
2026-09-13): a complete, well-formed spec.md answer was submitted via
`forge agent submit`, but its last physical line was an indented
continuation of a "- [x] ..." checklist item — the bullet's text wrapped
onto a second, hanging-indented line, e.g.:
- [x] Migration applied and verified against local Postgres test DB (real
RPC call: idempotency + P0002 unknown-subscription path confirmed)
isListItem only recognizes a bullet's own first physical line (the one
starting with "- "/"1. "/etc.), so the continuation line — ending in
ordinary prose with no terminal punctuation, heading, or list marker of its
own — was misclassified as truncated by looksComplete. The checkpoint then
silently discarded the answer and kept re-serving a stub on every
subsequent `forge ship --agent-mode` re-entry, with no visible error beyond
the "spec review truncated/incomplete after retry" digest line.
Fix: looksComplete now also checks whether the last line is an indented
continuation of an earlier list item (isWrappedListContinuation) by walking
backward over consecutively-indented lines to find the bullet's own first
line. Same rationale as the existing isListItem terminal-shape check — this
is a normal, intentional document ending, not a truncation signal.
Guarded against over-broadening in both directions:
- an indented continuation of plain prose (not under an actual list item)
is still judged on its own terminal shape, unchanged from before
- a wrapped continuation line that is itself cut off mid-word inside an
unclosed inline code span is still flagged truncated (the unclosed-
backtick check runs unconditionally, earlier in the function)
Verified against the exact real-world content that triggered the incident.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ixed fixture files Discovered while pushing the looksComplete fix in this same session: stage 1 (`gofmt -s -l .` / `goimports -l .`) walks every .go file under the repo root, including tests/fixtures/hygiene-corpus/'s deliberately-invalid Go scratch-file fixtures (DEV-M0-32) — e.g. .forge/scratch/plan3.go and scratch-files/_scratch_idea.go, which are intentionally not valid Go source since they exist only to test forge's own hygiene/scratch-file scanner against realistic-looking junk. Unlike gofmt/goimports, `go build`/`go vet`/`go list ./...` already skip these paths, because Go's own package-discovery convention ignores any directory or file whose name starts with "." or "_" (see `go help packages`). gofmt/goimports have no equivalent rule and simply try to parse every .go file they're given, so this stage failed unconditionally for every contributor on every push — the exact case makes it hard to notice locally if a previous formatting/import issue is masking it, or if SKIP_PRE_PUSH is already habitual for other reasons. Fix: build the file list via `git ls-files` filtered through the same dot/underscore-prefix exclusion Go's own tools already apply, and pass that explicit list to gofmt/goimports instead of a bare directory — matching what every other stage in this hook (go vet/build/test, golangci-lint, all of which use `./...`) already effectively checks. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
forge ship --agent-mode, 2026-09-13): a complete, well-formedspec.mdanswer was submitted viaforge agent submit, but its last physical line was an indented continuation of a- [x] ...checklist item (the bullet's text wrapped onto a second, hanging-indented line).isListItemonly recognizes a bullet's own first physical line, solooksCompletemisclassified the continuation line as truncated, and the checkpoint silently discarded the real answer, re-serving a stub on every subsequentforge ship --agent-modere-entry.looksCompletenow also recognizes an indented continuation of an earlier list item (isWrappedListContinuation), by walking backward over consecutively-indented lines to find the bullet's own first line — same "normal, intentional document ending" rationale the existingisListItemcheck already uses. Guarded in both directions: plain indented prose (not under a list item) is unaffected, and a continuation line cut off mid-word inside an unclosed inline code span is still flagged truncated (the existing unclosed-backtick check runs first, unconditionally)..githooks/pre-pushstage 1 (gofmt -s -l ./goimports -l .) chokes ontests/fixtures/hygiene-corpus's intentionally-invalid Go scratch-file fixtures (.forge/scratch/plan3.go,_scratch_idea.go) — unlikego build/go vet/go list ./..., which already skip dot/underscore-prefixed paths per Go's own package-discovery convention, gofmt/goimports have no such rule. This blocked every push in the repo through the real hook. Fixed by building the file list viagit ls-filesfiltered through the same dot/underscore-prefix exclusion, so this stage checks exactly the same files the rest of the hook (./...-based stages) already does.Test plan
TestLooksComplete_EndsOnWrappedListContinuation_NotTruncated(3 cases: checklist item, ordered list, unordered list), plus two false-positive guards (TestLooksComplete_IndentedProseNotUnderListItem_StillTruncated,TestLooksComplete_WrappedListContinuationCutMidWord_StillTruncated)looksCompletego build ./...,go vet ./...,golangci-lint run ./...— cleango test ./...— full repo suite green.githooks/pre-pushgate end-to-end — all 13 stages passed, including the QA real-command suite and ship dry-run scenarios🤖 Generated with Claude Code