diff --git a/.githooks/pre-push b/.githooks/pre-push index c3d83f7..1b4aa5f 100755 --- a/.githooks/pre-push +++ b/.githooks/pre-push @@ -90,8 +90,27 @@ echo "━━━ forge pre-push quality gate (13 checks) ━━━" echo "" # ── 1. formatting ──────────────────────────────────────────────────────────── +# `gofmt -s -l .` / `goimports -l .` walk every .go file under the given +# directory, including ones under a dot- or underscore-prefixed path segment +# (e.g. .forge/scratch/plan3.go, tests/fixtures/.../_scratch_idea.go). Unlike +# `go build`/`go vet`/`go list`, which silently skip such paths per Go's own +# package-discovery convention (`go help packages`: "directories with names +# starting with '.' or '_' are ignored"), gofmt/goimports have no such rule +# and simply try to parse every .go file they find. tests/fixtures/hygiene- +# corpus intentionally contains syntactically-invalid .go scratch-file +# fixtures (DEV-M0-32) that forge's own hygiene scanner is tested against — +# gofmt/goimports choking on them is a false positive, not a real formatting +# issue, and previously blocked every push in the repo unconditionally. +# GOFILES applies the same dot/underscore-prefix exclusion Go's own tools +# already use, so this stage checks exactly the .go files the rest of this +# hook (go vet/build/test, golangci-lint, all of which use `./...`) checks. +mapfile -t GOFILES < <(git ls-files '*.go' | grep -vE '(^|/)[._][^/]*($|/)') echo "[1/12] formatting (gofmt + goimports)" -FMT_OUT=$(gofmt -s -l . 2>/dev/null || true) +if [[ ${#GOFILES[@]} -eq 0 ]]; then + FMT_OUT="" +else + FMT_OUT=$(gofmt -s -l "${GOFILES[@]}" 2>/dev/null || true) +fi if [[ -n "$FMT_OUT" ]]; then fail "gofmt -s" echo " Files needing formatting:" @@ -102,7 +121,11 @@ else fi if need goimports; then - IMP_OUT=$(goimports -l . 2>/dev/null || true) + if [[ ${#GOFILES[@]} -eq 0 ]]; then + IMP_OUT="" + else + IMP_OUT=$(goimports -l "${GOFILES[@]}" 2>/dev/null || true) + fi if [[ -n "$IMP_OUT" ]]; then fail "goimports" echo " Files with import issues:" diff --git a/internal/cli/cmdship/artefact_validate.go b/internal/cli/cmdship/artefact_validate.go index 308db9b..cb465f3 100644 --- a/internal/cli/cmdship/artefact_validate.go +++ b/internal/cli/cmdship/artefact_validate.go @@ -80,10 +80,18 @@ func looksComplete(s string) bool { return false // unbalanced fenced code block — cut off mid-block } - last := lastNonEmptyLine(trimmed) - if last == "" { + lines := strings.Split(trimmed, "\n") + lastIdx := -1 + for i := len(lines) - 1; i >= 0; i-- { + if strings.TrimSpace(lines[i]) != "" { + lastIdx = i + break + } + } + if lastIdx == -1 { return false } + last := strings.TrimRight(lines[lastIdx], " \t\r") if strings.HasSuffix(last, "```") { return true } @@ -114,6 +122,17 @@ func looksComplete(s string) bool { // happens inside an inline code span, which is the shape actually // observed to recur. } + if isWrappedListContinuation(lines, lastIdx) { + return true // ends on an indented continuation line of an earlier + // list item — e.g. a checklist item whose text wraps onto a + // hanging-indented second line. isListItem above only recognizes + // the bullet's own first physical line, so a document that + // legitimately ends mid-continuation-line was previously always + // flagged truncated (root-caused 2026-09-13, dogfooding on + // ai-marketing-platfrom: a complete, well-formed spec answer + // submitted via `forge agent submit` was silently discarded for + // exactly this reason). + } runes := []rune(last) switch runes[len(runes)-1] { case '.', '!', '?', ':', ')', ']', '"', '\'', '`', '|', '>': @@ -136,16 +155,38 @@ func isListItem(line string) bool { i+1 < len(line) && line[i+1] == ' ' } -// lastNonEmptyLine returns the last non-blank line of s, or "" if s has none. -func lastNonEmptyLine(s string) string { - lines := strings.Split(s, "\n") - for i := len(lines) - 1; i >= 0; i-- { - l := strings.TrimRight(lines[i], " \t\r") - if l != "" { - return l +// isWrappedListContinuation reports whether the line at idx is an indented +// continuation of an earlier Markdown list item on the same bullet — e.g. a +// checklist item whose text wraps onto a second, hanging-indented line: +// +// - [x] Migration applied and verified against local Postgres test DB (real +// RPC call: idempotency + P0002 unknown-subscription path confirmed) +// +// isListItem only recognizes the bullet's own first physical line (the one +// starting with "- "/"1. "/etc.); walking backward over consecutively +// indented lines finds that first line so the same "normal, intentional +// document ending" rationale isListItem already applies here too. +func isWrappedListContinuation(lines []string, idx int) bool { + indent := leadingWhitespace(lines[idx]) + if indent == "" { + return false // not indented — cannot be a hanging continuation + } + for i := idx - 1; i >= 0; i-- { + l := lines[i] + if strings.TrimSpace(l) == "" { + return false // blank line breaks the block — not a continuation + } + if len(leadingWhitespace(l)) >= len(indent) { + continue // still inside the same wrapped block — keep walking back } + return isListItem(strings.TrimSpace(l)) } - return "" + return false +} + +// leadingWhitespace returns the leading run of spaces/tabs in s. +func leadingWhitespace(s string) string { + return s[:len(s)-len(strings.TrimLeft(s, " \t"))] } // generateWithValidation runs invoke, validates the result (J8/J9/J11), and diff --git a/internal/cli/cmdship/artefact_validate_test.go b/internal/cli/cmdship/artefact_validate_test.go index 6ef8b9c..f070f5b 100644 --- a/internal/cli/cmdship/artefact_validate_test.go +++ b/internal/cli/cmdship/artefact_validate_test.go @@ -103,6 +103,60 @@ func TestLooksComplete_ListItemWithUnclosedInlineCode_Truncated(t *testing.T) { } } +// TestLooksComplete_EndsOnWrappedListContinuation_NotTruncated is the +// regression test for the incident that motivated this check: root-caused +// 2026-09-13 (dogfooding on ai-marketing-platfrom, forge ship agent-mode) — +// 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). isListItem only recognizes a +// bullet's own first line, so the continuation line — ending in ordinary +// prose with no terminal punctuation — was misclassified as truncated, and +// the checkpoint silently discarded the answer and kept re-serving a stub. +func TestLooksComplete_EndsOnWrappedListContinuation_NotTruncated(t *testing.T) { + t.Parallel() + cases := []string{ + "# Spec\n\n## DoD\n- [x] Migration applied and verified against local Postgres test DB (real\n RPC call: idempotency + P0002 unknown-subscription path confirmed)", + "# Spec\n\n## Tasks\n1. Implement the RPC, following the exact pattern already used by\n the sibling wrapper functions in this file", + "# Spec\n\n## Out of Scope\n- Automatic retry of the failed charge, since no such flow exists\n anywhere in the app yet", + } + for _, c := range cases { + if !looksComplete(c) { + t.Errorf("content ending on a wrapped list-item continuation line should not be flagged truncated: %q", c) + } + } +} + +// TestLooksComplete_IndentedProseNotUnderListItem_StillTruncated guards +// against over-broadening the fix above: an indented line that is NOT +// actually a continuation of a list item (its nearest less-indented +// ancestor is plain prose, not a bullet) must still be judged on its own +// terminal shape, same as before. +func TestLooksComplete_IndentedProseNotUnderListItem_StillTruncated(t *testing.T) { + t.Parallel() + // The indented second line continues a plain paragraph, not a list item — + // isWrappedListContinuation must not fire, so this stays truncated exactly + // as it was before the fix. + truncated := "# Spec\n\nThe acceptance criteria for this feature are as follows\n and they continue onto this indented line and" + if looksComplete(truncated) { + t.Error("an indented continuation of plain prose (not a list item) should still be flagged truncated") + } +} + +// TestLooksComplete_WrappedListContinuationCutMidWord_StillTruncated guards +// the other direction: a wrapped continuation line that itself is cut off +// mid-word inside an unclosed inline code span must still be flagged +// truncated — isWrappedListContinuation only overrides the terminal-shape +// fallback at the very end of looksComplete, never the unclosed-backtick +// check that already runs earlier on every line unconditionally. +func TestLooksComplete_WrappedListContinuationCutMidWord_StillTruncated(t *testing.T) { + t.Parallel() + truncated := "# Spec\n\n- [x] See the handler at\n `src/app/api/billing/payment-" + if looksComplete(truncated) { + t.Error("a wrapped continuation line cut off inside an unclosed inline code span should still be flagged truncated") + } +} + func TestLooksComplete_EndsMidSentence_Truncated(t *testing.T) { t.Parallel() // No terminal punctuation, not a heading, not a list item, not a closing fence.