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
13 changes: 13 additions & 0 deletions .githooks/pre-push
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,19 @@

set -euo pipefail

# ── isolate from git's own hook environment ─────────────────────────────────
# git exports GIT_DIR (and, depending on version and worktree layout,
# GIT_WORK_TREE / GIT_INDEX_FILE / GIT_COMMON_DIR / ...) to hooks. Every child
# process below inherits them, so ANY test that shells out to git in a temp dir
# (`git init`, `git commit`, `git branch -M`, `git checkout -b`, ...) silently
# operates on THIS repository instead: it commits test fixtures onto the
# branch being pushed, force-renames branches (clobbering `main`), and rewrites
# .git/config (core.bare, user.name/email). Seen on a real push from a linked
# worktree. Cleared here so every stage — and everything they spawn — resolves
# the repository from the working directory, as it does outside a hook.
unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_PREFIX GIT_COMMON_DIR \
GIT_OBJECT_DIRECTORY GIT_ALTERNATE_OBJECT_DIRECTORIES GIT_NAMESPACE

# ── emergency bypass ────────────────────────────────────────────────────────
if [[ "${SKIP_PRE_PUSH:-}" == "1" ]]; then
echo "⚠ SKIP_PRE_PUSH=1: all pre-push checks skipped — use only in emergencies"
Expand Down
23 changes: 23 additions & 0 deletions docs/verbs/ship.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ forge ship auth/email arch
# Run only the QA agent checkpoint
forge ship auth/email qa-verify

# Stop after arch so the spec and ADR can be reviewed before any code exists
forge ship auth/email --until arch

# Then continue from the next checkpoint
forge ship auth/email --from test

# Skip the QA agent (no test runner configured)
forge ship auth/email --skip-checkpoint qa-verify

Expand Down Expand Up @@ -112,8 +118,25 @@ forge ship --tag v1.2.3
| `--no-branch` | false | Do not create or switch to a feature branch; run on current branch |
| `--tag <version>` | — | After a clean pipeline, tag and push a release |
| `--skip-checkpoint <name>` | — | Skip a named checkpoint (e.g. `qa-verify` when no test runner is configured) |
| `--until <checkpoint>` | — | Stop after the named checkpoint (e.g. `--until arch` runs spec and arch, then stops so you can review before test/breakdown/code). Repeat it on every `--agent-mode` continuation; resume later with `--from <next>` |
| `--strict-testing` | false | Enforce the 4-stage testing pipeline (local → pre-push/CI → staging → production) as a blocking `qa-verify` gate instead of an advisory reminder — see below |

## Multi-repository projects (`related_repos`)

A spec or ADR for one repository often cites files that live in a sibling
repository (a web app and its agent service, an API and its SDK). List those
siblings in `forge.yml` and forge will (a) resolve cited paths against them
instead of flagging every cross-repo citation as "may be hallucinated", and
(b) tell the model they exist, with their detected stacks, in the workspace
context it is given.

```yaml
related_repos:
- ../ai-agent-system # relative to the project root, or an absolute path
```

Entries that do not exist are ignored.

## The 4-stage testing pipeline (`--strict-testing`)

`forge ship`'s checkpoints prove a feature was specced, coded, and passed
Expand Down
20 changes: 19 additions & 1 deletion internal/cli/cmdship/arch.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,10 @@ func checkArch(root, description, specName string, pipe *LLMPipe, dryRun bool) C
"3) Data Model & Consistency — data entities, migration strategy, consistency model; " +
"4) Non-Functional Requirements — p99 latency, throughput, availability SLOs; " +
"5) Security Threat Model — STRIDE threats, mitigations, auth/authz boundaries; " +
"6) Deployment & Observability — topology, health checks, metrics, tracing, DR plan. " +
"6) Deployment & Observability — topology, health checks, metrics, tracing, DR plan; " +
"7) Alternatives Considered — at least two genuinely different options you rejected " +
"(label them Alternative A, Alternative B, ...), each with the reason it lost; the ADR " +
"quality gate fails an ADR that evaluates fewer than two. " +
"End with a concise ADR summary: Status, Context, Decision, Consequences. " +
"After the ADR, append a fenced ```yaml block containing a valid OpenAPI 3.1.0 contract " +
"for all API endpoints introduced by this feature. " +
Expand Down Expand Up @@ -287,6 +290,21 @@ func checkArch(root, description, specName string, pipe *LLMPipe, dryRun bool) C
archContent, openapiContent = extractOpenAPIBlock(generated, description)
// P1: run parallel role debate and append reviewer concerns.
debateSuffix := runParallelArchDebate(pipe, description, archContent, 300)
// Agent mode: a role's turn is owed. Nothing may be written yet.
// Writing arch.md here with placeholder "(no concerns raised)"
// sections made the next run hit the "arch.md already exists"
// idempotency shortcut above, so the debate never resumed and the
// answer the host agent submitted was silently discarded (seen on
// a real run: all six roles read "(no concerns raised)" although
// one had been answered). Return before the write; on the next
// run the recorded generate answer and the answered roles replay,
// and the next unanswered role is asked.
if b := pipe.Bridge(); b != nil && b.Paused() {
cp.Status = "ok"
cp.Detail = "awaiting host-agent turn for arch-parallel-debate — run: forge agent prompt"
cp.AgentPaused = true
return cp
}
if debateSuffix != "" {
archContent += debateSuffix
}
Expand Down
141 changes: 141 additions & 0 deletions internal/cli/cmdship/arch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ import (
"os"
"path/filepath"
"strings"
"sync"
"testing"

"github.com/teragrid/forge/internal/agentbridge"
"github.com/teragrid/forge/internal/llmprovider"
)

Expand Down Expand Up @@ -706,3 +708,142 @@ func TestCheckArch_LLM_SupabaseRPCResponse(t *testing.T) {
t.Errorf("expected supabase-rpc style detected from extracted openapi.yaml, got %q", style)
}
}

// TestCheckArch_AgentMode_AsksEveryRoleAndKeepsTheAnswers is a regression test
// for the agent-mode arch debate silently collapsing to a single turn.
//
// checkArch used to run the debate, notice nothing, and write arch.md straight
// away even though the bridge had just paused on the first role's turn. The
// file therefore contained "(no concerns raised)" for every role, and the next
// run hit the "arch.md already exists" idempotency shortcut — the debate never
// resumed, and the answer the host agent had just submitted was discarded.
// Seen on a real run: six roles, one answered, all six read "(no concerns
// raised)" in the final document.
//
// Contract pinned here: while any role's turn is owed the checkpoint pauses and
// writes nothing; once every role has answered, arch.md holds every answer and
// no placeholder.
func TestCheckArch_AgentMode_AsksEveryRoleAndKeepsTheAnswers(t *testing.T) {
t.Parallel()
root := t.TempDir()
const slug = "debate-feat"

specDir := filepath.Join(root, ".forge", "specs", slug)
if err := os.MkdirAll(specDir, 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(specDir, "spec.md"),
[]byte("# Spec: debate feat\n\n## What\nSomething.\n"), 0o600); err != nil {
t.Fatal(err)
}
archPath := filepath.Join(specDir, "arch.md")

bridge, err := agentbridge.Open(root, agentbridge.DefaultSession)
if err != nil {
t.Fatalf("open bridge: %v", err)
}
pipe := newLLMPipeAgent(root, bridge)

const adr = "# Architecture Decision Record: Debate Feat\n\n" +
"## 1. Component Topology\n\nOne handler.\n\n" +
"## 2. API Contracts\n\nREST.\n\n" +
"## ADR Summary\n\n- Status: Proposed.\n- Decision: build it.\n\n" +
"```yaml\nopenapi: 3.1.0\ninfo:\n title: Debate Feat\n version: 1.0.0\npaths: {}\n```\n"

roles := defaultArchRoles()
debateTurns := 0
maxIterations := len(roles) + 6
done := false
for i := 0; i < maxIterations && !done; i++ {
cp := checkArch(root, slug, slug, pipe, false)
if !cp.AgentPaused {
done = true
break
}
if _, statErr := os.Stat(archPath); statErr == nil {
t.Fatalf("iteration %d: arch.md was written while a host-agent turn was still owed", i)
}
pending, ok := bridge.Pending()
if !ok {
t.Fatalf("iteration %d: checkpoint paused but no turn is pending", i)
}
answer := adr
if pending.Operation == "arch-parallel-debate" {
debateTurns++
answer = fmt.Sprintf("concern-from-role-%d: check the boundary", debateTurns)
}
if _, ferr := bridge.Fulfil(answer); ferr != nil {
t.Fatalf("iteration %d: fulfil %s: %v", i, pending.Operation, ferr)
}
}
if !done {
t.Fatalf("checkArch never completed within %d iterations", maxIterations)
}
if debateTurns != len(roles) {
t.Fatalf("host agent was asked %d debate turn(s), want one per role (%d)", debateTurns, len(roles))
}

data, err := os.ReadFile(archPath)
if err != nil {
t.Fatalf("arch.md missing after completion: %v", err)
}
doc := string(data)
for n := 1; n <= len(roles); n++ {
if want := fmt.Sprintf("concern-from-role-%d", n); !strings.Contains(doc, want) {
t.Errorf("arch.md lost the answer %q", want)
}
}
if strings.Contains(doc, "(no concerns raised)") {
t.Errorf("arch.md contains a placeholder although every role answered:\n%s", doc)
}
}

// TestCheckArch_PromptAsksForAlternatives pins the agreement between the arch
// generation prompt and the adr-quality-gate. The gate fails an ADR that
// evaluates fewer than two alternatives, but the prompt used to list six
// sections and never mention alternatives — so a model that followed the
// prompt to the letter tripped forge's own gate on the first try.
func TestCheckArch_PromptAsksForAlternatives(t *testing.T) {
t.Parallel()
root := t.TempDir()
slug := slugify("prompt alternatives")
dir := filepath.Join(root, ".forge", "specs", slug)
if err := os.MkdirAll(dir, 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(dir, "spec.md"), []byte("# Spec\n"), 0o600); err != nil {
t.Fatal(err)
}

var (
mu sync.Mutex
systems []string
)
mock := &llmprovider.MockProvider{
Fn: func(req *llmprovider.Request) (*llmprovider.Response, error) {
mu.Lock()
systems = append(systems, req.SystemPrompt)
mu.Unlock()
return mockResponse("# Architecture\n\n## 1. Component Topology\n\nOne service.\n"), nil
},
}
_ = checkArch(root, "prompt alternatives", "", mockPipe(root, mock), false)

mu.Lock()
defer mu.Unlock()
found := false
for _, s := range systems {
if strings.Contains(s, "Structure the document with these sections") {
found = true
if !strings.Contains(s, "Alternatives Considered") {
t.Errorf("arch generation prompt must ask for an Alternatives Considered section; got:\n%s", s)
}
if !strings.Contains(s, "Consequences") {
t.Errorf("arch generation prompt must ask for Consequences; got:\n%s", s)
}
}
}
if !found {
t.Fatal("arch generation system prompt was never sent to the provider")
}
}
74 changes: 72 additions & 2 deletions internal/cli/cmdship/artefact_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import (
"path/filepath"
"regexp"
"strings"

"gopkg.in/yaml.v3"
)

// validateArtefact strips conversational preamble from raw and reports
Expand Down Expand Up @@ -240,6 +242,73 @@ func generateWithValidation(invoke func() (string, bool, error)) (content string
// ordinary prose sentences don't get flagged.
var filePathRefPattern = regexp.MustCompile("`((?:[A-Za-z0-9_.-]+/)+[A-Za-z0-9_.-]+\\.[A-Za-z0-9]{1,6})`")

// loadRelatedRepos returns the absolute roots of the sibling repositories a
// project declares in forge.yml:
//
// related_repos:
// - ../ai-agent-system
//
// Many projects are several repositories (a web app and its agent service, an
// API and its SDK), and a spec or ADR for one legitimately cites files that
// live in another. Without this, the unverified-file-reference check could only
// ever look inside the current worktree and flagged every cross-repo citation
// as "may be hallucinated". Relative entries resolve against root. Entries that
// do not exist are dropped; an unreadable or absent forge.yml yields none.
func loadRelatedRepos(root string) []string {
if root == "" {
return nil
}
data, err := os.ReadFile(filepath.Join(root, "forge.yml"))
if err != nil {
return nil
}
var doc struct {
RelatedRepos []string `yaml:"related_repos"`
}
if err := yaml.Unmarshal(data, &doc); err != nil {
return nil
}
var out []string
for _, r := range doc.RelatedRepos {
r = strings.TrimSpace(r)
if r == "" {
continue
}
if !filepath.IsAbs(r) {
r = filepath.Join(root, filepath.FromSlash(r))
}
r = filepath.Clean(r)
if fi, statErr := os.Stat(r); statErr == nil && fi.IsDir() {
out = append(out, r)
}
}
return out
}

// pathExistsInRepos reports whether the repo-relative candidate exists under
// root or any related repo. A candidate may also be prefixed with a related
// repo's directory name ("ai-agent-system/src/x.py"), the way a spec cites a
// file in a sibling repo, in which case the prefix is stripped for that repo.
func pathExistsInRepos(root string, related []string, candidate string) bool {
rel := filepath.FromSlash(candidate)
if _, err := os.Stat(filepath.Join(root, rel)); err == nil {
return true
}
for _, repo := range related {
if _, err := os.Stat(filepath.Join(repo, rel)); err == nil {
return true
}
prefix := filepath.Base(repo) + "/"
if strings.HasPrefix(candidate, prefix) {
stripped := filepath.FromSlash(strings.TrimPrefix(candidate, prefix))
if _, err := os.Stat(filepath.Join(repo, stripped)); err == nil {
return true
}
}
}
return false
}

// findUnverifiedFileReferences scans generated Markdown for backtick-wrapped
// file-path-shaped references and returns the ones that do not exist on disk
// under root, deduplicated and in first-seen order.
Expand All @@ -259,6 +328,7 @@ func findUnverifiedFileReferences(root, content string) []string {
}
seen := make(map[string]bool)
var unverified []string
related := loadRelatedRepos(root)
for _, m := range filePathRefPattern.FindAllStringSubmatch(content, -1) {
candidate := m[1]
if seen[candidate] {
Expand All @@ -270,7 +340,7 @@ func findUnverifiedFileReferences(root, content string) []string {
if strings.HasPrefix(candidate, ".forge/") {
continue
}
if _, err := os.Stat(filepath.Join(root, filepath.FromSlash(candidate))); os.IsNotExist(err) {
if !pathExistsInRepos(root, related, candidate) {
unverified = append(unverified, candidate)
}
}
Expand All @@ -292,7 +362,7 @@ func appendUnverifiedPathsWarning(root, content string) string {
if !strings.HasSuffix(content, "\n") {
b.WriteString("\n")
}
b.WriteString("\n---\n\n> **⚠ Unverified file references (automated check):** the following paths mentioned above were not found in this repository — verify before relying on them, they may be hallucinated:\n")
b.WriteString("\n---\n\n> **⚠ Unverified file references (automated check):** the following paths mentioned above were not found in this repository or its forge.yml related_repos — verify before relying on them, they may be hallucinated:\n")
for _, p := range unverified {
b.WriteString("> - `" + p + "`\n")
}
Expand Down
Loading
Loading