Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions .githooks/pre-push
Original file line number Diff line number Diff line change
Expand Up @@ -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:"
Expand All @@ -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:"
Expand Down
61 changes: 51 additions & 10 deletions internal/cli/cmdship/artefact_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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 '.', '!', '?', ':', ')', ']', '"', '\'', '`', '|', '>':
Expand All @@ -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
Expand Down
54 changes: 54 additions & 0 deletions internal/cli/cmdship/artefact_validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading