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
45 changes: 45 additions & 0 deletions BREAKING.md
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,51 @@ See `docs/SECURITY.md` for the full vulnerability disclosure and patching policy

---

## v1.10.0 — quality-gate verdicts and checkpoint evidence

**Feature:** three-state `Verdict`, evidence-backed checkpoints, pre-checkpoint hooks activated
**Package:** `internal/cli/cmdship`

### What changed

| Area | Old | New |
|---|---|---|
| `HookResult.Passed` | `bool` | **removed** — replaced by `HookResult.Verdict` |
| Gate outcome | pass / fail | `VerdictUnknown` / `VerdictPass` / `VerdictFail` |
| `Checkpoint` | status only | gains `Evidence []Evidence` |
| `PhasePreCheckpoint` | declared, never invoked | runs before every checkpoint (advisory) |

`cmdship` is an `internal/` package, so nothing outside this module can import it and no external consumer can break. It is recorded here anyway: the gate that demanded this entry is right to treat the module's own API as worth tracking, and a policy that is waived the first time it is inconvenient stops being a policy.

### Why `Passed bool` had to go

A bool forced every gate that *could not check* — artefact missing, tool absent, config it cannot parse — to answer either pass or fail. Authors picked pass, because failing a build over something that is not the user's fault is obviously wrong. So "I did not verify this" and "I verified this and it is fine" became the same value.

`VerdictUnknown` is the **zero value** deliberately: a handler that forgets to set a verdict yields "unverified", not a false pass.

### Migration (for anyone patching or forking this package)

```go
// before
return HookResult{Passed: true}
return HookResult{Passed: false, Message: "..."}

// after
return gatePass()
return gateFail("...")
return gateUnknown("...") // when the gate could not check — this case is new
```

The third constructor is the point of the change. A handler that previously returned `Passed: true` because its artefact was missing must now return `gateUnknown` with a reason.

### Runtime behaviour changes (no action required)

- A checkpoint reaching `ok` with **no independent evidence** is downgraded to `warning` and annotated `UNVERIFIED[…]`. It is never escalated to `fail`, and `res.Ready` keys on `fail` — so no working pipeline breaks.
- `self-review-gate` now actually executes. It is advisory: it annotates and downgrades to `warning`, never fails. `HookConfig.Strict` remains the opt-in for making hook findings blocking.
- `.forge/specs/<slug>/<checkpoint>.md` markers gain an `Evidence:` line.

---

## v1.7.0 — LLM-first rearchitecture (piped-output migration)

**Feature:** `internal/llmresponse` + `forge ship --human` + 10 MCP tools
Expand Down
43 changes: 43 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,49 @@ All notable changes to forge will be documented in this file. Format follows [Ke

## [Unreleased]

## [1.10.0] — 2026-08-07 — Quality gates that can fail, admit ignorance, and cite their evidence

### Added

- **M1 — a checkpoint may no longer report green on forge's own say-so.** `Checkpoint.Status` is a plain string that any of twenty-odd code paths could set to `"ok"`, and nothing ever required the code setting it to say *why*. Because forge is the actor in all of those paths, the thing being implicitly asserted was always true — "I wrote the file", "I ran the generator", "I completed the step". Those are facts about forge's behaviour, not about whether the change is sound. Every bug in this family has had the same shape:

| forge verified | what mattered |
|---|---|
| `spec.md` written | `spec.md` is complete |
| test file written | the test will ever run |
| the gate returned | the gate examined anything |
| the checkpoint ran | the checkpoint verified anything |

Checkpoints now carry `Evidence`, tagged by source. `SourceExternalTool` (a scanner, test runner, linter or git was asked and answered) and `SourceReadBack` (forge re-read the artefact from disk and re-validated it, judging it as it would judge a stranger's) count as independent. `SourceForgeClaim` — forge asserting its own success — is recorded, reported, and never sufficient on its own.

Enforcement is at the reporting boundary, not via a private field or a mandatory setter: routing every assignment through `Pass(evidence)` would just invite `SourceForgeClaim` boilerplate that satisfies the compiler and nothing else. A checkpoint reaching `"ok"` with no independent evidence is downgraded to `"warning"` and annotated `UNVERIFIED[…]`.

**This cannot break a working pipeline.** The claim a downgrade makes is "nobody checked" — a reason to withhold confidence, not to block a release. `res.Ready` keys on `"fail"`, which this policy never produces (`TestEvidencePolicy_NeverBlocksARun`).

- **The gates turned out to be the evidence system already.** A hook returning `VerdictPass` has read an artefact off disk and re-validated it — that *is* read-back evidence, it was simply never recorded as the basis for the status. Wiring it up gave most checkpoints real evidence without touching a single `Status = "ok"` line. Only `VerdictUnknown` contributes nothing, which is exactly what M3 was for.

- **Checkpoint marker files now record the basis, not just the outcome.** `.forge/specs/<slug>/<checkpoint>.md` gains an `Evidence:` line. The marker is the durable record — what `forge ship status` reads and what someone opens months later to ask "was this actually checked?" — and recording a status without its basis left that question unanswerable. Evidence is also emitted in `forge ship --json` so CI can audit a green run instead of taking the word `ok` for it.

- **`PhasePreCheckpoint` hooks now actually run.** `self-review-gate` was declared, listed in `defaultHooks()`, documented in the package header, covered by tests — and had never executed, because `runWithOptions` only ever called `runHooks` for the two later phases. Counting it among forge's quality gates was inaccurate from the day it was written.

It is wired in as **advisory**: findings annotate the checkpoint and downgrade `ok` to `warning`, but do not fail it. Every project using forge has been shipping without this gate, so switching it on as a blocker would break builds over artefacts that were acceptable yesterday. `HookConfig.Strict` is the opt-in for making it stop a run, exactly as with every other hook. All three pre-checkpoint calls are now routed through one `beforeCheckpoint()` helper alongside the snapshot and the agent-mode checkpoint marker, so a future checkpoint cannot pick up two of the three and silently miss the third.

- **`Verdict` — quality gates now have three outcomes, not two.** `HookResult.Passed bool` is replaced by `Verdict` (`VerdictUnknown` / `VerdictPass` / `VerdictFail`).

A bool forced every gate that *could not check* — artefact missing, tool not installed, config it cannot parse — to answer either "pass" or "fail". Gate authors almost always picked pass, because failing a build over something that is not the user's fault is obviously wrong. So "I did not verify this" and "I verified this and it is fine" became the same value, and the caller could not tell them apart. Every instance of that in forge has been the same bug: a green checkpoint standing on a check that never ran.

`VerdictUnknown` is deliberately the **zero value** — a handler that forgets to set a verdict yields "unverified", which is honest, rather than falling into a false pass. Pinned by `TestVerdict_UnknownIsTheZeroValue`, because if `VerdictPass` ever became iota's first value, every incomplete handler in the codebase would silently start reporting success.

Unverified gates annotate the checkpoint `UNVERIFIED[…]` and never escalate it. They are suppressed on an already-failed checkpoint, which has a real error to show.

- **Every "could not check" path now says so.** Nine gates returned `Passed: true` when their artefact was missing (`// no spec file yet`, `// no ADR file → nothing to check`, …). All now return `VerdictUnknown` with a reason naming the missing file. Enforced going forward by `TestGateMutation_NoGateReportsCleanOnAnEmptyProject`: no gate may report clean on a project where none of its artefacts exist.

### Fixed

- **`spec-code-alignment-gate` reported PASS on projects it had never examined** — the gap the M2 mutation table found on its first run. `auditSlug()` returns early when `spec.md` is absent, skipping every check, and the gate fell through to pass. `forge ship --from=code` on a project whose spec was never written got a green alignment gate that verified nothing. It now returns `VerdictUnknown`: the gate did not find the project acceptable, it found it *unexaminable*, and those are different facts.

- **`self-review-gate` reported PASS after scanning zero files.** Same shape, found by the same test.

### Added

- **Gate mutation testing (`gate_mutation_test.go`) — tests for the quality gates themselves.** Every other test in the suite asks "does the pipeline behave correctly?"; these ask whether the gates *check anything at all*. Each of the 13 hooks in `defaultHooks()` is now run against a **known-bad** fixture it must reject, and a known-good one it must accept. A gate that cannot fail is not a gate — and that is not hypothetical: 1.8.1's reachability checker compiled `\\.` from config source meaning `\.`, matched nothing in any real path, and reported the dead zone it was written to catch as fine. It was green, it was wrong, and nothing in the suite would have noticed, because every existing test asked only whether *good* input passed.
Expand Down
173 changes: 173 additions & 0 deletions internal/cli/cmdship/evidence.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
// Copyright 2024 The Forge Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// evidence.go — M1: a checkpoint may not report "ok" on forge's own say-so.
//
// # The failure this closes
//
// M2 asked whether the gates check anything. M3 gave them a way to say "I
// could not check". This asks the question one level up: **what is a green
// checkpoint standing on?**
//
// `Checkpoint.Status` is a plain string that any of twenty-odd code paths can
// set to "ok". Nothing has ever required the code setting it to say why. And
// because forge is the actor in every one of those paths, the thing it is
// implicitly asserting is always true — "I wrote the file", "I ran the
// generator", "I completed the step". Those are facts about forge's own
// behaviour, not about whether the change is sound. Every bug in this family
// has had the same shape:
//
// spec.md written ≠ spec.md is complete
// test file written ≠ the test will ever run
// the gate returned ≠ the gate examined anything
// the checkpoint ran ≠ the checkpoint verified anything
//
// # What counts as evidence
//
// Evidence is an observation about the world that did not come from forge
// asserting its own success. Two sources qualify:
//
// SourceExternalTool — something outside forge was asked and answered:
// a test runner, a scanner, git, a linter.
// SourceReadBack — forge re-read what landed on disk and re-validated
// it, judging the artefact as it would judge a
// stranger's, rather than trusting the value it just
// held in memory.
//
// And one does not:
//
// SourceForgeClaim — "I did the thing." Recorded, reported, and never
// sufficient on its own to make a checkpoint green.
//
// # How it is enforced
//
// Not by making Status private or routing every assignment through a setter —
// that invites a SourceForgeClaim boilerplate that satisfies the compiler and
// nothing else. It is enforced where it matters, at the reporting boundary: a
// checkpoint that reaches "ok" with no qualifying evidence is downgraded to
// "warning" and annotated UNVERIFIED.
//
// The downgrade is deliberately not a failure. The claim being made is "nobody
// checked", which is a reason to withhold confidence, not a reason to block a
// release. `res.Ready` is unaffected — it turns on "fail", not "warning" — so
// this cannot break a pipeline that was working.
package cmdship

import (
"fmt"
"strings"
)

// EvidenceSource says where an observation came from, which is the only thing
// that makes it worth anything.
type EvidenceSource string

const (
// SourceExternalTool — a tool outside forge was asked and answered.
SourceExternalTool EvidenceSource = "external-tool"
// SourceReadBack — forge re-read the artefact from disk and re-validated it.
SourceReadBack EvidenceSource = "read-back"
// SourceForgeClaim — forge asserting its own success. Never sufficient.
SourceForgeClaim EvidenceSource = "forge-claim"
)

// Independent reports whether this source counts toward a green checkpoint.
//
// The name is the point: evidence is only worth having when it is independent
// of the party being assessed, and forge assessing forge is not.
func (s EvidenceSource) Independent() bool {
return s == SourceExternalTool || s == SourceReadBack
}

// Evidence is one observation supporting a checkpoint's status.
type Evidence struct {
// Source is where the observation came from.
Source EvidenceSource `json:"source"`
// Claim is what it establishes, in the checkpoint's own terms
// (e.g. "spec.md has an Acceptance Criteria section with Given/When/Then").
Claim string `json:"claim"`
// Observed is the raw finding, kept short — the count, the exit code, the
// gate name. It is what a reviewer would ask for when the claim is
// disputed.
Observed string `json:"observed,omitempty"`
}

// String renders one evidence entry for a checkpoint detail line.
func (e Evidence) String() string {
if e.Observed == "" {
return string(e.Source) + ": " + e.Claim
}
return string(e.Source) + ": " + e.Claim + " (" + e.Observed + ")"
}

// AddEvidence records an observation on the checkpoint.
func (cp *Checkpoint) AddEvidence(source EvidenceSource, claim, observed string) {
if cp == nil {
return
}
cp.Evidence = append(cp.Evidence, Evidence{Source: source, Claim: claim, Observed: observed})
}

// HasIndependentEvidence reports whether anything other than forge's own
// say-so supports this checkpoint.
func (cp *Checkpoint) HasIndependentEvidence() bool {
if cp == nil {
return false
}
for _, e := range cp.Evidence {
if e.Source.Independent() {
return true
}
}
return false
}

// EvidenceSummary renders the independent evidence for a detail line, or "" if
// there is none.
func (cp *Checkpoint) EvidenceSummary() string {
if cp == nil {
return ""
}
var parts []string
for _, e := range cp.Evidence {
if e.Source.Independent() {
parts = append(parts, e.String())
}
}
if len(parts) == 0 {
return ""
}
return strings.Join(parts, "; ")
}

// applyEvidencePolicy is the enforcement point.
//
// A checkpoint that reached "ok" without a single independent observation is
// reporting confidence it has not earned, so it is downgraded to "warning" and
// told to say so. Statuses other than "ok" are left alone: a checkpoint that
// already failed has a real finding to report, and one already at "warning" is
// not claiming anything that needs qualifying.
func applyEvidencePolicy(cp *Checkpoint) {
if cp == nil || cp.Status != "ok" {
return
}
if cp.HasIndependentEvidence() {
return
}
cp.Status = "warning"
cp.Detail += fmt.Sprintf(
" | UNVERIFIED[%s reported ok with no independent evidence: nothing outside forge "+
"confirmed this, and no artefact was re-read and re-validated]",
strings.ToLower(cp.Name))
}
Loading
Loading