diff --git a/internal/human/reconcile.go b/internal/human/reconcile.go index cd051d7..e021d71 100644 --- a/internal/human/reconcile.go +++ b/internal/human/reconcile.go @@ -54,6 +54,30 @@ type Reconciler struct { // decision and the handoff is not. const operatorInstructionSubject = "operator_instruction" +// verifyPrefix is the one keyed form a human comment may take. Everything +// else lands under operatorInstructionSubject, so an ordinary "looks good" +// cannot satisfy a plan phase's manual gate. +// +// The comment names only the phase. Orchestra supplies the plan ref from the +// task's own accepted plan, so the approval binds to the plan that was +// current when the human wrote it and can never be aimed at another one. +const verifyPrefix = "orchestra verify " + +// decisionSubject classifies one comment. A body whose first line is +// "orchestra verify " approves that phase of this task's accepted +// plan; anything else is an operator instruction. +func decisionSubject(t domain.Task, body string) string { + first := strings.TrimSpace(strings.SplitN(body, "\n", 2)[0]) + if !strings.HasPrefix(strings.ToLower(first), verifyPrefix) { + return operatorInstructionSubject + } + phase := strings.TrimSpace(first[len(verifyPrefix):]) + if phase == "" || t.PlanRef == "" { + return operatorInstructionSubject + } + return domain.PlanPhaseSubject(t.PlanRef, phase) +} + // Reconcile imports every input newer than the stored cursor, then advances // the cursor. It fails closed: any provider or append error returns an error // and leaves the cursor where it was, so the caller refuses the launch and a @@ -141,7 +165,7 @@ func (r *Reconciler) record(task domain.Task, provider string, in Input) error { payload := map[string]any{ "decision_id": domain.NewID(), "kind": string(domain.HumanDecisionCorrection), - "subject": operatorInstructionSubject, + "subject": decisionSubject(current, in.Body), "value": in.Body, "source": map[string]any{"provider": provider, "external_id": in.ExternalID}, "author": in.Author, diff --git a/internal/human/reconcile_test.go b/internal/human/reconcile_test.go index 17bf8af..d462a6c 100644 --- a/internal/human/reconcile_test.go +++ b/internal/human/reconcile_test.go @@ -288,3 +288,36 @@ func TestReconcileUsesOnlyTheTaskOwnSource(t *testing.T) { t.Fatalf("an unowned task reached a source: owner %d, other %d", owner.calls, other.calls) } } + +// A manual plan-phase gate is satisfied by a keyed comment and by nothing +// else. Without a producer for this subject the gate was unreachable: every +// imported comment landed under operator_instruction, so a phase with a +// manual step could never become verified. +func TestOnlyAKeyedCommentApprovesAPlanPhase(t *testing.T) { + task := domain.Task{ID: "t1", PlanRef: "plan-ref-a"} + want := domain.PlanPhaseSubject("plan-ref-a", "phase-2") + for _, c := range []struct { + body string + subject string + }{ + {"looks good to me", operatorInstructionSubject}, + {"orchestra verify phase-2", want}, + {"Orchestra Verify phase-2", want}, + {"orchestra verify phase-2\n\nthe json line read right", want}, + {"orchestra verify ", operatorInstructionSubject}, + {"please orchestra verify phase-2", operatorInstructionSubject}, + } { + if got := decisionSubject(task, c.body); got != c.subject { + t.Errorf("decisionSubject(%q) = %q, want %q", c.body, got, c.subject) + } + } + // No accepted plan means no phase to approve. + if got := decisionSubject(domain.Task{ID: "t1"}, "orchestra verify phase-2"); got != operatorInstructionSubject { + t.Errorf("without a plan ref: got %q", got) + } + // The subject binds the plan, so the same words under a later plan do not + // satisfy a gate earned under the earlier one. + if decisionSubject(domain.Task{ID: "t1", PlanRef: "plan-ref-b"}, "orchestra verify phase-2") == want { + t.Error("a different plan ref produced the same subject") + } +}