-
Notifications
You must be signed in to change notification settings - Fork 1
Fix locked plan transfer across worktrees #214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c7d6629
a438ecf
45114bd
1b0cb42
4e3f295
3a59a27
69a2f62
22a37db
d63b4ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,36 @@ | ||
| package durable | ||
|
|
||
| import "testing" | ||
| import ( | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/operatorstack/boatstack/boatstack/internal/softwaredelivery/model" | ||
| ) | ||
|
|
||
| func TestStateRejectsPriorObjectiveSchema(t *testing.T) { | ||
| state := State{SchemaVersion: StateSchemaVersion - 1} | ||
| if err := state.Validate(); err == nil { | ||
| t.Fatal("prior objective state schema was accepted") | ||
| } | ||
| } | ||
|
|
||
| func TestStateSchemaPermitsLegacyApprovedStateWithoutApprovalFingerprint(t *testing.T) { | ||
| state := State{ | ||
| SchemaVersion: StateSchemaVersion, RepositoryID: "repo", GitCommonID: "common", WorktreeID: "worktree", Revision: 1, | ||
| Phase: model.PhaseActive, Engagement: model.EngagementActive, Delivery: model.DeliveryApproved, Workspace: model.WorkspaceAbsent, | ||
| Plan: model.PlanApproved, Configuration: model.ConfigurationUnsupported, Runtime: model.RuntimeAbsent, Publication: model.PublicationNone, | ||
| Verification: model.VerificationUnverified, Recovery: model.RecoveryNone, Transaction: model.TransactionNone, Terminal: model.TerminalNonterminal, | ||
| Objective: model.Objective{ID: "objective", TargetID: model.ObjectiveOpenPR, DeliveryID: "delivery"}, PlanFingerprint: "legacy-plan", UpdatedAt: time.Unix(1, 0).UTC(), | ||
| } | ||
| raw, err := EncodeState(state) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| decoded, err := DecodeState(raw) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if decoded.ApprovalFingerprint != "" { | ||
| t.Fatalf("legacy approval fingerprint = %q", decoded.ApprovalFingerprint) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -151,6 +151,7 @@ func prepareArtifacts(layout ports.ControllerLayout, admission protocol.Admissio | |
| } | ||
| mutations = append(mutations, approvalMutation) | ||
| state.PlanFingerprint = fingerprint | ||
| state.ApprovalFingerprint = "" | ||
| case "plan.validate": | ||
| path := filepath.Join(artifactRoot, "plans", deliveryID+".source") | ||
| raw, readErr := os.ReadFile(path) | ||
|
|
@@ -174,12 +175,14 @@ func prepareArtifacts(layout ports.ControllerLayout, admission protocol.Admissio | |
| return nil, mutationErr | ||
| } | ||
| mutations = append(mutations, mutation) | ||
| state.ApprovalFingerprint = sha256Bytes(raw) | ||
| case "evidence.approval.revoke": | ||
| mutation, mutationErr := mutationFor(filepath.Join(artifactRoot, "approvals", deliveryID+".json"), nil, 0o644, false, true) | ||
| if mutationErr != nil { | ||
| return nil, mutationErr | ||
| } | ||
| mutations = append(mutations, mutation) | ||
| state.ApprovalFingerprint = "" | ||
| case "gate.build.record", "gate.test.record", "gate.review.record", "gate.change.record", "gate.journey.record": | ||
| revision, _ := admission.Parameters.Get("source_revision") | ||
| evidencePath, _ := admission.Parameters.Get("evidence_path") | ||
|
|
@@ -299,6 +302,70 @@ func prepareArtifacts(layout ports.ControllerLayout, admission protocol.Admissio | |
| return mutations, nil | ||
| } | ||
|
|
||
| // prepareWorkspacePlanTransfer carries runtime-owned plan artifacts into a | ||
| // newly cut worktree. A run binds the plan bytes before the cut, so the target | ||
| // worktree must observe those exact bytes rather than fall back to the inbox | ||
| // and accidentally select new intent. | ||
| func prepareWorkspacePlanTransfer(repositoryRoot, workspacePath, deliveryID, expectedPlanFingerprint, expectedApprovalFingerprint string) ([]ports.ResourceMutation, error) { | ||
| if expectedPlanFingerprint == "" || deliveryID == "" { | ||
| return nil, nil | ||
| } | ||
| if workspacePath == "" || expectedApprovalFingerprint == "" { | ||
| return nil, fmt.Errorf("workspace plan transfer requires destination and exact approval for a bound plan") | ||
|
Comment on lines
+313
to
+314
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Do not require approval for every bound plan Invariant: any Confidence: 0.99 |
||
| } | ||
| deliveryID, err := safeSegment(deliveryID, "delivery identity") | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| sourceRoot := filepath.Join(repositoryRoot, ".boatstack") | ||
| destinationRoot := filepath.Join(workspacePath, ".boatstack") | ||
| planPath := filepath.Join(sourceRoot, "plans", deliveryID+".source") | ||
| planRaw, err := readRegularWorkspacePlanArtifact(planPath) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if actual := sha256Bytes(planRaw); actual != expectedPlanFingerprint { | ||
| return nil, fmt.Errorf("workspace plan artifact fingerprint changed: got %s", actual) | ||
| } | ||
| approvalPath := filepath.Join(sourceRoot, "approvals", deliveryID+".json") | ||
| approvalRaw, err := readRegularWorkspacePlanArtifact(approvalPath) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if actual := sha256Bytes(approvalRaw); actual != expectedApprovalFingerprint { | ||
| return nil, fmt.Errorf("workspace approval artifact fingerprint changed: got %s", actual) | ||
| } | ||
| var approval approvalArtifact | ||
| if err := decodeStrictArtifact(approvalRaw, &approval); err != nil || approval.SchemaVersion != 1 || approval.DeliveryID != deliveryID || | ||
| approval.PlanFingerprint != expectedPlanFingerprint || approval.Actor == "" || approval.AdmissionID == "" || approval.ApprovedAt.IsZero() { | ||
| return nil, fmt.Errorf("workspace approval artifact does not bind the admitted plan") | ||
| } | ||
| planMutation, err := mutationFor(filepath.Join(destinationRoot, "plans", deliveryID+".source"), planRaw, 0o644, false, false) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| approvalMutation, err := mutationFor(filepath.Join(destinationRoot, "approvals", deliveryID+".json"), approvalRaw, 0o644, false, false) | ||
|
Comment on lines
+343
to
+347
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Bind destination artifacts only after establishing their actual paths Invariant: staged mutations must bind the actual prior resource and remain inside the admitted workspace. Confidence: 0.99 |
||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return []ports.ResourceMutation{planMutation, approvalMutation}, nil | ||
| } | ||
|
|
||
| func readRegularWorkspacePlanArtifact(path string) ([]byte, error) { | ||
| info, err := os.Lstat(path) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("inspect workspace plan artifact %s: %w", path, err) | ||
| } | ||
| if !info.Mode().IsRegular() { | ||
| return nil, fmt.Errorf("workspace plan artifact is not a regular file: %s", path) | ||
| } | ||
| raw, err := os.ReadFile(path) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("read workspace plan artifact %s: %w", path, err) | ||
| } | ||
| return raw, nil | ||
| } | ||
|
|
||
| func transitionUsesDeliveryArtifacts(id catalog.TransitionID) bool { | ||
| switch id { | ||
| case "plan.create", "plan.amend", "plan.validate", "plan.approve", "plan.approve-amendment", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Version the durable state ABI for the new field
Invariant: equal state-schema identities must denote mutually compatible durable wire formats. Head now emits
approval_fingerprintwhile retaining schema version 4; the base runtime's strict decoder rejects that unknown field. Thus after head commits an approval, an exact base candidate cannot even observe the state to perform an otherwise legal runtime downgrade/update, despite both pins declaring schema 4. The new legacy test checks only that the current decoder reads field-absent bytes and does not exercise the real old decoder. A regression test should use the actual base decoder against a head-emitted approved state and require either compatibility or a distinct schema identity with an explicit migration path.Confidence: 0.96