diff --git a/cmd/orchestra-worker/phase_test.go b/cmd/orchestra-worker/phase_test.go index d47fee3..a756e24 100644 --- a/cmd/orchestra-worker/phase_test.go +++ b/cmd/orchestra-worker/phase_test.go @@ -155,7 +155,7 @@ func TestPhaseRequestCarriesTheSealedArtifact(t *testing.T) { w.tasks["task"] = task w.sessions["task"] = herdr.Session{PaneID: "pane", Worktree: wt, Phase: string(domain.WorkPhaseResearch)} - sealed, err := workphase.Encode(workphase.Research{Findings: []workphase.Finding{{Claim: "c", Evidence: "e"}}}) + sealed, err := workphase.Encode(workphase.Research{Findings: []workphase.Finding{{ID: "r1", Confidence: workphase.Fact, Claim: "c", Evidence: "e"}}}) if err != nil { t.Fatal(err) } diff --git a/internal/agentctx/agentctx.go b/internal/agentctx/agentctx.go index 524899a..1b59c6c 100644 --- a/internal/agentctx/agentctx.go +++ b/internal/agentctx/agentctx.go @@ -430,7 +430,10 @@ func renderSealed(in Input) string { if research != nil { b.WriteString("\n## Accepted research\n\n") for _, f := range research.Findings { - fmt.Fprintf(&b, "- %s (evidence: %s)\n", collapse(f.Claim), collapse(f.Evidence)) + // The id is printed so a plan can cite "research:" and a reader + // can resolve it. Plan seal validation checks every citation + // against this same artifact. + fmt.Fprintf(&b, "- [%s] %s: %s (evidence: %s)\n", f.ID, f.Confidence, collapse(f.Claim), collapse(f.Evidence)) } for _, c := range research.Code { fmt.Fprintf(&b, "- relevant: %s (%s)\n", collapse(c.Path), collapse(c.Why)) diff --git a/internal/integration/acefca_test.go b/internal/integration/acefca_test.go index bb99fe1..da07e8c 100644 --- a/internal/integration/acefca_test.go +++ b/internal/integration/acefca_test.go @@ -230,7 +230,7 @@ func advance(t *testing.T, s *store.Store, project registry.Project, taskID stri func sealedResearch(t *testing.T) []byte { t.Helper() b, err := workphase.Encode(workphase.Research{ - Findings: []workphase.Finding{{Claim: "attribution runs per figure", Evidence: "internal/attr/attr.go:88"}}, + Findings: []workphase.Finding{{ID: "r1", Confidence: workphase.Fact, Claim: "attribution runs per figure", Evidence: "internal/attr/attr.go:88"}}, DeadEnds: []workphase.DeadEnd{{Tried: "figure plurality", WhyFailed: "no measured gain"}}, }) if err != nil { diff --git a/internal/integration/phase_protocol_test.go b/internal/integration/phase_protocol_test.go index bb88daf..df4c0c7 100644 --- a/internal/integration/phase_protocol_test.go +++ b/internal/integration/phase_protocol_test.go @@ -120,7 +120,7 @@ func TestPhaseRequestPathSealsAndFences(t *testing.T) { } } -var researchArtifact = workphase.Research{Findings: []workphase.Finding{{Claim: "runs per figure", Evidence: "attr.go:88"}}} +var researchArtifact = workphase.Research{Findings: []workphase.Finding{{ID: "r1", Confidence: workphase.Fact, Claim: "runs per figure", Evidence: "attr.go:88"}}} func sealed(t *testing.T, v interface{ Validate() error }) []byte { t.Helper() diff --git a/internal/operations/trajectory.go b/internal/operations/trajectory.go index 9c246ce..1ba3e7e 100644 --- a/internal/operations/trajectory.go +++ b/internal/operations/trajectory.go @@ -104,7 +104,7 @@ func TrajectoryGatePacket(s *store.Store, t domain.Task, from, to domain.WorkPha } if t.ResearchRef != "" { if raw, err := s.Artifact(t.ResearchRef); err == nil { - if r, err := workphase.DecodeResearch(raw); err == nil { + if r, err := workphase.DecodeStoredResearch(raw); err == nil { b.WriteString("\nWhat research established:\n") for _, f := range r.Findings { fmt.Fprintf(&b, "- %s (%s)\n", oneLine(f.Claim), oneLine(f.Evidence)) diff --git a/internal/operations/workphase_test.go b/internal/operations/workphase_test.go index 9ee5c32..7700dc4 100644 --- a/internal/operations/workphase_test.go +++ b/internal/operations/workphase_test.go @@ -46,7 +46,7 @@ func sealed(t *testing.T, v interface{ Validate() error }) []byte { return b } -var research = workphase.Research{Findings: []workphase.Finding{{Claim: "runs per figure", Evidence: "attr.go:88"}}} +var research = workphase.Research{Findings: []workphase.Finding{{ID: "r1", Confidence: workphase.Fact, Claim: "runs per figure", Evidence: "attr.go:88"}}} var plan = workphase.Plan{Changes: []workphase.Change{{Target: "attr.go", Intent: "aggregate per person"}}} func TestFullPhasePathSealsEachArtifact(t *testing.T) { diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index 16b5af7..69cffd0 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -1296,7 +1296,7 @@ func (c *Coordinator) research(ref string) (*workphase.Research, error) { if err != nil { return nil, err } - r, err := workphase.DecodeResearch(b) + r, err := workphase.DecodeStoredResearch(b) if err != nil { return nil, err } diff --git a/internal/workphase/workphase.go b/internal/workphase/workphase.go index 60f2c9c..de32414 100644 --- a/internal/workphase/workphase.go +++ b/internal/workphase/workphase.go @@ -11,16 +11,51 @@ package workphase import ( "encoding/json" "fmt" + "regexp" "sort" "strings" ) -// Finding is one thing research established, with the evidence for it. -type Finding struct { - Claim string `json:"claim"` - Evidence string `json:"evidence"` +// Confidence separates what research observed from what it concluded. A plan +// that cites an assumption as if it were a fact is the failure this exists to +// make visible. +type Confidence string + +const ( + // Fact is directly observed in the repository or from a live probe. + Fact Confidence = "fact" + // Inference is supported by evidence but was not verified directly. + Inference Confidence = "inference" + // Assumption is required because the evidence is missing. + Assumption Confidence = "assumption" +) + +func (c Confidence) Valid() bool { + switch c { + case Fact, Inference, Assumption: + return true + } + return false } +// Finding is one thing research established, with the evidence for it. +// +// ID is what makes a finding citable. A plan phase references "research:r12", +// and the seal resolves that against the accepted research, so a plan cannot +// rest on a finding nobody recorded. The advertised schema at agentctx.go +// promised id and confidence for months while the struct dropped both. +type Finding struct { + ID string `json:"id"` + Claim string `json:"claim"` + Evidence string `json:"evidence"` + Confidence Confidence `json:"confidence"` +} + +// findingID is deliberately narrow. An id appears in plan prose as +// "research:", so anything with spaces or punctuation would make the +// citation ambiguous to both a parser and a reader. +var findingID = regexp.MustCompile(`^[a-z0-9][a-z0-9_-]{0,63}$`) + // CodePath is a location the next phase will need, and why. type CodePath struct { Path string `json:"path"` @@ -68,7 +103,18 @@ func (r Research) Validate() error { if err := bound("findings", len(r.Findings)); err != nil { return err } + seen := map[string]bool{} for i, f := range r.Findings { + if !findingID.MatchString(f.ID) { + return fmt.Errorf("research: findings[%d].id %q must be lowercase letters, digits, dash or underscore, at most 64 characters", i, f.ID) + } + if seen[f.ID] { + return fmt.Errorf("research: duplicate finding id %q", f.ID) + } + seen[f.ID] = true + if !f.Confidence.Valid() { + return fmt.Errorf("research: findings[%d].confidence %q is not fact, inference, or assumption", i, f.Confidence) + } if err := line(fmt.Sprintf("findings[%d].claim", i), f.Claim, true); err != nil { return err } @@ -129,6 +175,9 @@ func Encode(v interface{ Validate() error }) ([]byte, error) { return json.Marshal(v) } +// DecodeResearch validates a newly sealed artifact. It is strict: a finding +// without an id or a confidence is refused, and the refusal reaches the agent +// that wrote it. func DecodeResearch(b []byte) (Research, error) { var r Research if err := json.Unmarshal(b, &r); err != nil { @@ -137,6 +186,30 @@ func DecodeResearch(b []byte) (Research, error) { return r, r.Validate() } +// DecodeStoredResearch reads an artifact already in the CAS. Artifacts sealed +// before ids existed carry neither field, and refusing them would block every +// task whose research predates this change, including at rotation, where the +// agent that could fix it is already gone. +// +// A backfilled finding is labelled inference: the old schema required +// evidence and made no verification claim, so inference is what it actually +// meant. Calling it fact would upgrade an unverified claim on the way in. +func DecodeStoredResearch(b []byte) (Research, error) { + var r Research + if err := json.Unmarshal(b, &r); err != nil { + return Research{}, fmt.Errorf("research artifact: %w", err) + } + for i := range r.Findings { + if r.Findings[i].ID == "" { + r.Findings[i].ID = fmt.Sprintf("legacy-%d", i+1) + } + if r.Findings[i].Confidence == "" { + r.Findings[i].Confidence = Inference + } + } + return r, r.Validate() +} + func DecodePlan(b []byte) (Plan, error) { var p Plan if err := json.Unmarshal(b, &p); err != nil { diff --git a/internal/workphase/workphase_test.go b/internal/workphase/workphase_test.go index 0a524cc..fd6154c 100644 --- a/internal/workphase/workphase_test.go +++ b/internal/workphase/workphase_test.go @@ -7,7 +7,7 @@ import ( func research() Research { return Research{ - Findings: []Finding{{Claim: "attribution runs per figure", Evidence: "internal/attr/attr.go:88"}}, + Findings: []Finding{{ID: "r1", Confidence: Fact, Claim: "attribution runs per figure", Evidence: "internal/attr/attr.go:88"}}, Code: []CodePath{{Path: "internal/attr/attr.go", Why: "aggregation happens here"}}, Invariants: []string{"identity semantics must not change"}, DeadEnds: []DeadEnd{{Tried: "figure plurality", WhyFailed: "no measured gain"}}, @@ -59,7 +59,7 @@ func TestBoundsRejectUnboundedArtifacts(t *testing.T) { "too many findings": func() error { r := Research{} for i := 0; i < 65; i++ { - r.Findings = append(r.Findings, Finding{Claim: "c", Evidence: "e"}) + r.Findings = append(r.Findings, Finding{ID: "r1", Confidence: Fact, Claim: "c", Evidence: "e"}) } return r.Validate() }, @@ -99,3 +99,47 @@ func TestEncodeRejectsInvalid(t *testing.T) { t.Fatal("Decode must reject non-JSON") } } + +// The advertised schema promised findings[].id and findings[].confidence while +// the struct carried neither, so both were dropped silently on every seal. A +// plan cannot cite what was never stored. +func TestFindingIdentityIsRequiredAndUnique(t *testing.T) { + cases := map[string]Research{ + "no id": {Findings: []Finding{{Confidence: Fact, Claim: "c", Evidence: "e"}}}, + "upper case id": {Findings: []Finding{{ID: "R1", Confidence: Fact, Claim: "c", Evidence: "e"}}}, + "spaced id": {Findings: []Finding{{ID: "r 1", Confidence: Fact, Claim: "c", Evidence: "e"}}}, + "no confidence": {Findings: []Finding{{ID: "r1", Claim: "c", Evidence: "e"}}}, + "bad confidence": {Findings: []Finding{{ID: "r1", Confidence: "certain", Claim: "c", Evidence: "e"}}}, + "duplicate id": {Findings: []Finding{ + {ID: "r1", Confidence: Fact, Claim: "c", Evidence: "e"}, + {ID: "r1", Confidence: Inference, Claim: "d", Evidence: "f"}, + }}, + } + for name, r := range cases { + if err := r.Validate(); err == nil { + t.Errorf("%s: accepted, want a refusal", name) + } + } +} + +// Research sealed before ids existed must stay readable. Refusing it would +// block every task whose research predates the change, including at rotation, +// where the agent that could fix it is already gone. +func TestStoredResearchWithoutIdentityStillDecodes(t *testing.T) { + legacy := []byte(`{"findings":[{"claim":"c","evidence":"e"},{"claim":"d","evidence":"f"}]}`) + if _, err := DecodeResearch(legacy); err == nil { + t.Fatal("a new seal without ids must be refused") + } + r, err := DecodeStoredResearch(legacy) + if err != nil { + t.Fatalf("stored decode: %v", err) + } + if r.Findings[0].ID != "legacy-1" || r.Findings[1].ID != "legacy-2" { + t.Fatalf("backfilled ids are not stable: %+v", r.Findings) + } + // Inference, not fact: the old schema required evidence and made no + // verification claim, so calling it fact would upgrade it on the way in. + if r.Findings[0].Confidence != Inference { + t.Fatalf("backfilled confidence is %q, want inference", r.Findings[0].Confidence) + } +}