Bind a manual sign-off to the tree it was given against
F63, found live on run 19. A manual check on these projects is a human reading what the code prints. RecordPlanPhaseVerification asked only whether a sign-off for that plan and phase existed, and one exists forever, so rerunning a phase's automated checks at a new commit carried the human half along with it. The rig proved it twice: two operator commits and two re-verification requests, each coming back verified without anyone looking. The reducer now records which tree the human confirmed, the record carries it forward as provenance, and a run whose commit does not match it waits for the human again. A sign-off given before any run has no confirmed tree and still counts, so the ordinary ordering is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CVbaKucEYBjMqVeUgJUsc1
This commit is contained in:
@@ -55,6 +55,13 @@ type PlanPhaseRecord struct {
|
||||
// EvidenceRef is the CAS ref of the captured command output.
|
||||
EvidenceRef string `json:"evidence_ref,omitempty"`
|
||||
At time.Time `json:"at"`
|
||||
// ManualAtSHA is the tree a human was actually looking at when they signed
|
||||
// this phase off. A manual check on most projects is a human reading
|
||||
// output, so a sign-off establishes something about one tree and nothing
|
||||
// about the next one (F63). Rerunning the automated half re-establishes it
|
||||
// at the new commit; the manual half has to be given again, and this is
|
||||
// what makes the difference visible instead of assumed.
|
||||
ManualAtSHA string `json:"manual_at_sha,omitempty"`
|
||||
}
|
||||
|
||||
// Stale reports whether the tree has moved since this phase was verified. A
|
||||
|
||||
@@ -124,7 +124,12 @@ func RecordPlanPhaseVerification(s *store.Store, project registry.Project, taskI
|
||||
// was established.
|
||||
record.Status = domain.PlanPhaseAwaitingManual
|
||||
}
|
||||
if record.Status != domain.PlanPhaseInProgress && manuallySignedOff(s, t, phaseID) {
|
||||
// Carry the confirmed tree forward as provenance. Without it a second
|
||||
// rerun would compare against nothing and re-inherit the sign-off.
|
||||
if prior, ok := t.PlanPhase(phaseID); ok {
|
||||
record.ManualAtSHA = prior.ManualAtSHA
|
||||
}
|
||||
if record.Status != domain.PlanPhaseInProgress && manuallySignedOff(s, t, phaseID, record) {
|
||||
record.Status = domain.PlanPhaseVerified
|
||||
}
|
||||
if ref, err := s.PutArtifact(verificationEvidence(runs)); err == nil {
|
||||
@@ -135,7 +140,7 @@ func RecordPlanPhaseVerification(s *store.Store, project registry.Project, taskI
|
||||
payload := map[string]any{
|
||||
"plan_ref": record.PlanRef, "phase_id": record.PhaseID, "status": string(record.Status),
|
||||
"commands": record.Commands, "exit_codes": record.ExitCodes, "at_sha": record.AtSHA,
|
||||
"evidence_ref": record.EvidenceRef, "at": record.At,
|
||||
"evidence_ref": record.EvidenceRef, "at": record.At, "manual_at_sha": record.ManualAtSHA,
|
||||
}
|
||||
if t.Lease != nil {
|
||||
payload["harness_id"], payload["lease_epoch"] = t.Lease.HarnessID, t.Lease.Epoch
|
||||
@@ -149,9 +154,20 @@ func RecordPlanPhaseVerification(s *store.Store, project registry.Project, taskI
|
||||
}
|
||||
|
||||
// manuallySignedOff reports whether a human has already approved this exact
|
||||
// phase of this exact plan. The subject carries both, so a later "looks good"
|
||||
// on an unrelated thread cannot satisfy a gate nobody was discussing.
|
||||
func manuallySignedOff(s *store.Store, t domain.Task, phaseID string) bool {
|
||||
// phase of this exact plan, against the tree this run examined. The subject
|
||||
// carries plan and phase, so a later "looks good" on an unrelated thread
|
||||
// cannot satisfy a gate nobody was discussing.
|
||||
//
|
||||
// The tree matters as much as the subject (F63). A sign-off is a human saying
|
||||
// they read what this code prints; an edit afterwards can change exactly that.
|
||||
// A record whose ManualAtSHA names a different commit is therefore not signed
|
||||
// off, and waits for the human again. A sign-off given before any run has no
|
||||
// confirmed tree to compare against and still counts, which keeps the ordinary
|
||||
// ordering unchanged.
|
||||
func manuallySignedOff(s *store.Store, t domain.Task, phaseID string, record domain.PlanPhaseRecord) bool {
|
||||
if record.ManualAtSHA != "" && record.ManualAtSHA != record.AtSHA {
|
||||
return false
|
||||
}
|
||||
intent, err := s.EffectiveIntent(t.ID)
|
||||
if err != nil {
|
||||
return false
|
||||
|
||||
@@ -13,6 +13,7 @@ import (
|
||||
|
||||
const shaOne = "1111111111111111111111111111111111111111"
|
||||
const shaTwo = "2222222222222222222222222222222222222222"
|
||||
const shaThree = "3333333333333333333333333333333333333333"
|
||||
|
||||
func planProject() registry.Project {
|
||||
p := registry.Project{
|
||||
@@ -251,6 +252,14 @@ func TestLegacyPlanIsExplicitlyNonProgressable(t *testing.T) {
|
||||
// signOff records a human decision bound to one phase of one plan, which is
|
||||
// the only thing that satisfies a manual verification gate.
|
||||
func signOff(t *testing.T, s *store.Store, taskID, subject string) {
|
||||
t.Helper()
|
||||
signOffFrom(t, s, taskID, subject, "signoff-"+subject)
|
||||
}
|
||||
|
||||
// signOffFrom names the comment the sign-off came from. Two sign-offs on one
|
||||
// subject are a real sequence once a rerun sends a phase back to the human,
|
||||
// and provenance is unique per comment.
|
||||
func signOffFrom(t *testing.T, s *store.Store, taskID, subject, externalID string) {
|
||||
t.Helper()
|
||||
task, _ := s.Task(taskID)
|
||||
if err := s.Append(domain.Event{
|
||||
@@ -259,9 +268,60 @@ func signOff(t *testing.T, s *store.Store, taskID, subject string) {
|
||||
Payload: mustJSONBytes(t, map[string]any{
|
||||
"decision_id": domain.NewID(), "kind": "answer", "subject": subject,
|
||||
"value": "manual steps confirmed",
|
||||
"source": map[string]any{"provider": "gitea", "external_id": "signoff-" + subject},
|
||||
"source": map[string]any{"provider": "gitea", "external_id": externalID},
|
||||
}),
|
||||
}); err != nil {
|
||||
t.Fatalf("sign off: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// F63, found live on run 19. A manual sign-off says a human read what this
|
||||
// code prints. An edit afterwards can change exactly that, so rerunning the
|
||||
// automated half at a new commit must not carry the human half with it.
|
||||
func TestASignOffDoesNotSurviveTheTreeItWasGivenAgainst(t *testing.T) {
|
||||
s, project, id := planWith(t, twoPhasePlan)
|
||||
run := []VerificationRun{{Command: []string{"go", "test", "./internal/..."}, ExitCode: 0}}
|
||||
if _, err := RecordPlanPhaseVerification(s, project, id, "phase-2", shaOne, run); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
task, _ := s.Task(id)
|
||||
signOff(t, s, id, domain.PlanPhaseSubject(task.PlanRef, "phase-2"))
|
||||
task, _ = s.Task(id)
|
||||
rec, _ := task.PlanPhase("phase-2")
|
||||
if rec.Status != domain.PlanPhaseVerified || rec.ManualAtSHA != shaOne {
|
||||
t.Fatalf("sign-off did not bind to the tree it read: %+v", rec)
|
||||
}
|
||||
|
||||
// The tree moves and the phase is re-verified. The commands pass again;
|
||||
// the human has not seen the new output.
|
||||
if _, err := RecordPlanPhaseVerification(s, project, id, "phase-2", shaTwo, run); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
task, _ = s.Task(id)
|
||||
rec, _ = task.PlanPhase("phase-2")
|
||||
if rec.Status != domain.PlanPhaseAwaitingManual {
|
||||
t.Fatalf("status = %q at a tree the human never saw, want awaiting_manual_verification", rec.Status)
|
||||
}
|
||||
if rec.ManualAtSHA != shaOne {
|
||||
t.Fatalf("the confirmed tree was lost: %+v", rec)
|
||||
}
|
||||
|
||||
// A second rerun must not re-inherit it either, which is what carrying
|
||||
// ManualAtSHA forward is for.
|
||||
if _, err := RecordPlanPhaseVerification(s, project, id, "phase-2", shaThree, run); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
task, _ = s.Task(id)
|
||||
rec, _ = task.PlanPhase("phase-2")
|
||||
if rec.Status != domain.PlanPhaseAwaitingManual {
|
||||
t.Fatalf("a second rerun re-inherited the sign-off: %q", rec.Status)
|
||||
}
|
||||
|
||||
// Signing off again, on the tree that is now current, verifies it.
|
||||
signOffFrom(t, s, id, domain.PlanPhaseSubject(task.PlanRef, "phase-2"), "signoff-second")
|
||||
task, _ = s.Task(id)
|
||||
rec, _ = task.PlanPhase("phase-2")
|
||||
if rec.Status != domain.PlanPhaseVerified || rec.ManualAtSHA != shaThree {
|
||||
t.Fatalf("a fresh sign-off did not verify the current tree: %+v", rec)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -272,6 +272,9 @@ func (s *Store) apply(e domain.Event) error {
|
||||
}
|
||||
if p.Subject == domain.PlanPhaseSubject(rec.PlanRef, rec.PhaseID) {
|
||||
t.PlanProgress.Phases[i].Status = domain.PlanPhaseVerified
|
||||
// Record which tree the sign-off was about, so a later run
|
||||
// at a different commit cannot inherit it (F63).
|
||||
t.PlanProgress.Phases[i].ManualAtSHA = rec.AtSHA
|
||||
t.Version = e.Version
|
||||
s.replaceTask(e.TaskID, t)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user