diff --git a/docs/runtime-install.md b/docs/runtime-install.md index 08ff1891d..dfdfc816c 100644 --- a/docs/runtime-install.md +++ b/docs/runtime-install.md @@ -153,6 +153,7 @@ under the destination or in the host record is created before all three hold. `--backup-state-to ` (on `install`, `update` and `rollback`) is the acknowledgement that the swap brings the additive DAG zone, or ordinary indexes on tables the store already holds, to a store that lacks them: the command copies the whole relay state directory to `` before promoting, and without it that swap refuses and names the flag ([the route](#why-the-schema-reading-compares-statements-and-not-versions)). +The same artifact can be taken outside an install, by hand, with `crw install backup-state --to ` ([the operator command](#the-state-backup-outside-an-install)). What follows is one run, in this order, and the result lists the steps it took: @@ -419,22 +420,33 @@ it copies the whole state directory the gate read to `DIR`: copy only, byte for or deleted, here or on a failure. Directories and regular files are copied with their bytes and, once every byte is in place and verified, their permission bits, each synced after its mode is set so the mode and not only the bytes survives a power loss (the directory the backup is made in stays 0700, and a copy always keeps its owner able to open it: a source the installer could read only through its group gets the owner's read bit, and the manifest records both modes); a symbolic link to a regular file is copied as the file's bytes under the link's name (a link alone would back up nothing), and when `relay.sqlite3` is such a link the real file's `-wal` is copied beside it, so a restore opens with the commits only the log held; a socket, a FIFO or a device is listed as skipped. The store's two -sidecars follow SQLite's WAL mode: `relay.sqlite3-shm` is never listed, copied or compared, because SQLite rebuilds that index from the log on -open, and `relay.sqlite3-wal` is copied when it is there at its copy, while an empty one that goes or appears between the listing and the copy is -not a refusal (a log listed and gone is dropped, one that appeared after the listing is not copied, and the manifest's `storeSidecars` records which -of the four happened). An empty log is the case this route exists for: a read-only open of a store no connection holds leaves one. A log that was +sidecars are recognised by the path SQLite resolves the database to, not by their top-level name: with `relay.sqlite3` linked to a file inside +the state directory (or outside it), the sidecars are that file's `-wal` and `-shm`, wherever they lie. `-shm` is never listed, copied or compared, +because SQLite rebuilds that index from the log on open, and `-wal` is copied when it is there at its copy, under the restore-compatible name +`relay.sqlite3-wal`, while an empty one that goes or appears between the listing and the copy is not a refusal (a log listed and gone is dropped, +one that appeared after the listing is not copied, and the manifest's `storeSidecars` records which of the four happened). An empty log is the case +this route exists for: a read-only open of a store no connection holds leaves one. The log is compared by its identity, not its size: its kind, its +link target and the file its bytes come from must be the same in both listings, so a log whose link target or resolved source moved under the copy +refuses ("the store's write-ahead log changed"), while the log's size and its coming and going are left to the sidecar rules above. A log that was not copied and holds frames in the second listing refuses instead, because a commit that stays in the log does not touch `relay.sqlite3` until a checkpoint, so the digest check alone would not see it and the backup would claim success while the live store held a row the copy lacks. `relay.sqlite3` itself and every other file keep the rule above, so a store that was written under the copy still refuses, and a copied `-wal` that is still there at the verification must digest to what was copied: one that a checkpoint has taken away by then is not a refusal either, and -`relay.sqlite3` is the consistency the verification keeps. +`relay.sqlite3` is the consistency the verification keeps. A store's `-wal` or `-shm` that goes between a listing's directory read and the entry's own +information is recorded as gone before its copy rather than refusing; every other file's disappearance there still refuses. Each file is hashed while it is read and synced; then the state directory is read again, and the listing, every size and every file's digest, and the digest of every file in the copy, must be what was copied. Any difference, in any file, refuses the swap ("the state directory changed under the copy"): the copy is of one moment or it is not made. The backup, its manifest and the directories made for them are synced in their parents after the manifest is written, so a power loss cannot keep the files and lose the names that reach them. `DIR` and its manifest must not exist, must not lie inside the state directory, and must not lie inside the runtime destination tree (a failed run removes its candidate runtime, and a backup there would go with it); the free space on its -filesystem must cover the directory. The record is written last, beside the backup and not inside it (`DIR.manifest.json`: source, destination, -time, issue, every entry with its size, mode and digest, what was skipped, an aggregate digest), and the command's result carries the same facts +filesystem must cover the directory. Before the record is written the copy passes the integrity gate: a scratch duplicate of the copied store (its +`relay.sqlite3` and, when copied, its `-wal`) is opened with SQLite and asked `PRAGMA integrity_check`. The duplicate is what SQLite touches, never +the copy, so the backup’s bytes are unchanged by the check, and the duplicate is removed afterwards. The destination’s free-space preflight covers +the backup and that duplicate together. The manifest records `integrityCheck` (SQLite’s +answer) and `restoreCandidate` (true only when the check passed), and a copy that fails the check is not a restore candidate and refuses. The +record is written last, beside the backup and not inside it (`DIR.manifest.json`: source, destination, +time, issue, every entry with its size, mode and digest, what was skipped, an aggregate digest, `integrityCheck` and `restoreCandidate`), and the command's +result carries the same facts as `swapGate.stateBackup`. A backup that fails part-way stays where it is and is reported with `partial: true`; a swap that fails after the backup keeps it too, and a rerun needs a new destination. @@ -494,6 +506,25 @@ The warning against write commands stays: opening a store for writing with this relay command run by hand against a live state directory takes no backup), so the route makes the warning unnecessary only inside the install command. Run no write command of this build against a live state directory before the install has taken its backup. +### The state backup outside an install + +`crw install backup-state --to ` takes the same stopped byte copy the install route takes, outside any install, so an operator can +take the default artifact on demand. It calls the same `backupState` routine, so the copy, its manifest and its `integrityCheck` and +`restoreCandidate` fields are the same, and the destination rules (`` and its manifest must not exist, must not lie inside the state +directory or the runtime destination tree, and the free space must cover the directory) are the same. It is routed before the generic install +options, like `features`, `config` and `migrate-state`: it takes its own `--to` flag and prints its own JSON report (`command: "backup-state"`, +`applied`, `stateBackup`), and the frozen `crw install` usage line still names only the commands it named before this one arrived. + +It refuses while the relay service runs, reading the relay the same way the swap gate does: the selected relay’s own `service status`. Stopping +the service is the operator’s job. A reading that could not be taken refuses too, so a backup is never taken on the strength of an unasked +service. While it copies, it holds the store’s write gate exclusively, so no relay writer reaches the store under the copy: the gate is the one +the relay writers lock, beside the database as SQLite resolves it (`holdGate`), which is the state directory’s own `write-gate.lock` in the +ordinary layout and the linked file’s directory otherwise. A writer that already holds it refuses the command rather than waiting, and the +command takes no other lock. + +What this command does not do: it does not stop or start the service, it does not restore, and it does not schedule anything. The online +`sqlite3_backup` snapshot and its schedule are a separate decision and are not built here. + ### The order a swap commits in The selection in the host record and the pointer on disk are two truths, and both the order they diff --git a/internal/manage/improve_collect.go b/internal/manage/improve_collect.go index 1eb98ed15..414cef9cc 100644 --- a/internal/manage/improve_collect.go +++ b/internal/manage/improve_collect.go @@ -127,16 +127,17 @@ func improveNewAccumulator() *improveAccumulator { // improveAdd merges one record. A record's what is kept from the first row of its identity, // so the value does not depend on which row a later read happened to reach first. The // identity is a JSON array rather than joined text, so a key that carries a delimiter -// cannot make two different identities read as one. +// cannot make two different identities read as one. The two times are compared as the +// instants they name, so a fractional second or a different offset cannot reverse them. func (a *improveAccumulator) improveAdd(r improveRecord) { identity := improveIdentity(r.Kind, r.Key, r.Where) if at, ok := a.index[identity]; ok { current := &a.records[at] current.Count += r.Count - if r.FirstAt != "" && (current.FirstAt == "" || r.FirstAt < current.FirstAt) { + if improveParseEarlier(r.FirstAt, current.FirstAt) { current.FirstAt = r.FirstAt } - if r.LastAt > current.LastAt { + if improveParseLater(r.LastAt, current.LastAt) { current.LastAt = r.LastAt } if current.What == "" || (improveGenericReason(current.What) && !improveGenericReason(r.What)) { @@ -149,6 +150,67 @@ func (a *improveAccumulator) improveAdd(r improveRecord) { a.records = append(a.records, r) } +// improveParseInstant parses a stored time as the instant it names. The second result is false +// when the text is empty or is not an RFC3339Nano instant this build can read. +func improveParseInstant(value string) (time.Time, bool) { + if value == "" { + return time.Time{}, false + } + parsed, err := time.Parse(time.RFC3339Nano, value) + if err != nil { + return time.Time{}, false + } + return parsed, true +} + +// improveParseEarlier reports whether candidate is the earlier of two stored times and should +// replace current as a record's first_at. An empty candidate never does and an empty current +// always does. A value that is not an instant goes after one that is, because comparing the text +// instead reverses "…00:00:00.5Z" and "…00:00:00Z": '.' sorts before 'Z'. Two values that are +// both unreadable fall back to their text, so the same input still writes the same bytes. +func improveParseEarlier(candidate, current string) bool { + if candidate == "" { + return false + } + if current == "" { + return true + } + parsed, ok := improveParseInstant(candidate) + if !ok { + if _, currentOK := improveParseInstant(current); currentOK { + return false + } + return candidate < current + } + if currentParsed, currentOK := improveParseInstant(current); currentOK { + return parsed.Before(currentParsed) + } + return true +} + +// improveParseLater reports whether candidate is the later of two stored times and should replace +// current as a record's last_at. It is improveParseEarlier's mirror, including where a value that +// is not an instant sits: after every value that is one. +func improveParseLater(candidate, current string) bool { + if candidate == "" { + return false + } + if current == "" { + return true + } + parsed, ok := improveParseInstant(candidate) + if !ok { + if _, currentOK := improveParseInstant(current); currentOK { + return true + } + return candidate > current + } + if currentParsed, currentOK := improveParseInstant(current); currentOK { + return parsed.After(currentParsed) + } + return false +} + // improveGenericReason reports whether a split record description is only the bare outcome // name, which a more specific reason from another row of the same record replaces. func improveGenericReason(reason string) bool { @@ -725,19 +787,42 @@ func improveReadRelay(ctx context.Context, dbPath string, acc *improveAccumulato if err != nil { return err } + // A child's blocked_needs_input receipt carries no reason field: receiptFields in + // internal/relay/store/receipt_shape.go allows none, so reading one there found no + // reason for any real blockage. The reason is the note of the decision reply that + // answered the event, and that receipt names the answered event in its answersEvent, so + // one prepass indexes the replies before the blocked rows are read. + answered := map[improveParseBlockedKey]string{} + for _, row := range splits { + if row.Text("outcome") != "decision_reply" { + continue + } + receipt := improveParseJSONObject(row.Text("receipt")) + event := improveStringField(receipt, "answersEvent") + if event == "" { + continue + } + answered[improveParseBlockedKey{relationship: row.Text("relationship_id"), event: event}] = improveStringField(receipt, "note") + } for _, row := range splits { outcome, receipt := row.Text("outcome"), improveParseJSONObject(row.Text("receipt")) + project := row.Text("project_key") + issue := row.Text("issue_key") + var reason string if outcome == "decision_reply" { decision := improveStringField(receipt, "decision") if decision != "split_approval" && decision != "scope_change" { continue } - } - project := row.Text("project_key") - issue := row.Text("issue_key") - reason := improveStringField(receipt, "reason", "detail", "note", "question", "summary") - if reason == "" { - reason = improveStringField(receipt, "outcome", "decision") + reason = improveStringField(receipt, "reason", "detail", "note", "question", "summary") + if reason == "" { + reason = improveStringField(receipt, "outcome", "decision") + } + } else { + reason = answered[improveParseBlockedKey{relationship: row.Text("relationship_id"), event: row.Text("event_id")}] + if reason == "" { + reason = string(store.BlockedNeedsInput) + } } rows++ acc.improveAdd(improveRecord{Kind: improveKindSplit, Key: improveSplitKey(project, issue), Where: row.Text("relationship_id"), @@ -762,6 +847,14 @@ func improveSplitKey(project, issue string) string { return issue } +// improveParseBlockedKey names one blocked event inside one relationship. The two fields are +// compared as themselves rather than joined, so a value that carries a delimiter cannot make two +// different events read as one. +type improveParseBlockedKey struct { + relationship string + event string +} + // improveReadDag reads each plan the source's pattern names out of the store at dbPath and // normalizes each metric of the measurements into one record. // @@ -879,47 +972,100 @@ func improveReadInterventions(path string, acc *improveAccumulator) (int, error) return len(lines), nil } -// improveReadDrafts reads the audit drafts a management session keeps: one JSON file per -// draft, under a directory or a single file, and normalizes each by the issue it drafts. +// improveReadDrafts reads the audit drafts a management session keeps: one crw-issue-draft/1 +// document per draft, under a directory or as a single file. A candidate that decodes to an object +// of another schema — the directory's index.json, or any other document — is skipped and not +// counted, so the listing is never read as a draft of its own. Each draft becomes one record keyed +// by its fingerprint, whose first and last sighting are the earliest and latest seen[].at. A +// document whose schema is right but whose fingerprint is empty is refused, because a record keyed +// on nothing would merge two different drafts. +// +// A candidate that is not a JSON object at all is refused rather than skipped: it is not a +// document of another schema, so reading it as nothing would report a complete bundle over a file +// the drafts directory could not be read from. That is the rule improveReadJSONLines already +// applies to a null line, and it is what this reader did before the schema gate existed. func improveReadDrafts(path string, acc *improveAccumulator) (int, error) { - info, err := os.Stat(path) + files, err := improveParseDraftFiles(path) if err != nil { return 0, err } - var files []string - if info.IsDir() { - entries, err := os.ReadDir(path) - if err != nil { - return 0, err - } - for _, entry := range entries { - if !entry.IsDir() && strings.HasSuffix(entry.Name(), ".json") { - files = append(files, filepath.Join(path, entry.Name())) - } - } - sort.Strings(files) - } else { - files = []string{path} - } rows := 0 for _, file := range files { data, err := os.ReadFile(file) if err != nil { return 0, err } - draft := improveParseJSONObject(string(data)) - if draft == nil { - return 0, fmt.Errorf("%s: not a JSON object", file) + schema, err := improveParseDraftSchema(file, data) + if err != nil { + return 0, err + } + if schema != auditDraftSchema { + continue + } + // auditDraftLoad (CRW-695) is the writer's own reader: it refuses an empty or mismatched + // fingerprint and a key this build does not know, so a document that claims to be a draft + // but is not one is named rather than read as something else. + draft, err := auditDraftLoad(file) + if err != nil { + return 0, err } + first, last := improveParseSeenBounds(draft.Seen) rows++ - acc.improveAdd(improveRecord{Kind: improveKindDraft, Key: improveStringField(draft, "issue", "identifier"), - Where: improveStringField(draft, "project", "project_key"), What: improveStringField(draft, "title", "summary"), Count: 1, - FirstAt: improveStringField(draft, "created_at", "recorded_at", "graded_at"), LastAt: improveStringField(draft, "created_at", "recorded_at", "graded_at"), - Evidence: []string{file}}) + acc.improveAdd(improveRecord{Kind: improveKindDraft, Key: draft.Fingerprint, Where: draft.Project, What: draft.Title, Count: 1, + FirstAt: first, LastAt: last, Evidence: []string{file}}) } return rows, nil } +// improveParseDraftFiles is the candidate draft files of a configured path: the .json files +// directly inside a directory, or the path itself when it is a file. +func improveParseDraftFiles(path string) ([]string, error) { + info, err := os.Stat(path) + if err != nil { + return nil, err + } + if !info.IsDir() { + return []string{path}, nil + } + entries, err := os.ReadDir(path) + if err != nil { + return nil, err + } + files := make([]string, 0, len(entries)) + for _, entry := range entries { + if !entry.IsDir() && strings.HasSuffix(entry.Name(), ".json") { + files = append(files, filepath.Join(path, entry.Name())) + } + } + sort.Strings(files) + return files, nil +} + +// improveParseDraftSchema is the schema a candidate file declares. A file that is not a JSON +// object carries no schema and is refused, because the drafts directory could not be read as what +// it holds rather than as a document of another schema. +func improveParseDraftSchema(path string, data []byte) (string, error) { + object := improveParseJSONObject(string(data)) + if object == nil { + return "", fmt.Errorf("%s: not a JSON object", path) + } + return improveStringField(object, "schema"), nil +} + +// improveParseSeenBounds is the earliest and latest sighting of a draft's seen list, compared as +// instants so a fractional second or a different offset cannot reverse them. +func improveParseSeenBounds(seen []auditDraftSeen) (first, last string) { + for _, entry := range seen { + if improveParseEarlier(entry.At, first) { + first = entry.At + } + if improveParseLater(entry.At, last) { + last = entry.At + } + } + return first, last +} + // improveReadIssues reads the issue list a management session exported: an array of issues, // or an object carrying them under issues. func improveReadIssues(path string, acc *improveAccumulator) (int, error) { diff --git a/internal/manage/improve_collect_parse_test.go b/internal/manage/improve_collect_parse_test.go new file mode 100644 index 000000000..a75322663 --- /dev/null +++ b/internal/manage/improve_collect_parse_test.go @@ -0,0 +1,318 @@ +package manage + +import ( + "database/sql" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// The tests here pin the reading rules CRW-873 fixes: a draft is a crw-issue-draft/1 document +// keyed by its fingerprint, a blocked receipt's reason comes from the decision reply that +// answered it, and stored times compare as instants. They are hermetic like the rest of this +// package's tests: temporary paths, a synthetic store, no real relay state. + +// improveParseDraft is a crw-issue-draft/1 document in the shape auditDraftLoad accepts. +func improveParseDraft(fingerprint, project, title string, seen []auditDraftSeen) auditDraft { + return auditDraft{ + Schema: auditDraftSchema, + Fingerprint: fingerprint, + Source: auditDraftSource, + Project: project, + Title: title, + Severity: auditDraftDefaultSeverity, + Body: "the defect this draft carries", + Labels: []string{"bug"}, + Seen: seen, + State: auditDraftStateDraft, + } +} + +// improveParseDraftJSON is a draft document as a file holds it. +func improveParseDraftJSON(t *testing.T, doc auditDraft) string { + t.Helper() + data, err := json.MarshalIndent(doc, "", " ") + if err != nil { + t.Fatal(err) + } + return string(append(data, '\n')) +} + +// improveParseWriteDraft writes one draft under the name its fingerprint gives it. +func improveParseWriteDraft(t *testing.T, dir string, doc auditDraft) string { + t.Helper() + path := filepath.Join(dir, doc.Fingerprint+".json") + improveTestWrite(t, path, improveParseDraftJSON(t, doc)) + return path +} + +// improveParseWriteIndex writes the drafts directory's index.json, which is a listing and not a +// draft. +func improveParseWriteIndex(t *testing.T, dir string, fingerprints ...string) { + t.Helper() + data, err := json.Marshal(map[string]any{"schema": auditDraftIndexSchema, "drafts": fingerprints}) + if err != nil { + t.Fatal(err) + } + improveTestWrite(t, filepath.Join(dir, auditDraftIndexFile), string(data)) +} + +// improveParseDraftsDir makes the drafts directory and returns it. +func improveParseDraftsDir(t *testing.T, s *improveTestState) string { + t.Helper() + dir := filepath.Join(s.root, "drafts") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + return dir +} + +// improveParseConfig configures the relay store and the drafts directory and returns the bundle +// path a run writes to. +func improveParseConfig(t *testing.T, s *improveTestState, drafts string) string { + t.Helper() + improveTestConfig(t, s, map[string]any{"manage": map[string]any{"improve": map[string]any{ + "sources": map[string]any{ + "relay": map[string]any{"path": s.stateDir}, + "draft": map[string]any{"path": drafts}, + }, + }}}) + return filepath.Join(s.root, "bundle.json") +} + +// improveParseCollect runs the collector and returns the bundle it wrote. +func improveParseCollect(t *testing.T, s *improveTestState, out string) improveBundle { + t.Helper() + if code, _, stderr := improveTestRun(t, s, "--out", out); code != 0 { + t.Fatalf("collect: exit %d, stderr %s", code, stderr) + } + return improveTestReadBundle(t, out) +} + +// TestImproveParseDraftsAreReadByFingerprint covers C1: only a crw-issue-draft/1 document is a +// draft, keyed by its fingerprint, and the directory's index.json is a listing rather than a +// draft, so it is neither read nor counted. +func TestImproveParseDraftsAreReadByFingerprint(t *testing.T) { + s := improveTestSetup(t) + improveTestStore(t, s, func(t *testing.T, db *sql.DB) {}) + dir := improveParseDraftsDir(t, s) + improveParseWriteDraft(t, dir, improveParseDraft("aaaa1111bbbb2222", "project-a", "first draft", + []auditDraftSeen{{At: "2026-10-06T02:00:00Z"}, {At: "2026-10-06T01:00:00Z"}})) + improveParseWriteDraft(t, dir, improveParseDraft("cccc3333dddd4444", "project-b", "second draft", + []auditDraftSeen{{At: "2026-10-06T05:00:00Z"}})) + improveParseWriteIndex(t, dir, "aaaa1111bbbb2222", "cccc3333dddd4444") + out := improveParseConfig(t, s, dir) + + bundle := improveParseCollect(t, s, out) + if got := improveTestSourceOf(t, bundle, improveKindDraft); got.State != improveStateRead || got.Rows != 2 { + t.Errorf("the draft source = %+v, want read with 2 rows: index.json is not a draft", got) + } + records := improveTestRecordsOf(bundle, improveKindDraft) + if len(records) != 2 { + t.Fatalf("draft records = %d, want 2 keyed by fingerprint: %+v", len(records), records) + } + byKey := map[string]improveRecord{} + for _, record := range records { + byKey[record.Key] = record + } + first, ok := byKey["aaaa1111bbbb2222"] + if !ok { + t.Fatalf("no record is keyed by the first fingerprint: %+v", records) + } + if first.Where != "project-a" || first.What != "first draft" { + t.Errorf("the first draft record = %+v, want its project and title", first) + } + if first.FirstAt != "2026-10-06T01:00:00Z" || first.LastAt != "2026-10-06T02:00:00Z" { + t.Errorf("the first draft times = %q..%q, want the earliest and latest seen[].at", first.FirstAt, first.LastAt) + } + if len(first.Evidence) != 1 || !strings.HasSuffix(first.Evidence[0], "aaaa1111bbbb2222.json") { + t.Errorf("the first draft evidence = %v, want its file path", first.Evidence) + } + second, ok := byKey["cccc3333dddd4444"] + if !ok || second.Where != "project-b" || second.What != "second draft" { + t.Errorf("the second draft record = %+v, want the other fingerprint", second) + } +} + +// TestImproveParseDraftWithoutAFingerprintIsUnreadable covers C1's refusal: a document whose +// schema is crw-issue-draft/1 but whose fingerprint is empty is not a draft the collector can +// name, so the source is unreadable rather than silently keyed on nothing. +func TestImproveParseDraftWithoutAFingerprintIsUnreadable(t *testing.T) { + s := improveTestSetup(t) + improveTestStore(t, s, func(t *testing.T, db *sql.DB) {}) + dir := improveParseDraftsDir(t, s) + doc := improveParseDraft("", "project-a", "nameless", []auditDraftSeen{{At: "2026-10-06T01:00:00Z"}}) + improveTestWrite(t, filepath.Join(dir, "nameless.json"), improveParseDraftJSON(t, doc)) + out := improveParseConfig(t, s, dir) + + code, stdout, stderr := improveTestRun(t, s, "--out", out) + if code != 1 || !strings.Contains(stderr, improveReasonSourceUnreadable) { + t.Fatalf("a draft with no fingerprint: exit %d, stderr %q, want exit 1 naming %s", code, stderr, improveReasonSourceUnreadable) + } + if stdout != "" { + t.Errorf("a refused run wrote to stdout: %q", stdout) + } + if _, err := os.Stat(out); !os.IsNotExist(err) { + t.Errorf("a refused run left a bundle behind (stat err %v)", err) + } +} + +// TestImproveParseDraftFileThatIsNotAnObjectIsRefused covers the reader's fail-closed edge: a +// .json file in the drafts directory that is not an object is neither a draft nor a document of +// another schema, so the source is unreadable rather than reported as a complete bundle that +// quietly left the file out. +func TestImproveParseDraftFileThatIsNotAnObjectIsRefused(t *testing.T) { + s := improveTestSetup(t) + improveTestStore(t, s, func(t *testing.T, db *sql.DB) {}) + dir := improveParseDraftsDir(t, s) + improveParseWriteDraft(t, dir, improveParseDraft("aaaa1111bbbb2222", "project-a", "a real draft", + []auditDraftSeen{{At: "2026-10-06T01:00:00Z"}})) + improveTestWrite(t, filepath.Join(dir, "torn.json"), "[\"not an object\"]\n") + out := improveParseConfig(t, s, dir) + + code, stdout, stderr := improveTestRun(t, s, "--out", out) + if code != 1 || !strings.Contains(stderr, improveReasonSourceUnreadable) { + t.Fatalf("a candidate that is not an object: exit %d, stderr %q, want exit 1 naming %s", code, stderr, improveReasonSourceUnreadable) + } + if stdout != "" { + t.Errorf("a refused run wrote to stdout: %q", stdout) + } + if _, err := os.Stat(out); !os.IsNotExist(err) { + t.Errorf("a refused run left a bundle behind (stat err %v)", err) + } +} + +// TestImproveParseDraftsSkipAForeignSchemaDocument covers C1's "any other schema" clause on its +// own: a document in the drafts directory that carries a schema and is not a crw-issue-draft/1 is +// skipped and not counted, exactly as index.json is. Only the listing was covered before, so a +// reader that keyed on any object would still have passed. +func TestImproveParseDraftsSkipAForeignSchemaDocument(t *testing.T) { + s := improveTestSetup(t) + improveTestStore(t, s, func(t *testing.T, db *sql.DB) {}) + dir := improveParseDraftsDir(t, s) + improveParseWriteDraft(t, dir, improveParseDraft("aaaa1111bbbb2222", "project-a", "the only draft", + []auditDraftSeen{{At: "2026-10-06T01:00:00Z"}})) + improveTestWrite(t, filepath.Join(dir, "foreign.json"), "{\"schema\":\"crw-improve-bundle/1\",\"records\":[]}\n") + out := improveParseConfig(t, s, dir) + + bundle := improveParseCollect(t, s, out) + if got := improveTestSourceOf(t, bundle, improveKindDraft); got.State != improveStateRead || got.Rows != 1 { + t.Errorf("the draft source = %+v, want read with 1 row: a foreign schema is not a draft", got) + } + records := improveTestRecordsOf(bundle, improveKindDraft) + if len(records) != 1 || records[0].Key != "aaaa1111bbbb2222" { + t.Errorf("draft records = %+v, want only the crw-issue-draft/1 document", records) + } +} + +// TestImproveParseDraftSourceMayBeASingleFile covers the other source shape: the configured path +// may name one draft file rather than a directory, which is what the reader accepted before and +// still accepts. +func TestImproveParseDraftSourceMayBeASingleFile(t *testing.T) { + s := improveTestSetup(t) + improveTestStore(t, s, func(t *testing.T, db *sql.DB) {}) + dir := improveParseDraftsDir(t, s) + path := improveParseWriteDraft(t, dir, improveParseDraft("eeee5555ffff6666", "project-c", "the single file", + []auditDraftSeen{{At: "2026-10-06T04:00:00Z"}, {At: "2026-10-06T03:00:00Z"}})) + out := improveParseConfig(t, s, path) + + bundle := improveParseCollect(t, s, out) + if got := improveTestSourceOf(t, bundle, improveKindDraft); got.State != improveStateRead || got.Rows != 1 { + t.Errorf("a single-file draft source = %+v, want read with 1 row", got) + } + records := improveTestRecordsOf(bundle, improveKindDraft) + if len(records) != 1 || records[0].Key != "eeee5555ffff6666" || records[0].Where != "project-c" { + t.Fatalf("draft records = %+v, want the one draft the file holds", records) + } + if records[0].FirstAt != "2026-10-06T03:00:00Z" || records[0].LastAt != "2026-10-06T04:00:00Z" { + t.Errorf("the single-file draft times = %q..%q, want its seen bounds", records[0].FirstAt, records[0].LastAt) + } +} + +// TestImproveParseBlockedReasonComesFromTheDecisionReply covers C2: a child receipt carries no +// reason field, so the reason of a blockage is the note of the decision reply that answered it, +// and a blockage no decision answered keeps the bare outcome name. +func TestImproveParseBlockedReasonComesFromTheDecisionReply(t *testing.T) { + s := improveTestSetup(t) + improveTestStore(t, s, func(t *testing.T, db *sql.DB) { + for _, rel := range []struct{ id, issue, project string }{ + {"rel-a", "CRW-1", "project-a"}, + {"rel-b", "CRW-2", "project-b"}, + } { + improveTestInsert(t, db, "INSERT INTO relationships (relationship_id, issue_key, status, parent_task_id, parent_host_id, child_task_id, child_host_id, execution_generation, artifact_roots, allowed_recipients, created_at, updated_at) VALUES (?,?,'active','parent','host','child','host',1,'[]','[]','2026-10-06T00:00:00Z','2026-10-06T00:00:00Z')", rel.id, rel.issue) + improveTestInsert(t, db, "INSERT INTO relationship_scope (relationship_id, project_key, recorded_at) VALUES (?,?,'2026-10-06T00:00:00Z')", rel.id, rel.project) + } + // The child's own receipt cannot carry a reason; a planted one must never be read. + improveTestInsert(t, db, "INSERT INTO events (event_id, relationship_id, execution_generation, revision_hash, outcome, producer, turn_thread_id, turn_id, turn_status, receipt, stage, first_seen_at, last_seen_at) VALUES ('ev-blocked','rel-a',1,?,'blocked_needs_input','child','t','turn-1','completed',?,'final','2026-10-06T01:00:00Z','2026-10-06T01:00:00Z')", strings.Repeat("0", 64), "{\"outcome\":\"blocked_needs_input\",\"reason\":\"planted\",\"detail\":\"planted\"}") + // The parent's decision reply answered that event, and its note is the reason. + improveTestInsert(t, db, "INSERT INTO events (event_id, relationship_id, execution_generation, revision_hash, outcome, producer, turn_thread_id, turn_id, turn_status, receipt, stage, first_seen_at, last_seen_at) VALUES ('ev-reply','rel-a',1,?,'decision_reply','parent','t','turn-1','completed',?,'final','2026-10-06T02:00:00Z','2026-10-06T02:00:00Z')", strings.Repeat("0", 64), "{\"decision\":\"answer\",\"answersEvent\":\"ev-blocked\",\"note\":\"size over the line\"}") + // A blockage no decision answered keeps its bare outcome name. + improveTestInsert(t, db, "INSERT INTO events (event_id, relationship_id, execution_generation, revision_hash, outcome, producer, turn_thread_id, turn_id, turn_status, receipt, stage, first_seen_at, last_seen_at) VALUES ('ev-lonely','rel-b',1,?,'blocked_needs_input','child','t','turn-1','completed',?,'final','2026-10-06T03:00:00Z','2026-10-06T03:00:00Z')", strings.Repeat("0", 64), "{\"outcome\":\"blocked_needs_input\"}") + }) + out := improveParseConfig(t, s, improveParseDraftsDir(t, s)) + + bundle := improveParseCollect(t, s, out) + splits := improveTestRecordsOf(bundle, improveKindSplit) + if len(splits) != 2 { + t.Fatalf("split records = %d, want one per relationship: %+v", len(splits), splits) + } + byWhere := map[string]improveRecord{} + for _, record := range splits { + byWhere[record.Where] = record + } + answered, ok := byWhere["rel-a"] + if !ok { + t.Fatalf("no record for the answered blockage: %+v", splits) + } + if answered.What != "size over the line" { + t.Errorf("the answered blockage kept %q, want the decision reply's note", answered.What) + } + lonely, ok := byWhere["rel-b"] + if !ok { + t.Fatalf("no record for the unanswered blockage: %+v", splits) + } + if lonely.What != "blocked_needs_input" { + t.Errorf("the unanswered blockage kept %q, want the bare outcome name", lonely.What) + } +} + +// TestImproveParseTimesCompareAsInstants covers C3: a stored time is compared as the instant it +// names, so fractional seconds and offsets cannot reverse the order, and a value that is not an +// instant sorts after one that is while keeping its text. +func TestImproveParseTimesCompareAsInstants(t *testing.T) { + s := improveTestSetup(t) + improveTestStore(t, s, func(t *testing.T, db *sql.DB) { + improveTestInsert(t, db, "INSERT INTO refusals (at, relationship_id, event_id, reason, detail) VALUES ('2026-10-06T00:00:00.5Z','rel-a','ev-a','fraction','half a second in')") + improveTestInsert(t, db, "INSERT INTO refusals (at, relationship_id, event_id, reason, detail) VALUES ('2026-10-06T00:00:00Z','rel-b','ev-b','fraction','the whole second')") + improveTestInsert(t, db, "INSERT INTO refusals (at, relationship_id, event_id, reason, detail) VALUES ('2026-10-06T01:00:00Z','rel-c','ev-c','offset','one hour in')") + improveTestInsert(t, db, "INSERT INTO refusals (at, relationship_id, event_id, reason, detail) VALUES ('2026-10-06T09:30:00+09:00','rel-d','ev-d','offset','half an hour in, spelled with an offset')") + improveTestInsert(t, db, "INSERT INTO refusals (at, relationship_id, event_id, reason, detail) VALUES ('not-a-time','rel-e','ev-e','unparsed','a value that is not an instant')") + improveTestInsert(t, db, "INSERT INTO refusals (at, relationship_id, event_id, reason, detail) VALUES ('2026-10-06T01:00:00Z','rel-f','ev-f','unparsed','a value that is an instant')") + }) + out := improveParseConfig(t, s, improveParseDraftsDir(t, s)) + + bundle := improveParseCollect(t, s, out) + byKey := map[string]improveRecord{} + for _, record := range improveTestRecordsOf(bundle, improveKindRefusal) { + byKey[record.Key] = record + } + cases := []struct { + reason, first, last string + }{ + {"fraction", "2026-10-06T00:00:00Z", "2026-10-06T00:00:00.5Z"}, + {"offset", "2026-10-06T09:30:00+09:00", "2026-10-06T01:00:00Z"}, + {"unparsed", "2026-10-06T01:00:00Z", "not-a-time"}, + } + for _, want := range cases { + got, ok := byKey[want.reason] + if !ok { + t.Errorf("no record for the %s case: %+v", want.reason, byKey) + continue + } + if got.FirstAt != want.first || got.LastAt != want.last { + t.Errorf("the %s case = %q..%q, want %q..%q", want.reason, got.FirstAt, got.LastAt, want.first, want.last) + } + } +} diff --git a/internal/manage/improve_collect_test.go b/internal/manage/improve_collect_test.go index 19da8b6cc..770b976e9 100644 --- a/internal/manage/improve_collect_test.go +++ b/internal/manage/improve_collect_test.go @@ -595,7 +595,7 @@ func TestImproveCollectNormalizesCriteriaAndDrafts(t *testing.T) { if err := os.MkdirAll(drafts, 0o755); err != nil { t.Fatal(err) } - improveTestWrite(t, filepath.Join(drafts, "one.json"), "{\"issue\":\"CRW-1\",\"project\":\"project-a\",\"title\":\"one\"}") + improveTestWrite(t, filepath.Join(drafts, "one.json"), "{\"schema\":\"crw-issue-draft/1\",\"fingerprint\":\"one\",\"project\":\"project-a\",\"title\":\"one\"}") improveTestConfig(t, s, map[string]any{"manage": map[string]any{"improve": map[string]any{ "sources": map[string]any{ "relay": map[string]any{"path": s.stateDir}, @@ -614,8 +614,8 @@ func TestImproveCollectNormalizesCriteriaAndDrafts(t *testing.T) { if criteria[0].Where != "digest-1" || criteria[0].Count != 2 || criteria[0].FirstAt != "2026-10-06T01:00:00Z" || criteria[0].LastAt != "2026-10-06T02:00:00Z" { t.Errorf("the digest-1 record = %+v", criteria[0]) } - if got := improveTestRecordsOf(bundle, improveKindDraft); len(got) != 1 || got[0].Key != "CRW-1" || got[0].Where != "project-a" { - t.Errorf("draft records = %+v, want one CRW-1 at project-a", got) + if got := improveTestRecordsOf(bundle, improveKindDraft); len(got) != 1 || got[0].Key != "one" || got[0].Where != "project-a" { + t.Errorf("draft records = %+v, want the one fingerprint at project-a", got) } if got := improveTestSourceOf(t, bundle, improveKindIntervention); got.State != improveStateMissing { t.Errorf("the unconfigured intervention source = %+v, want missing", got) diff --git a/internal/runtime/install/backup_cmd.go b/internal/runtime/install/backup_cmd.go new file mode 100644 index 000000000..717d11101 --- /dev/null +++ b/internal/runtime/install/backup_cmd.go @@ -0,0 +1,161 @@ +package install + +// backup_cmd.go is `crw install backup-state --to `: the operator route to the stopped byte copy +// outside an install (CRW-862, docs/port/decisions.md section 83 item 4 and slice S7). The install route reaches +// backupState only when the swap actually introduces the additive DAG zone or an ordinary index +// (swapgate.AdditiveArrivalOnly), so an operator cannot take the default artifact on demand; this command takes +// the same artifact through the same routine. It is routed before the generic install options, like features, +// config and migrate-state: it has its own flag and prints its own JSON report. + +import ( + "context" + "errors" + "flag" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + + "github.com/thisisjun786/codex-relay-workflow/internal/contract" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/store" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/store/ownership" + "github.com/thisisjun786/codex-relay-workflow/internal/runtime/definition" + "github.com/thisisjun786/codex-relay-workflow/internal/runtime/record" + "github.com/thisisjun786/codex-relay-workflow/internal/runtime/scope" + "github.com/thisisjun786/codex-relay-workflow/internal/runtime/swapgate" +) + +// backupStateUsage is this command's own help. The legacy `crw install` usage line is a frozen contract and +// does not name this command. +const backupStateUsage = `usage: crw install backup-state --to [--state ] [--socket ] + +Copy the whole relay state directory to outside an install: the same stopped byte copy the install +route takes before a swap that brings the additive DAG zone or an ordinary index, with the same manifest +beside it and the same integrity_check gate. The relay service must be stopped: this command refuses while +the relay's own service reading reports it running, and it holds the store's write gate exclusively while +it copies, so no relay write reaches the store under the copy. and its manifest must not exist and +must not lie inside the state directory or the runtime destination tree, exactly as on the install route. + +The manifest records the copy's integrityCheck and whether it is a restoreCandidate (true only when +PRAGMA integrity_check passed on a scratch duplicate of the copy, never on the copy itself). +` + +// serviceReading is the relay's own service reading, the same signal the swap gate takes +// (swapgate.DaemonCell over the relay's `service status`). It is a seam: a test fakes a running service +// without starting a daemon, which the packet requires (the "service running" case is a faked reading). +var serviceReading = func(ctx context.Context, o Options) Object { + rec, _ := record.Load(o.RecordPath, definition.Version).Value.(Object) + executable := filepath.Join(o.Dest, "current", "bin", definition.Relay) + if install := selectedInstall(rec, definition.Relay); install != nil { + if entry, ok := record.Get(install, "entryPoint").(string); ok && entry != "" { + executable = entry + } + } + return swapgate.DaemonCell(scope.Relay(ctx, []string{"service", "status"}, executable, o.Socket, o.State, o.Env, false, 0)) +} + +// runBackupState is `crw install backup-state`. It parses its own flags, resolves the same paths the other +// commands read, and takes the backup. +func runBackupState(ctx context.Context, args []string, env scope.Env, stdout, stderr io.Writer) int { + flags := flag.NewFlagSet("crw install backup-state", flag.ContinueOnError) + flags.SetOutput(stderr) + to := flags.String("to", "", "the directory the whole relay state directory is copied to (required)") + codexHome := flags.String("codex-home", "", "the Codex home (default $CODEX_HOME or ~/.codex)") + recordPath := flags.String("record", "", "the host record (default $XDG_STATE_HOME/codex-relay-workflow/host-record.json)") + issue := flags.String("issue", DefaultIssue, "the issue recorded as the evidence of this run") + socket := flags.String("socket", "", "the App Server socket the relay service reading is made against") + state := flags.String("state", "", "the relay state directory to copy") + for _, arg := range args { + if arg == "--help" || arg == "-h" || arg == "help" { + fmt.Fprint(stdout, backupStateUsage) + return OK + } + } + positional, err := parse(flags, args) + if err != nil { + if errors.Is(err, flag.ErrHelp) { + return OK + } + fmt.Fprintln(stderr, "crw install backup-state: error: "+err.Error()) + return Usage + } + if len(positional) > 0 { + fmt.Fprintln(stderr, "crw install backup-state: error: unrecognized arguments: "+positional[0]) + return Usage + } + if *to == "" { + fmt.Fprintln(stderr, "crw install backup-state: error: argument --to: the directory the state directory is copied to is required") + return Usage + } + if why := notUTF8([]namedValue{{"--to", *to}, {"--state", *state}, {"--socket", *socket}, {"--codex-home", *codexHome}, {"--record", *recordPath}}); why != "" { + fmt.Fprintln(stderr, "crw install backup-state: error: "+why) + return Usage + } + o, err := options(env, *codexHome, *recordPath, *issue, *socket, *state) + if err != nil { + fmt.Fprintln(stderr, "crw install backup-state: error: "+err.Error()) + return Usage + } + if refused := foreignDestination(o, "backup-state"); refused != nil { + if err := contract.Emit(stdout, refused); err != nil { + fmt.Fprintln(stderr, "crw install backup-state: "+err.Error()) + } + return Refused + } + dest, err := absolute(*to) + if err != nil { + fmt.Fprintln(stderr, "crw install backup-state: error: --to: "+err.Error()) + return Usage + } + result, code := BackupState(ctx, o, dest) + if err := contract.Emit(stdout, result); err != nil { + fmt.Fprintln(stderr, "crw install backup-state: "+err.Error()) + return Refused + } + return code +} + +// BackupState takes the stopped byte copy outside an install. It refuses while the relay service runs, holds the +// store's write gate exclusively for the whole copy, and takes the same artifact backupState makes. +func BackupState(ctx context.Context, o Options, dest string) (Object, int) { + reading := serviceReading(ctx, o) + if record.Get(reading, "readable") != true { + return refusedResult("backup-state", "whether the relay service is running could not be established: "+record.Text(reading, "detail"), + "nothing was read and nothing was copied: a backup is taken only while the service is known stopped.") + } + if record.Get(reading, "answer") == scope.Running { + return refusedResult("backup-state", "the relay service is running ("+record.Text(reading, "detail")+")", + "nothing was read and nothing was copied: stop the relay service and run this command again.") + } + selection, err := store.ResolveStateDir(o.State, o.Socket) + if err != nil { + return refusedResult("backup-state", "the state directory could not be resolved: "+err.Error(), "nothing was read and nothing was copied.") + } + // The relay's write gate sits beside the database as SQLite resolves it (holdGate), which is the state + // directory's own relay.sqlite3 in the ordinary layout and the linked file's directory otherwise. + resolved, err := resolveStorePath(selection.DBPath()) + if err != nil { + return refusedResult("backup-state", "the store's database path could not be resolved: "+err.Error(), "nothing was read and nothing was copied.") + } + gatePath := filepath.Join(filepath.Dir(resolved), "write-gate.lock") + if _, err := os.Lstat(gatePath); errors.Is(err, fs.ErrNotExist) { + return refusedResult("backup-state", "no relay store write gate exists at "+gatePath+", so the store cannot be held still for a copy", + "nothing was read and nothing was copied.") + } else if err != nil { + return refusedResult("backup-state", gatePath+" could not be examined: "+err.Error(), "nothing was read and nothing was copied.") + } + gate, err := ownership.Lock(gatePath, true, false) + if err != nil { + return refusedResult("backup-state", "the store's write gate could not be taken exclusively ("+err.Error()+"), so a relay writer may be reaching the store", + "nothing was read and nothing was copied: stop the relay service and run this command again.") + } + defer gate.Close() + backup, err := backupState(ctx, o, dest, backupForOperator) + if err != nil { + return Object{field("command", "backup-state"), field("applied", false), field("refused", err.Error()), field("stateBackup", backup), + field("note", "nothing but what is reported under stateBackup was written: a partial copy stays where it is and is never vouched for by a manifest it did not write.")}, Refused + } + return Object{field("command", "backup-state"), field("applied", true), field("stateBackup", backup), + field("note", "the state directory was copied byte for byte with the relay service stopped and the store's write gate held exclusively; the copy and its manifest stay where they are, and a rerun needs a new destination.")}, OK +} diff --git a/internal/runtime/install/backup_cmd_test.go b/internal/runtime/install/backup_cmd_test.go new file mode 100644 index 000000000..314a9c9e5 --- /dev/null +++ b/internal/runtime/install/backup_cmd_test.go @@ -0,0 +1,280 @@ +package install_test + +// backup_cmd_test.go is the operator command of CRW-862: `crw install backup-state --to ` takes the same +// stopped byte copy outside an install, refuses while the relay service runs, and holds the store's write gate +// exclusively while it copies. The service reading is faked, as the packet requires (the "service running" case is a +// faked reading): no daemon is started and no live relay state is touched. + +import ( + "bytes" + "context" + "database/sql" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/thisisjun786/codex-relay-workflow/internal/relay/store/ownership" + "github.com/thisisjun786/codex-relay-workflow/internal/runtime/golden" + "github.com/thisisjun786/codex-relay-workflow/internal/runtime/install" + "github.com/thisisjun786/codex-relay-workflow/internal/runtime/scope" + "github.com/thisisjun786/codex-relay-workflow/internal/testsupport" + + _ "modernc.org/sqlite" +) + +// backupStateHost is a host with a relay state directory holding a whole store (a database, its write gate and its +// ownership mirror), as a stopped relay leaves one. +func backupStateHost(t *testing.T) *host { + t.Helper() + h := newHost(t) + testsupport.Create(t, filepath.Join(h.relayState, "relay.sqlite3"), "", "go") + write(t, filepath.Join(h.relayState, "ledger.log"), "line one\n") + return h +} + +// The standalone backup records the route that took it: the operator command takes no swap and reads no in-flight +// cells, so its manifest and result must not claim the guarded install route's checks. Before the fix both carried +// the install route's words, which could be mistaken for evidence from the swap gate. +// +// sequential: replaces the service reading. +func TestTheOperatorCommandRecordsItsOwnRoute(t *testing.T) { + h := backupStateHost(t) + restore := install.ReplaceServiceReading(func(context.Context, install.Options) install.Object { + return install.ServiceCell(scope.Stopped, true, "the service answered and reports itself not running") + }) + defer restore() + dest := filepath.Join(h.home, "operator-backup") + result, code := install.BackupState(context.Background(), h.options(), dest) + if code != install.OK || at(result, "stateBackup", "takenBy") != "operator" { + t.Fatalf("exit %d\n%s", code, golden.Canon(result)) + } + var manifest struct { + TakenBy string `json:"takenBy"` + Meaning string `json:"meaning"` + } + if err := json.Unmarshal(mustRead(t, dest+install.ManifestSuffix), &manifest); err != nil { + t.Fatal(err) + } + if manifest.TakenBy != "operator" || strings.Contains(manifest.Meaning, "before the swap") { + t.Fatalf("the manifest claims the install route: %+v", manifest) + } +} + +// ---- CRW-862 review fixes ---- + +// The write gate the command locks is the one the relay writers lock: beside the database as SQLite resolves it +// (holdGate), which for a linked store is the linked file directory and not the state directory root. Before the fix +// the command locked the state directory gate, so a writer holding the resolved gate kept writing under the copy. +// +// sequential: takes the real write gate and replaces the service reading. +func TestTheOperatorCommandLocksTheGateBesideTheResolvedStore(t *testing.T) { + h := backupStateHost(t) + real := filepath.Join(h.relayState, "data", "real.sqlite3") + testsupport.Create(t, real, "", "go") + if err := os.Remove(filepath.Join(h.relayState, "relay.sqlite3")); err != nil { + t.Fatal(err) + } + if err := os.Symlink(real, filepath.Join(h.relayState, "relay.sqlite3")); err != nil { + t.Fatal(err) + } + restore := install.ReplaceServiceReading(func(context.Context, install.Options) install.Object { + return install.ServiceCell(scope.Stopped, true, "the service answered and reports itself not running") + }) + defer restore() + // a relay writer holds the gate beside the resolved database + held, err := ownership.Lock(filepath.Join(filepath.Dir(real), "write-gate.lock"), true, true) + if err != nil { + t.Fatal(err) + } + defer held.Close() + dest := filepath.Join(h.home, "operator-backup") + result, code := install.BackupState(context.Background(), h.options(), dest) + if code != install.Refused || at(result, "applied") != false || !strings.Contains(text(at(result, "refused")), "write gate") { + t.Fatalf("a held resolved gate must refuse: exit %d\n%s", code, golden.Canon(result)) + } + nothingAt(t, dest) +} + +// A --state holding a byte that is not UTF-8 is refused as usage: the manifest would otherwise record a source path +// with the bytes replaced, naming no real directory. Before the fix only --to was validated. +// +// sequential: none. +func TestTheOperatorCommandRefusesANonUTF8State(t *testing.T) { + h := backupStateHost(t) + var stdout, stderr strings.Builder + code := install.Main(context.Background(), []string{"backup-state", "--to", filepath.Join(h.home, "b"), "--state", string([]byte{0xff, 0xfe})}, h.env, &stdout, &stderr) + if code != install.Usage || !strings.Contains(stderr.String(), "not UTF-8") { + t.Fatalf("exit %d stderr=%q", code, stderr.String()) + } +} + +// The integrity gate duplicates a store of any size with a fixed buffer: a multi-megabyte store is duplicated and +// checked without the process holding it in memory, and the copy is byte-identical. +// +// sequential: replaces the service reading. +func TestTheOperatorCommandDuplicatesALargeStoreWithoutHoldingItInMemory(t *testing.T) { + // a store several times the 1 MiB copy buffer, written through SQLite so it is a real database + h := newHost(t) + big := filepath.Join(h.relayState, "relay.sqlite3") + testsupport.Create(t, big, "", "go") + db, err := sql.Open("sqlite", big) + if err != nil { + t.Fatal(err) + } + db.SetMaxOpenConns(1) + if _, err := db.Exec("CREATE TABLE filler (id INTEGER PRIMARY KEY, blob BLOB)"); err != nil { + t.Fatal(err) + } + for i := 0; i < 64; i++ { + if _, err := db.Exec("INSERT INTO filler (blob) VALUES (randomblob(65536))"); err != nil { + t.Fatal(err) + } + } + if err := db.Close(); err != nil { + t.Fatal(err) + } + info, err := os.Stat(big) + if err != nil || info.Size() < 4<<20 { + t.Fatalf("the fixture store is %v bytes: %v", info, err) + } + restore := install.ReplaceServiceReading(func(context.Context, install.Options) install.Object { + return install.ServiceCell(scope.Stopped, true, "the service answered and reports itself not running") + }) + defer restore() + dest := filepath.Join(h.home, "operator-backup") + result, code := install.BackupState(context.Background(), h.options(), dest) + if code != install.OK || at(result, "stateBackup", "restoreCandidate") != true { + t.Fatalf("exit %d\n%s", code, golden.Canon(result)) + } + if !bytes.Equal(mustRead(t, filepath.Join(dest, "relay.sqlite3")), mustRead(t, big)) { + t.Fatal("the copy of a large store is not byte-identical") + } +} + +// The command takes the same artifact outside an install: the copy, its manifest and the integrity gate, with the +// service stopped and the write gate held exclusively. +// +// sequential: replaces the service-reading seam. +func TestTheOperatorCommandTakesTheArtifactOutsideAnInstall(t *testing.T) { + h := backupStateHost(t) + restore := install.ReplaceServiceReading(func(context.Context, install.Options) install.Object { + return install.ServiceCell(scope.Stopped, true, "the service answered and reports itself not running") + }) + defer restore() + dest := filepath.Join(h.home, "operator-backup") + result, code := install.BackupState(context.Background(), h.options(), dest) + if code != install.OK || at(result, "applied") != true || at(result, "stateBackup", "made") != true || at(result, "stateBackup", "restoreCandidate") != true { + t.Fatalf("exit %d\n%s", code, golden.Canon(result)) + } + if at(result, "stateBackup", "integrityCheck") != "ok" { + t.Fatalf("the artifact records its integrity: %s", golden.Canon(at(result, "stateBackup"))) + } + // the copy and its manifest are there, and the manifest records the gate + var manifest struct { + IntegrityCheck string `json:"integrityCheck"` + RestoreCandidate bool `json:"restoreCandidate"` + } + if err := json.Unmarshal(mustRead(t, dest+install.ManifestSuffix), &manifest); err != nil { + t.Fatal(err) + } + if !manifest.RestoreCandidate || manifest.IntegrityCheck != "ok" { + t.Fatalf("manifest: %+v", manifest) + } +} + +// The command refuses while the relay service reads as running: it copies nothing and writes nothing. +// +// sequential: replaces the service-reading seam. +func TestTheOperatorCommandRefusesWhileTheServiceRuns(t *testing.T) { + h := backupStateHost(t) + restore := install.ReplaceServiceReading(func(context.Context, install.Options) install.Object { + return install.ServiceCell(scope.Running, true, "the service answered and reports itself running") + }) + defer restore() + dest := filepath.Join(h.home, "operator-backup") + result, code := install.BackupState(context.Background(), h.options(), dest) + if code != install.Refused || at(result, "applied") != false || !strings.Contains(text(at(result, "refused")), "running") { + t.Fatalf("exit %d\n%s", code, golden.Canon(result)) + } + nothingAt(t, dest) + nothingAt(t, dest+install.ManifestSuffix) +} + +// A service reading that could not be taken refuses rather than assuming the service is stopped. +// +// sequential: replaces the service-reading seam. +func TestTheOperatorCommandRefusesAnUnreadableServiceReading(t *testing.T) { + h := backupStateHost(t) + restore := install.ReplaceServiceReading(func(context.Context, install.Options) install.Object { + return install.ServiceCell("UNREADABLE", false, "the service could not be asked") + }) + defer restore() + dest := filepath.Join(h.home, "operator-backup") + result, code := install.BackupState(context.Background(), h.options(), dest) + if code != install.Refused || at(result, "applied") != false || !strings.Contains(text(at(result, "refused")), "could not be established") { + t.Fatalf("exit %d\n%s", code, golden.Canon(result)) + } + nothingAt(t, dest) +} + +// The command holds the store's write gate exclusively: while a relay writer holds it, the command refuses instead of +// copying a store a writer may be reaching. +// +// sequential: takes the real write gate. +func TestTheOperatorCommandRefusesWhileTheWriteGateIsHeld(t *testing.T) { + h := backupStateHost(t) + restore := install.ReplaceServiceReading(func(context.Context, install.Options) install.Object { + return install.ServiceCell(scope.Stopped, true, "the service answered and reports itself not running") + }) + defer restore() + held, err := ownership.Lock(filepath.Join(h.relayState, "write-gate.lock"), true, false) + if err != nil { + t.Fatal(err) + } + defer held.Close() + dest := filepath.Join(h.home, "operator-backup") + result, code := install.BackupState(context.Background(), h.options(), dest) + if code != install.Refused || at(result, "applied") != false || !strings.Contains(text(at(result, "refused")), "write gate") { + t.Fatalf("exit %d\n%s", code, golden.Canon(result)) + } + nothingAt(t, dest) +} + +// The command's own flag and its own help surface, and the frozen install usage line does not name it. +// +// sequential: t.Setenv("PATH") is process-wide. +func TestTheOperatorCommandSurface(t *testing.T) { + fakeBin := t.TempDir() + writeExecutable(t, filepath.Join(fakeBin, "codex"), []byte("#!/bin/sh\necho codex-cli 0.154.0\n"), 0o755) + t.Setenv("PATH", fakeBin+string(os.PathListSeparator)+os.Getenv("PATH")) + h := backupStateHost(t) + common := []string{"--codex-home", h.codex, "--record", h.record, "--socket", h.fake.SocketPath, "--state", h.relayState} + run := func(args ...string) (string, string, int) { + var stdout, stderr strings.Builder + code := install.Main(context.Background(), append(args, common...), h.env, &stdout, &stderr) + return stdout.String(), stderr.String(), code + } + // help + if stdout, _, code := run("backup-state", "--help"); code != install.OK || !strings.Contains(stdout, "crw install backup-state --to ") { + t.Fatalf("help: exit %d\n%s", code, stdout) + } + // --to is required + if _, stderr, code := run("backup-state"); code != install.Usage || !strings.Contains(stderr, "--to") { + t.Fatalf("missing --to: exit %d %s", code, stderr) + } + // the frozen usage line still names only the commands it named before this one arrived + var stdout, stderr strings.Builder + if code := install.Main(context.Background(), []string{"help"}, nil, &stdout, &stderr); code != install.OK { + t.Fatalf("install help: exit %d", code) + } + want := "usage: crw install {install,update,rollback,remove,status,register-mcp,hook,register-service} ...\n" + if stdout.String() != want { + t.Fatalf("install usage = %q, want %q", stdout.String(), want) + } + // the invalid-choice list names it + if _, stderr, code := run("unpack"); code != install.Usage || !strings.Contains(stderr, "backup-state") { + t.Fatalf("invalid choice: exit %d %s", code, stderr) + } +} diff --git a/internal/runtime/install/cli.go b/internal/runtime/install/cli.go index 484213a7a..95758fd4e 100644 --- a/internal/runtime/install/cli.go +++ b/internal/runtime/install/cli.go @@ -8,6 +8,7 @@ import ( "io" "os" "path/filepath" + "slices" "strconv" "strings" "unicode/utf8" @@ -29,14 +30,18 @@ const DefaultIssue = "CRW-158" const backupHelp = "the directory the whole relay state directory is copied to (copy only, byte for byte, recorded) before a swap that brings the additive DAG zone, or ordinary indexes on tables the store already holds, to a store that lacks them; the acknowledgement that route needs" // Commands are `crw install`'s subcommands. -var Commands = []string{"install", "update", "rollback", "remove", "status", "register-mcp", "hook", "register-service", "features", "config", "migrate-state"} +var Commands = []string{"install", "update", "rollback", "remove", "status", "register-mcp", "hook", "register-service", "features", "config", "migrate-state", "backup-state"} + +// ownUsage are the commands with a help surface of their own, so the frozen legacy usage line still +// names only the commands it named before they arrived. +var ownUsage = []string{"features", "config", "migrate-state", "backup-state"} func usage(w io.Writer) { // The installer help is a frozen contract; features, config and migrate-state have their own help // surfaces, so the frozen line still names only the commands it named before. var legacy []string for _, command := range Commands { - if command != "features" && command != "config" && command != "migrate-state" { + if !slices.Contains(ownUsage, command) { legacy = append(legacy, command) } } @@ -97,6 +102,11 @@ func Main(ctx context.Context, args []string, env scope.Env, stdout, stderr io.W // flags and prints its own text or JSON report, not the installer's envelope. return migrate.Run(ctx, rest, env, stdout, stderr) } + if command == "backup-state" { + // Routed before the generic install options, like features, config and migrate-state: it takes + // its own flag and prints its own JSON report (CRW-862). + return runBackupState(ctx, rest, env, stdout, stderr) + } flags := flag.NewFlagSet("crw install "+command, flag.ContinueOnError) flags.SetOutput(stderr) codexHome := flags.String("codex-home", "", "the Codex home (default $CODEX_HOME or ~/.codex)") diff --git a/internal/runtime/install/export_test.go b/internal/runtime/install/export_test.go index 713cbed0d..1bd102d67 100644 --- a/internal/runtime/install/export_test.go +++ b/internal/runtime/install/export_test.go @@ -48,6 +48,65 @@ func ReplaceStateBackupListed(listed func() error) (restore func()) { return func() { stateBackupListed = saved } } +// ReplaceStateBackupWalk makes each listing of the state directory call walk for every entry, after the directory +// was read and before the entry's own information is asked, until restored: a test makes the store's write-ahead log +// go between the walk's read and its Info (CRW-862, PR #735 P1 3). +func ReplaceStateBackupWalk(walk func(path string) error) (restore func()) { + saved := stateBackupWalk + stateBackupWalk = walk + return func() { stateBackupWalk = saved } +} + +// ReplaceStateBackupVerified makes the state-directory backup call verified once every byte of the copy is in place +// and verified and before the integrity gate runs, until restored. +func ReplaceStateBackupVerified(verified func() error) (restore func()) { + saved := stateBackupVerified + stateBackupVerified = verified + return func() { stateBackupVerified = saved } +} + +// ReplaceIntegrityCheck makes the backup's integrity gate go through check until restored, so a test that cannot +// corrupt a real store substitutes the answer instead. +func ReplaceIntegrityCheck(check func(ctx context.Context, dest string, copied []BackedUp) (string, error)) (restore func()) { + saved := integrityCheckPath + integrityCheckPath = func(ctx context.Context, dest string, copied []backedUp) (string, error) { + return check(ctx, dest, copied) + } + return func() { integrityCheckPath = saved } +} + +// BackedUp is one entry of a backup's listing, for a test seam that judges the copied entries. +type BackedUp = backedUp + +// ReplaceServiceReading makes the operator command's relay service reading go through reading until restored, so a +// test fakes a running service without starting a daemon. +func ReplaceServiceReading(reading func(ctx context.Context, o Options) Object) (restore func()) { + saved := serviceReading + serviceReading = reading + return func() { serviceReading = saved } +} + +// ServiceCell is swapgate.DaemonCell's answer for a service reading, so a test can fake one without a daemon. +func ServiceCell(answer any, readable bool, detail string) Object { + return Object{field("answer", answer), field("readable", readable), field("detail", detail), field("command", nil), field("evidence", nil)} +} + +// SidecarsFor is the store's resolved -wal and -shm paths, the identity the listing and the comparison use. +func SidecarsFor(source, dbPath string) (wal, shm string) { + s := sidecarsFor(source, dbPath) + return s.wal, s.shm +} + +// ScratchNeed is the bytes the integrity gate's scratch duplicate needs for a backup whose copied files have +// these sizes and names. +func ScratchNeed(entries map[string]int64) int64 { + var list []backedUp + for path, size := range entries { + list = append(list, backedUp{Path: path, Kind: "file", Size: size}) + } + return scratchNeed(list) +} + // StoreSidecarWal is the manifest's reading of the store's write-ahead log, from what happened to it while the backup // was made: its bytes were copied, it was listed and had gone by its copy, it was absent from the first listing and // appeared in the second so it was not copied, or it was absent from both. The appeared reading cannot be reached end @@ -58,12 +117,18 @@ func StoreSidecarWal(copied, gone, appeared bool) string { } // ListingDiff is how two readings of a state directory differ, or "": the comparison the backup's verification makes, -// with the store's two sidecars left out. A white-box test pins the sidecars' exclusion here. +// with the store's write-ahead log compared by its identity and its shared-memory index never listed. A white-box test +// pins both here. The paths are the state directory's own top-level names, so relay.sqlite3-wal is the store's log and +// relay.sqlite3-shm its index. func ListingDiff(a, b []string) string { toEntries := func(paths []string) []backedUp { out := make([]backedUp, 0, len(paths)) for _, p := range paths { - out = append(out, backedUp{Path: p, Kind: "file", Size: 1}) + if p == shmSidecar { + // the shared-memory index is never listed + continue + } + out = append(out, backedUp{Path: p, Kind: "file", Size: 1, storeWal: p == walSidecar}) } return out } diff --git a/internal/runtime/install/zone.go b/internal/runtime/install/zone.go index 734bc9d36..9cea1a534 100644 --- a/internal/runtime/install/zone.go +++ b/internal/runtime/install/zone.go @@ -3,6 +3,7 @@ package install import ( "context" "crypto/sha256" + "database/sql" "encoding/hex" "encoding/json" "errors" @@ -16,6 +17,8 @@ import ( "golang.org/x/sys/unix" + _ "modernc.org/sqlite" + "github.com/thisisjun786/codex-relay-workflow/internal/contract" "github.com/thisisjun786/codex-relay-workflow/internal/relay/store" "github.com/thisisjun786/codex-relay-workflow/internal/runtime/record" @@ -44,6 +47,26 @@ var stateBackupStep = func(step string) error { return nil } // one twice would change them. var stateBackupListed = func() error { return nil } +// stateBackupVerified is a seam: it is called once every byte of the copy is in place and verified, and before the +// integrity gate opens its scratch duplicate. A test damages the copy here to prove the gate refuses it (CRW-862). +var stateBackupVerified = func() error { return nil } + +// integrityCheckPath is a seam: it opens the scratch duplicate and returns SQLite's PRAGMA integrity_check answer. +// nil runs the real check. A test that cannot corrupt a real store substitutes the answer instead. +var integrityCheckPath = sqliteIntegrityCheck + +// The two routes that take the backup: the install swap gate (OPS-4.5) and the operator command (CRW-862). The +// manifest and the result say which one took the copy, because the checks they rest on are not the same. +const ( + backupForSwap = "swap" + backupForOperator = "operator" +) + +// stateBackupWalk is a seam: it is called for each entry of a listing after the directory was read and before the +// entry's own information is asked, which is where another connection to the store makes a sidecar go. It is called +// with the entry's path. +var stateBackupWalk = func(path string) error { return nil } + // syncDirectory makes a directory's entries durable; a seam so a test can see which directories were synced and when. var syncDirectory = syncPath @@ -98,19 +121,43 @@ func storeSidecarWal(copied, gone, appeared bool) string { } } -// isStoreShm is whether a listing's path is the store's shared-memory index, which a listing leaves out entirely: it -// comes and goes with any other connection to the store, and SQLite rebuilds it from the log on open. -func isStoreShm(path string) bool { return path == shmSidecar } +// storeSidecars are the store's two sidecars as SQLite resolves them: the path store.InPlaceRead +// resolves the database to, with -wal and -shm appended. A store whose database is a link has its +// sidecars beside the file the link names, wherever that lies, so every reading recognises them by +// this resolved path and never by the top-level name alone (CRW-862, PR #735 P1 2). When the store's +// path cannot be resolved the state directory's own two names are used, which is what a listing did +// before the identity was resolved. +type storeSidecars struct{ wal, shm string } + +// sidecarsFor is the store's sidecars: the resolved database path's -wal and -shm, or the state +// directory's own two names when the store's path cannot be resolved at all. +func sidecarsFor(source, dbPath string) storeSidecars { + resolved, err := resolveStorePath(dbPath) + if err != nil { + return storeSidecars{wal: filepath.Join(source, walSidecar), shm: filepath.Join(source, shmSidecar)} + } + return storeSidecars{wal: resolved + "-wal", shm: resolved + "-shm"} +} -// isStoreSidecar is whether a listing's path is one of the store's two sidecars, which the comparison of two listings -// leaves out: neither is a fact about the state directory, and a log that a checkpoint rewrote is caught by the store's -// own file, which stays in the comparison. -func isStoreSidecar(path string) bool { return path == walSidecar || path == shmSidecar } +// resolveStorePath is the path SQLite resolves the database to, whether or not its sidecars can be +// read in place: store.InPlaceRead's answer where it gives one, and otherwise the resolved path it +// examined before it refused. InPlaceRead refuses a log that holds frames with no usable index +// (an unclean shutdown), and the database's directory is what the sidecars sit in either way, so a +// linked store whose log has frames and no index still has its sidecars recognised (CRW-862). +func resolveStorePath(dbPath string) (string, error) { + if resolved, _, err := store.InPlaceRead(dbPath); err == nil { + return resolved, nil + } + return store.Realpath(dbPath) +} + +func (s storeSidecars) isWal(path string) bool { return path == s.wal } +func (s storeSidecars) isShm(path string) bool { return path == s.shm } -// listingHas is whether a listing holds path. -func listingHas(entries []backedUp, path string) bool { +// listingHasWal is whether a listing holds the store's write-ahead log. +func listingHasWal(entries []backedUp) bool { for _, e := range entries { - if e.Path == path { + if e.storeWal { return true } } @@ -126,6 +173,8 @@ type backedUp struct { SHA256 string `json:"sha256,omitempty"` LinkTarget string `json:"linkTarget,omitempty"` source string // where the bytes are read from + storeWal bool // the store's write-ahead log, whichever path it was resolved to (CRW-862) + vanished bool // the store's log went between the walk's read and its own information (CRW-862, PR #735 P1 3) } // swapGate is OPS-4.4 asked of the relay the record selects now (the candidate's own on a @@ -142,7 +191,7 @@ func swapGate(ctx context.Context, o Options, rec Object, candidate string, cand } return gate } - backup, err := backupState(ctx, o, o.StateBackup) + backup, err := backupState(ctx, o, o.StateBackup, backupForSwap) if err != nil { gate = record.Set(gate, "stateBackup", backup) blocked, _ := record.Get(gate, "blockedBy").([]any) @@ -179,7 +228,7 @@ func (r *run) refusedAfterGate(detail, note string, extra ...contract.Field) (Ob // (a failed run removes its candidate runtime, and a backup there would go with it); every regular file is // copied with its bytes, hashed while it is read, then the source is read again and every file, the listing // and the store's three files must be what was copied. A partial copy after an error stays where it is. -func backupState(ctx context.Context, o Options, dest string) (Object, error) { +func backupState(ctx context.Context, o Options, dest, route string) (Object, error) { failure := func(created bool, format string, args ...any) (Object, error) { err := fmt.Errorf(format, args...) return Object{field("requested", true), field("made", false), field("destination", dest), field("partial", created), field("kept", created), field("error", err.Error())}, err @@ -214,7 +263,9 @@ func backupState(ctx context.Context, o Options, dest string) (Object, error) { for _, e := range entries { total += e.Size } - if err := enoughRoom(filepath.Dir(dest), total); err != nil { + // The integrity gate duplicates the copied store beside the backup before the manifest is written, so + // the destination's filesystem must hold the backup and that scratch duplicate at once (CRW-862). + if err := enoughRoom(filepath.Dir(dest), total+scratchNeed(entries)); err != nil { return failure(false, "%v", err) } // the nearest directory that exists now: the directories made below it are synced, up to it, once the backup is whole @@ -237,6 +288,11 @@ func backupState(ctx context.Context, o Options, dest string) (Object, error) { if err := ctx.Err(); err != nil { return failure(true, "interrupted while copying: %v", err) } + if e.vanished { + // the log went between the listing's read and its own information: it is recorded as gone before its copy + walGone = true + continue + } target := filepath.Join(dest, filepath.FromSlash(e.Path)) if e.Kind == "dir" { if err := os.Mkdir(target, 0o700); err != nil { @@ -250,14 +306,14 @@ func backupState(ctx context.Context, o Options, dest string) (Object, error) { // The store's log was checkpointed away between the listing and this copy: the store's own file still has // to be what was read, and that check is kept, so a log that went is not a refusal. Every other file that // goes between the two readings still refuses. - if e.Path == walSidecar && errors.Is(err, fs.ErrNotExist) { + if e.storeWal && errors.Is(err, fs.ErrNotExist) { walGone = true continue } return failure(true, "%s could not be copied: %v", e.Path, err) } e.SHA256 = digest - if e.Path == walSidecar { + if e.storeWal { // Only the log's entry takes the size of the bytes copied. Every other file keeps the size of the first // listing, so a file that changed size between that listing and its copy is still a change the comparison // below catches: the store's log is the one file another connection may rewrite under the copy, and its @@ -287,7 +343,7 @@ func backupState(ctx context.Context, o Options, dest string) (Object, error) { return failure(true, "the state directory could not be read again: %v", err) } // The appeared reading is the log absent from the first listing and present in the second, so it was not copied. - walAppeared := !walCopied && !walGone && !listingHas(entries, walSidecar) && listingHas(again, walSidecar) + walAppeared := !walCopied && !walGone && !listingHasWal(entries) && listingHasWal(again) // A log that was not copied but holds frames in the second listing is a commit the copy does not hold. Another // connection wrote it while the copy was being made, and a commit that stays in the log does not touch // relay.sqlite3 until a checkpoint, so the digest check below would not see it: the backup would report success @@ -295,7 +351,7 @@ func backupState(ctx context.Context, o Options, dest string) (Object, error) { // tolerated, which is the case this route exists for: a read-only open of a store no connection holds leaves one. if !walCopied { for _, e := range again { - if e.Path == walSidecar && e.Size > walHeaderBytes { + if e.storeWal && e.Size > walHeaderBytes { return failure(true, "relay.sqlite3-wal holds commits that were not copied (%d bytes appeared or were left after the first listing), so the backup is not a copy of any one moment", e.Size) } } @@ -309,7 +365,7 @@ func backupState(ctx context.Context, o Options, dest string) (Object, error) { } now, err := digestOf(e.source) switch { - case err != nil && e.Path == walSidecar && errors.Is(err, fs.ErrNotExist): + case err != nil && e.storeWal && errors.Is(err, fs.ErrNotExist): // The log was checkpointed away by the time of the verification, so the source cannot be compared; that is // not a refusal. The copy is still read below: skipping the whole entry would let a copy that was damaged // while its source went pass as a good backup. @@ -355,12 +411,25 @@ func backupState(ctx context.Context, o Options, dest string) (Object, error) { } } } + if err := stateBackupVerified(); err != nil { + return failure(true, "%v", err) + } + // The integrity gate (CRW-862, docs/port/decisions.md section 83 item 4 and slice S7): the copy is only a + // restore candidate when PRAGMA integrity_check passes on it. The check runs on a scratch duplicate beside the + // copy, never on the copy itself, because opening a database checkpoints its write-ahead log and would change the + // bytes the backup holds. The duplicate is removed either way. + integrity, err := integrityCheckPath(ctx, dest, copied) + restoreCandidate := err == nil + if err != nil && integrity == "" { + integrity = err.Error() + } aggregate, files, bytes := aggregateOf(copied) manifest := map[string]any{ "schema": "crw-state-backup/1", "source": source, "destination": dest, "issue": o.Issue, "at": o.stamp(), "files": files, "bytes": bytes, "aggregateDigest": aggregate, "skipped": skipped, "entries": copied, - "storeSidecars": map[string]string{shmSidecar: "not copied: the WAL index SQLite rebuilds from the log on open", walSidecar: storeSidecarWal(walCopied, walGone, walAppeared)}, - "meaning": "a copy of the relay state directory taken by crw install before the swap that brings the additive DAG zone or ordinary indexes (D-01, CRW-472, OPS-4.5); copy only, byte for byte", + "storeSidecars": map[string]string{shmSidecar: "not copied: the WAL index SQLite rebuilds from the log on open", walSidecar: storeSidecarWal(walCopied, walGone, walAppeared)}, + "integrityCheck": integrity, "restoreCandidate": restoreCandidate, "takenBy": route, + "meaning": backupMeaning(route), } encoded, err := json.MarshalIndent(manifest, "", " ") if err != nil { @@ -391,9 +460,36 @@ func backupState(ctx context.Context, o Options, dest string) (Object, error) { break } } + if !restoreCandidate { + // The copy and its manifest are there and stay there: the manifest is what records that this artifact is not a + // restore candidate, so the refusal carries it rather than a partial copy with no record. + err := fmt.Errorf("the copy is not a restore candidate: PRAGMA integrity_check answered %q", integrity) + return Object{field("requested", true), field("made", false), field("destination", dest), field("partial", true), field("kept", true), + field("manifest", manifestPath), field("files", files), field("bytes", bytes), field("aggregateDigest", aggregate), + field("integrityCheck", integrity), field("restoreCandidate", false), field("error", err.Error())}, err + } return Object{field("requested", true), field("made", true), field("destination", dest), field("manifest", manifestPath), field("source", source), field("files", files), field("bytes", bytes), field("skipped", strs(skipped)), field("aggregateDigest", aggregate), field("kept", true), field("at", o.stamp()), - field("meaning", "the state directory was copied, byte for byte, after the daemon and in-flight cells answered and before the swap; nothing was moved or deleted, and the copy stays whatever happens next (a rerun needs a new destination)")}, nil + field("integrityCheck", integrity), field("restoreCandidate", restoreCandidate), field("takenBy", route), + field("meaning", backupResultMeaning(route))}, nil +} + +// backupMeaning is the manifest's account of what the copy is, in the route's own words: the install route takes +// it before the swap, and the operator command takes it outside any install, so neither describes the other's +// checks (CRW-862). +func backupMeaning(route string) string { + if route == backupForOperator { + return "a copy of the relay state directory taken by crw install backup-state outside an install, with the relay service known stopped and the store's write gate held exclusively; copy only, byte for byte" + } + return "a copy of the relay state directory taken by crw install before the swap that brings the additive DAG zone or ordinary indexes (D-01, CRW-472, OPS-4.5); copy only, byte for byte" +} + +// backupResultMeaning is the result's account of the copy, in the route's own words. +func backupResultMeaning(route string) string { + if route == backupForOperator { + return "the state directory was copied, byte for byte, with the relay service known stopped and the store's write gate held exclusively; nothing was moved or deleted, and the copy stays whatever happens next (a rerun needs a new destination)" + } + return "the state directory was copied, byte for byte, after the daemon and in-flight cells answered and before the swap; nothing was moved or deleted, and the copy stays whatever happens next (a rerun needs a new destination)" } // refuseDestination is why dest cannot hold a backup, or nil: it lies in the source, or in the install's own @@ -476,11 +572,15 @@ func enoughRoom(parent string, need int64) error { // listState walks the state directory without following links: directories, regular files, and symbolic links to // regular files (whose bytes are copied under the link's name, since a link alone would back up nothing). The store's -// own files are named as SQLite resolves them: when relay.sqlite3 is a link, the real file's -wal is copied beside it -// as relay.sqlite3-wal, so a restore opens with the commits only the log held. Its -shm is not listed at all: SQLite +// own two sidecars are recognised by the path SQLite resolves the database to, not by the top-level name: a store +// whose relay.sqlite3 is a link to a file inside the state directory has its -wal and -shm beside that file, and the +// walk leaves the -shm out and hands the -wal to the sidecar rules there as it does at the top level (CRW-862, PR +// #735 P1 2). Whichever path the log was resolved to, it is listed under the restore-compatible name +// relay.sqlite3-wal, so the copy opens with the commits only the log held. Its -shm is not listed at all: SQLite // rebuilds that index from the log on open, so a backup neither copies it nor refuses when one appears or goes. A // socket, a FIFO or a device is listed as skipped. A link to anything else is a refusal. func listState(source, dbPath string) ([]backedUp, []string, error) { + sidecars := sidecarsFor(source, dbPath) var entries []backedUp var skipped []string err := filepath.WalkDir(source, func(path string, d fs.DirEntry, err error) error { @@ -495,18 +595,39 @@ func listState(source, dbPath string) ([]backedUp, []string, error) { if rel == "." { return nil } - if isStoreShm(rel) { + // The store's shared-memory index is never listed, wherever SQLite resolved it to. Its write-ahead log is + // listed under the restore-compatible name when it is the state directory's own; a log beside a linked store + // is registered below, under that same name, so the walk leaves it here. + if sidecars.isShm(path) { + return nil + } + storeWal := sidecars.isWal(path) + if storeWal && rel != walSidecar { return nil } + if err := stateBackupWalk(path); err != nil { + return err + } info, err := d.Info() if err != nil { + // Another connection to the store may make its sidecars go between the walk's ReadDir and this Info. A log + // or index of the store that went there is recorded as gone before its copy, not a refusal; every other + // file's ENOENT, and every other error, still refuses (CRW-862, PR #735 P1 3). + if errors.Is(err, fs.ErrNotExist) && (storeWal || sidecars.isShm(path)) { + if storeWal { + // the log is recorded in the listing as gone before its copy, so the manifest's reading and the copy + // loop see what happened even though there is nothing left to copy + entries = append(entries, backedUp{Path: rel, Kind: "file", storeWal: true, vanished: true}) + } + return nil + } return err } switch mode := info.Mode(); { case mode.IsDir(): entries = append(entries, backedUp{Path: rel, Kind: "dir", Mode: uint32(mode.Perm()), CopyMode: uint32(copyMode(true, mode.Perm()))}) case mode.IsRegular(): - entries = append(entries, backedUp{Path: rel, Kind: "file", Size: info.Size(), Mode: uint32(mode.Perm()), CopyMode: uint32(copyMode(false, mode.Perm())), source: path}) + entries = append(entries, backedUp{Path: rel, Kind: "file", Size: info.Size(), Mode: uint32(mode.Perm()), CopyMode: uint32(copyMode(false, mode.Perm())), source: path, storeWal: storeWal}) case mode&fs.ModeSymlink != 0: target, err := os.Readlink(path) if err != nil { @@ -520,7 +641,7 @@ func listState(source, dbPath string) ([]backedUp, []string, error) { if err != nil || !real.Mode().IsRegular() { return fmt.Errorf("%s is a link to %s, which is not a regular file, so there is no byte-identical copy of it to make", rel, target) } - entries = append(entries, backedUp{Path: rel, Kind: "link-file", Size: real.Size(), Mode: uint32(real.Mode().Perm()), CopyMode: uint32(copyMode(false, real.Mode().Perm())), LinkTarget: target, source: resolved}) + entries = append(entries, backedUp{Path: rel, Kind: "link-file", Size: real.Size(), Mode: uint32(real.Mode().Perm()), CopyMode: uint32(copyMode(false, real.Mode().Perm())), LinkTarget: target, source: resolved, storeWal: storeWal}) default: skipped = append(skipped, rel+" ("+kindOf(mode)+")") } @@ -529,25 +650,23 @@ func listState(source, dbPath string) ([]backedUp, []string, error) { if err != nil { return nil, nil, err } - // the store's log file, when its database is a link to a file elsewhere (its -shm is never listed) - if resolved, _, err := store.InPlaceRead(dbPath); err == nil && resolved != filepath.Join(source, "relay.sqlite3") { + // the store's log file, when its database is a link to a file elsewhere in the state directory or outside it (its + // -shm is never listed): it is copied beside the link's own copy under the restore-compatible name. The resolved + // path is used whether or not its log can be read in place, so a log that holds frames with no usable index is + // still carried (CRW-862). + if resolved, err := resolveStorePath(dbPath); err == nil && resolved != filepath.Join(source, "relay.sqlite3") { have := map[string]bool{} for _, e := range entries { have[e.Path] = true } - for _, suffix := range []string{"-wal"} { - info, err := os.Lstat(resolved + suffix) - if err != nil { - continue - } - name := "relay.sqlite3" + suffix + if info, err := os.Lstat(resolved + "-wal"); err == nil { if !info.Mode().IsRegular() { - return nil, nil, fmt.Errorf("%s is not a regular file, so the store's %s cannot be copied", resolved+suffix, suffix) + return nil, nil, fmt.Errorf("%s is not a regular file, so the store's write-ahead log cannot be copied", resolved+"-wal") } - if have[name] { - return nil, nil, fmt.Errorf("the state directory already holds %s, which the store's log %s would have to be copied over", name, resolved+suffix) + if have[walSidecar] { + return nil, nil, fmt.Errorf("the state directory already holds %s, which the store's log %s would have to be copied over", walSidecar, resolved+"-wal") } - entries = append(entries, backedUp{Path: name, Kind: "link-file", Size: info.Size(), Mode: uint32(info.Mode().Perm()), CopyMode: uint32(copyMode(false, info.Mode().Perm())), LinkTarget: resolved + suffix, source: resolved + suffix}) + entries = append(entries, backedUp{Path: walSidecar, Kind: "link-file", Size: info.Size(), Mode: uint32(info.Mode().Perm()), CopyMode: uint32(copyMode(false, info.Mode().Perm())), LinkTarget: resolved + "-wal", source: resolved + "-wal", storeWal: true}) } } sort.SliceStable(entries, func(i, j int) bool { return entries[i].Path < entries[j].Path }) @@ -579,40 +698,81 @@ func kindOf(mode fs.FileMode) string { } // listingDiff is how two readings of the state directory differ, or "". + +// listingKey is one entry of a listing as the comparison keys it: every field the two readings must agree on for +// an ordinary entry. It is a struct rather than a delimiter-joined string so that a byte a file name or a link +// target may legally hold cannot shift a field boundary (CRW-862 generation 2, failure class 6). +type listingKey struct { + Path string + Kind string + Size int64 + LinkTarget string + Source string +} + +// listingIdentity is the identity of the store write-ahead log: the fields the two readings must agree on for the +// log, whose size and presence belong to the sidecar rules instead. A struct for the same reason as listingKey. +type listingIdentity struct { + Path string + Kind string + LinkTarget string + Source string +} + func listingDiff(a []backedUp, askipped []string, b []backedUp, bskipped []string) string { - // The store's two sidecars are out of the comparison: they come and go with any other connection to the store, - // and neither is a fact about the state directory. relay.sqlite3 itself stays in, so a checkpoint that rewrote - // it under the copy is still a change. - drop := func(entries []backedUp) []backedUp { - kept := make([]backedUp, 0, len(entries)) - for _, e := range entries { - if isStoreSidecar(e.Path) { - continue - } - kept = append(kept, e) - } - return kept - } - a, b = drop(a), drop(b) - // the resolved file the bytes come from is part of what a reading says: a link in the chain that was pointed - // elsewhere between the two readings is a change, even where the new file is the same size - key := func(e backedUp) string { - return fmt.Sprintf("%s|%s|%d|%s|%s", e.Path, e.Kind, e.Size, e.LinkTarget, e.source) - } - seen := map[string]bool{} + // The store's write-ahead log is compared by its identity and not by its size or its presence: another + // connection may checkpoint it away and write a fresh one under the copy, so its size and its coming and going + // belong to the sidecar rules (the manifest's reading, and the frames-left-uncopied refusal). Its identity - + // which entry it is, whether it is a link, and which file the bytes come from - is a fact about the copy, and a + // log whose link target or resolved source moved between the two readings refuses (CRW-862, PR #735 P1 1). The + // store's shared-memory index is never listed. relay.sqlite3 itself and every other file keep the full key, + // size included, so a store written under the copy is still a change. + // The keys are comparable structs, not delimiter-joined strings: "|" is a legal byte in a file name and in a + // link target, so a joined key could read two different entries as one and let a retargeted link pass (CRW-862 + // generation 2, failure class 6). + full := func(e backedUp) listingKey { + return listingKey{Path: e.Path, Kind: e.Kind, Size: e.Size, LinkTarget: e.LinkTarget, Source: e.source} + } + identity := func(e backedUp) listingIdentity { + return listingIdentity{Path: e.Path, Kind: e.Kind, LinkTarget: e.LinkTarget, Source: e.source} + } + var walA, walB *listingIdentity + seen := map[listingKey]bool{} for _, e := range a { - seen[key(e)] = true + if e.storeWal && !e.vanished { + value := identity(e) + walA = &value + continue + } + if e.storeWal { + // the log went during this reading's walk: it is absent from the reading, as a log that went + // before the walk is, so its identity is not compared + continue + } + seen[full(e)] = true } var changed []string for _, e := range b { - if !seen[key(e)] { + if e.storeWal && !e.vanished { + value := identity(e) + walB = &value + continue + } + if e.storeWal { + continue + } + if !seen[full(e)] { changed = append(changed, "new or changed: "+e.Path) } - delete(seen, key(e)) + delete(seen, full(e)) } for k := range seen { - path, _, _ := strings.Cut(k, "|") - changed = append(changed, "gone or changed: "+path) + changed = append(changed, "gone or changed: "+k.Path) + } + // the log is a change only when both readings hold it and its identity moved: its absence from one reading is + // the churn the sidecar rules already carry + if walA != nil && walB != nil && *walA != *walB { + changed = append(changed, "the store's write-ahead log changed: "+walSidecar) } if strings.Join(askipped, "\n") != strings.Join(bskipped, "\n") { changed = append(changed, "an entry that is not a file appeared or went") @@ -701,6 +861,112 @@ func digestOf(path string) (string, error) { return hex.EncodeToString(h.Sum(nil)), nil } +// sqliteIntegrityCheck is the integrity gate of a backup copy: it duplicates the copied store beside the copy and + +// scratchNeed is the bytes the integrity gate scratch duplicate needs on the backup filesystem: the +// copied store own files (relay.sqlite3 and, when copied, its log), which the gate duplicates there. +func scratchNeed(entries []backedUp) int64 { + var need int64 + for _, e := range entries { + if e.Kind != "dir" && strings.HasPrefix(e.Path, "relay.sqlite3") { + need += e.Size + } + } + return need +} + +// copyFileInto copies source to target (created, never replaced) with a fixed buffer, so a file of any size +// is duplicated without holding it in memory. The target is not synced: it is a scratch file the caller removes. +func copyFileInto(source, target string) error { + in, err := os.Open(source) + if err != nil { + return err + } + defer in.Close() + out, err := os.OpenFile(target, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) + if err != nil { + return err + } + buf := make([]byte, 1<<20) + for { + n, readErr := in.Read(buf) + if n > 0 { + if _, err := out.Write(buf[:n]); err != nil { + _ = out.Close() + return err + } + } + if errors.Is(readErr, io.EOF) { + break + } + if readErr != nil { + _ = out.Close() + return readErr + } + } + return out.Close() +} + +// asks SQLite PRAGMA integrity_check on the duplicate. The copy itself is never opened: opening a database +// checkpoints its write-ahead log and would change the bytes the backup holds, so the duplicate is the one SQLite +// touches. The duplicate holds the copy's log too when the backup carries one, so a store whose commits live only +// in the log is checked with them. The duplicate is removed either way, and the copy's bytes are not touched. The +// answer is SQLite's own, joined when it is several rows; an error is why the copy is not a restore candidate. +func sqliteIntegrityCheck(ctx context.Context, dest string, copied []backedUp) (string, error) { + scratch, err := os.MkdirTemp(filepath.Dir(dest), ".crw-integrity-") + if err != nil { + return "", fmt.Errorf("the scratch directory for the integrity check could not be created: %v", err) + } + defer os.RemoveAll(scratch) + var database string + for _, e := range copied { + if e.Kind == "dir" || !strings.HasPrefix(e.Path, "relay.sqlite3") { + continue + } + // streamed with a fixed buffer, as the backup's own copy is: a store of any size is duplicated + // without holding it in memory + if err := copyFileInto(filepath.Join(dest, filepath.FromSlash(e.Path)), filepath.Join(scratch, filepath.Base(e.Path))); err != nil { + return "", fmt.Errorf("the integrity check's duplicate of %s could not be written: %v", e.Path, err) + } + if e.Path == "relay.sqlite3" { + database = filepath.Join(scratch, filepath.Base(e.Path)) + } + } + if database == "" { + return "", errors.New("the copy holds no relay.sqlite3, so there is nothing for the integrity check to open") + } + db, err := sql.Open("sqlite", database) + if err != nil { + return "", fmt.Errorf("the integrity check could not open the copy: %v", err) + } + defer db.Close() + db.SetMaxOpenConns(1) + rows, err := db.QueryContext(ctx, "PRAGMA integrity_check") + if err != nil { + return "", fmt.Errorf("the integrity check could not run: %v", err) + } + defer rows.Close() + var answers []string + for rows.Next() { + var answer string + if err := rows.Scan(&answer); err != nil { + return "", fmt.Errorf("the integrity check's answer could not be read: %v", err) + } + answers = append(answers, answer) + } + if err := rows.Err(); err != nil { + return "", fmt.Errorf("the integrity check did not finish: %v", err) + } + if len(answers) == 0 { + return "", errors.New("the integrity check answered nothing") + } + answer := strings.Join(answers, "; ") + if answer != "ok" { + return answer, fmt.Errorf("PRAGMA integrity_check answered %q", answer) + } + return answer, nil +} + func syncPath(path string) error { d, err := os.Open(path) if err != nil { diff --git a/internal/runtime/install/zone_sidecar_test.go b/internal/runtime/install/zone_sidecar_test.go index ca077d18f..218fba173 100644 --- a/internal/runtime/install/zone_sidecar_test.go +++ b/internal/runtime/install/zone_sidecar_test.go @@ -1,8 +1,10 @@ package install_test import ( + "bytes" "context" "encoding/json" + "errors" "os" "path/filepath" "strings" @@ -10,6 +12,7 @@ import ( "github.com/thisisjun786/codex-relay-workflow/internal/runtime/golden" "github.com/thisisjun786/codex-relay-workflow/internal/runtime/install" + "github.com/thisisjun786/codex-relay-workflow/internal/testsupport" ) // The store's two sidecars under the state backup. SQLite keeps a write-ahead log and a shared-memory index beside the @@ -37,6 +40,363 @@ func withoutShm(tree map[string]string) map[string]string { return out } +// pipeCollision builds the pair the GLM review names for failure class 6: two (link target, resolved source) pairs +// whose "|"-joined strings are identical although both fields moved. base is the state directory. It returns +// target1/source1 (the first reading) and target2/source2 (the retarget), all under base. +// +// target1|source1 = base/a | base/b|base/c and target2|source2 = base/a|base/b | base/c, which join to the same +// string because "|" is a legal byte in a path. +func pipeCollision(t *testing.T, base string) (target1, source1, target2, source2 string) { + t.Helper() + trim := strings.TrimPrefix(base, "/") + source1 = filepath.Join(base, "b|", trim, "c") + source2 = filepath.Join(base, "c") + write(t, source1, "") + write(t, source2, "") + target1 = base + "/a" + target2 = base + "/a|" + base + "/b" + if err := os.Symlink(source1, filepath.Join(base, "a")); err != nil { + t.Fatal(err) + } + second := filepath.Join(base, "a|", trim, "b") + if err := os.MkdirAll(filepath.Dir(second), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(source2, second); err != nil { + t.Fatal(err) + } + return target1, source1, target2, source2 +} + +// Failure class 6: the store log is compared by its identity, and the identity must not be a "|"-joined string, because +// "|" is a legal byte in a link target and in a resolved path. With the review pair both readings join to the same +// string, so a delimiter-joined comparison sees no change and the backup succeeds although the log moved. Refuses +// after the fix. +// +// sequential: replaces the state-backup seam. +func TestTheBackupRefusesALogRetargetedToAPathThatCollidesUnderTheDelimiter(t *testing.T) { + h, _, second, old, _ := zoneInstalled(t) + zoneStore(t, h) + target1, source1, target2, source2 := pipeCollision(t, h.relayState) + if got := target1 + "|" + source1; got != target2+"|"+source2 { + t.Fatalf("the fixture does not collide: %q against %q", got, target2+"|"+source2) + } + wal := filepath.Join(h.relayState, sidecarWalName) + // zoneStore leaves no log; the store's log is this link now + _ = os.Remove(wal) + if err := os.Symlink(target1, wal); err != nil { + t.Fatal(err) + } + restore := install.ReplaceStateBackupListed(func() error { + if err := os.Remove(wal); err != nil { + return err + } + return os.Symlink(target2, wal) + }) + defer restore() + o := h.options() + o.StateBackup = backupOf(h, "pipe-wal") + result, code := install.Install(context.Background(), o, "update", install.Source{From: second}) + if code != install.Refused || at(result, "swapGate", "verdict") != "BLOCKED" || h.pointerTarget(t) != old { + t.Fatalf("a retarget that collides under the delimiter must refuse: exit %d\n%s", code, golden.Canon(at(result, "swapGate"))) + } + if !strings.Contains(text(at(result, "swapGate", "stateBackup", "error")), "write-ahead log changed") { + t.Fatalf("the refusal must name the changed log: %s", golden.Canon(at(result, "swapGate", "stateBackup"))) + } +} + +// The same construction for an ordinary symlinked file in the state directory: the two full keys (Path, Kind, Size, +// LinkTarget, source) join to the same string although the link moved. Refuses after the fix. +// +// sequential: replaces the state-backup seam. +func TestTheBackupRefusesAnOrdinaryLinkRetargetedToAPathThatCollidesUnderTheDelimiter(t *testing.T) { + h, _, second, old, _ := zoneInstalled(t) + zoneStore(t, h) + putSidecar(t, h, sidecarWalName, 32) + target1, source1, target2, source2 := pipeCollision(t, h.relayState) + if got := "linked.txt|link-file|0|" + target1 + "|" + source1; got != "linked.txt|link-file|0|"+target2+"|"+source2 { + t.Fatalf("the fixture does not collide: %q", got) + } + link := filepath.Join(h.relayState, "linked.txt") + // zoneStore already places a linked.txt; this test's link replaces it + _ = os.Remove(link) + if err := os.Symlink(target1, link); err != nil { + t.Fatal(err) + } + restore := install.ReplaceStateBackupListed(func() error { + if err := os.Remove(link); err != nil { + return err + } + return os.Symlink(target2, link) + }) + defer restore() + o := h.options() + o.StateBackup = backupOf(h, "pipe-link") + result, code := install.Install(context.Background(), o, "update", install.Source{From: second}) + if code != install.Refused || at(result, "swapGate", "verdict") != "BLOCKED" || h.pointerTarget(t) != old { + t.Fatalf("an ordinary link retarget that collides under the delimiter must refuse: exit %d\n%s", code, golden.Canon(at(result, "swapGate"))) + } +} + +// ---- CRW-862 review fixes ---- + +// The store sidecars are classified by the resolved database path even when the log cannot be read in place: an +// unclean shutdown leaves a linked store with a -wal holding frames and no -shm, and InPlaceRead refuses that state, +// but the sidecars still sit beside the file the link names. Before the fix sidecarsFor fell back to the top-level +// names, so the real log was walked as an ordinary nested file and the copy could lose its committed frames. +// +// sequential: none (a white-box reading). +func TestTheSidecarsAreResolvedEvenWhenTheLogCannotBeReadInPlace(t *testing.T) { + h, _, _, _, _ := zoneInstalled(t) + zoneStore(t, h) + real := filepath.Join(h.relayState, "data", "real.sqlite3") + testsupport.Create(t, real, "", "go") + // frames in the log, no index: InPlaceRead refuses this state + write(t, real+"-wal", strings.Repeat("x", 64)) + if err := os.Remove(filepath.Join(h.relayState, "relay.sqlite3")); err != nil { + t.Fatal(err) + } + if err := os.Symlink(real, filepath.Join(h.relayState, "relay.sqlite3")); err != nil { + t.Fatal(err) + } + wal, shm := install.SidecarsFor(h.relayState, filepath.Join(h.relayState, "relay.sqlite3")) + if wal != real+"-wal" || shm != real+"-shm" { + t.Fatalf("sidecars = (%q, %q), want (%q, %q): the resolved file sidecars, not the top-level names", wal, shm, real+"-wal", real+"-shm") + } +} + +// A log that goes during the SECOND listing walk is absence, not a change: the comparison must not read its empty +// vanished entry as a moved identity. Before the fix that entry identity (with no source) differed from the copied +// log identity and the backup refused, so the same checkpoint race passed or failed on timing. +// +// sequential: replaces the state-backup walk seam. +func TestTheBackupDoesNotRefuseALogThatGoesDuringTheSecondListing(t *testing.T) { + h, _, second, _, next := zoneInstalled(t) + zoneStore(t, h) + putSidecar(t, h, sidecarWalName, 32) + wal := filepath.Join(h.relayState, sidecarWalName) + listings := 0 + restore := install.ReplaceStateBackupWalk(func(path string) error { + if path == wal { + listings++ + // the first listing and the copy see the log; the second listing loses it between its read and its Info + if listings >= 2 { + return os.Remove(path) + } + } + return nil + }) + defer restore() + o := h.options() + o.StateBackup = backupOf(h, "second-listing-gone") + result, code := install.Install(context.Background(), o, "update", install.Source{From: second}) + if code != install.OK || at(result, "promoted") != true || h.pointerTarget(t) != next { + t.Fatalf("a log that goes during the second listing must not refuse: exit %d\n%s", code, golden.Canon(at(result, "swapGate"))) + } +} + +// The integrity gate scratch duplicate is charged against the destination free space: the preflight must hold the +// backup and the duplicate at once, so a destination that fits the backup alone refuses before it copies. +// +// sequential: none (a white-box reading). +func TestTheSpacePreflightCoversTheIntegrityScratchDuplicate(t *testing.T) { + need := install.ScratchNeed(map[string]int64{ + "relay.sqlite3": 90 << 20, + "relay.sqlite3-wal": 4 << 20, + "ledger.log": 1 << 20, + }) + if want := int64(94 << 20); need != want { + t.Fatalf("scratch need = %d, want %d (the store and its log, not the other files)", need, want) + } +} + +// ---- CRW-862: the three PR #735 P1s and the integrity gate ---- + +// P1 1: the store's write-ahead log is compared by its identity, not dropped from the comparison. A log that is +// itself a symbolic link and is retargeted between the two listings is a change: the copy would hold bytes from a file +// the state directory no longer names. Before the fix the whole WAL entry was dropped from listingDiff and this passed. +// +// sequential: replaces the state-backup seam. +func TestTheBackupRefusesAWALLinkRetargetedBetweenTheListings(t *testing.T) { + h, _, second, old, _ := zoneInstalled(t) + zoneStore(t, h) + // the store's log is a link to a file inside the state directory (an empty log, so the gate's own read of the + // store still succeeds), and the link is retargeted between the first listing and the copy + first := filepath.Join(h.relayState, "wal-one") + secondTarget := filepath.Join(h.relayState, "wal-two") + write(t, first, "") + write(t, secondTarget, "") + if err := os.Symlink(first, filepath.Join(h.relayState, sidecarWalName)); err != nil { + t.Fatal(err) + } + restore := install.ReplaceStateBackupListed(func() error { + if err := os.Remove(filepath.Join(h.relayState, sidecarWalName)); err != nil { + return err + } + return os.Symlink(secondTarget, filepath.Join(h.relayState, sidecarWalName)) + }) + defer restore() + o := h.options() + o.StateBackup = backupOf(h, "wal-retargeted") + result, code := install.Install(context.Background(), o, "update", install.Source{From: second}) + if code != install.Refused || at(result, "swapGate", "verdict") != "BLOCKED" || h.pointerTarget(t) != old { + t.Fatalf("a retargeted WAL link must refuse: exit %d\n%s", code, golden.Canon(at(result, "swapGate"))) + } + if !strings.Contains(text(at(result, "swapGate", "stateBackup", "error")), "write-ahead log changed") { + t.Fatalf("the refusal must name the changed log: %s", golden.Canon(at(result, "swapGate", "stateBackup"))) + } +} + +// P1 2: the store's sidecars are recognised by the path SQLite resolves the database to, not by the top-level name. +// With relay.sqlite3 linked to data/real.sqlite3 inside the state directory, that file's -shm going is not a refusal +// and is never listed (SQLite rebuilds it from the log on open), and its -wal follows the sidecar rules (a log that +// goes before its copy is recorded as gone, not refused). Before the fix both were walked as ordinary files, so the +// -shm vanishing refused. +// +// sequential: replaces the state-backup seam. +func TestTheBackupRecognisesTheSidecarsOfAStoreLinkedInsideTheStateDirectory(t *testing.T) { + for name, gone := range map[string]string{ + "the index goes after the listing": "data/real.sqlite3-shm", + "the log goes before its copy": "data/real.sqlite3-wal", + } { + t.Run(name, func(t *testing.T) { + h, _, second, _, next := zoneInstalled(t) + zoneStore(t, h) + real := filepath.Join(h.relayState, "data", "real.sqlite3") + testsupport.Create(t, real, "", "go") + putSidecar(t, h, filepath.Join("data", "real.sqlite3-shm"), 32768) + putSidecar(t, h, filepath.Join("data", "real.sqlite3-wal"), 32) + if err := os.Remove(filepath.Join(h.relayState, "relay.sqlite3")); err != nil { + t.Fatal(err) + } + if err := os.Symlink(real, filepath.Join(h.relayState, "relay.sqlite3")); err != nil { + t.Fatal(err) + } + restore := install.ReplaceStateBackupListed(func() error { return os.Remove(filepath.Join(h.relayState, filepath.FromSlash(gone))) }) + defer restore() + o := h.options() + o.StateBackup = backupOf(h, "linked-inside") + result, code := install.Install(context.Background(), o, "update", install.Source{From: second}) + if code != install.OK || at(result, "promoted") != true || h.pointerTarget(t) != next { + t.Fatalf("exit %d\n%s", code, golden.Canon(at(result, "swapGate"))) + } + backup := backupOf(h, "linked-inside") + nothingAt(t, filepath.Join(backup, "data", "real.sqlite3-shm")) + }) + } +} + +// P1 3: an ENOENT from the walk's Info on the store's log is recorded as gone before its copy, not a refusal. The +// seam makes the log go between the directory read and the entry's own information. Another file removed the same way +// still refuses. +// +// sequential: replaces the state-backup walk seam. +func TestTheBackupRecordsALogThatGoesDuringTheWalk(t *testing.T) { + t.Run("the store's log", func(t *testing.T) { + h, _, second, _, next := zoneInstalled(t) + zoneStore(t, h) + putSidecar(t, h, sidecarWalName, 32) + wal := filepath.Join(h.relayState, sidecarWalName) + removed := false + restore := install.ReplaceStateBackupWalk(func(path string) error { + if path == wal && !removed { + removed = true + return os.Remove(path) + } + return nil + }) + defer restore() + o := h.options() + o.StateBackup = backupOf(h, "walk-gone") + result, code := install.Install(context.Background(), o, "update", install.Source{From: second}) + if code != install.OK || at(result, "promoted") != true || h.pointerTarget(t) != next { + t.Fatalf("a log that goes during the walk is not a refusal: exit %d\n%s", code, golden.Canon(at(result, "swapGate"))) + } + backup := backupOf(h, "walk-gone") + nothingAt(t, filepath.Join(backup, sidecarWalName)) + if got := storeSidecarsOf(t, backup)[sidecarWalName]; got != "gone before its copy" { + t.Errorf("storeSidecars[%s] = %q, want %q", sidecarWalName, got, "gone before its copy") + } + }) + t.Run("another file", func(t *testing.T) { + h, _, second, old, _ := zoneInstalled(t) + zoneStore(t, h) + ledger := filepath.Join(h.relayState, "ledger.log") + removed := false + restore := install.ReplaceStateBackupWalk(func(path string) error { + if path == ledger && !removed { + removed = true + return os.Remove(path) + } + return nil + }) + defer restore() + o := h.options() + o.StateBackup = backupOf(h, "walk-gone-other") + result, code := install.Install(context.Background(), o, "update", install.Source{From: second}) + if code != install.Refused || at(result, "swapGate", "verdict") != "BLOCKED" || h.pointerTarget(t) != old { + t.Fatalf("another file's ENOENT must still refuse: exit %d\n%s", code, golden.Canon(at(result, "swapGate"))) + } + }) +} + +// The integrity gate: a copy that fails PRAGMA integrity_check is not a restore candidate. The manifest records +// integrityCheck and restoreCandidate, the command refuses, and the copy's bytes are not changed by the check (it runs +// on a scratch duplicate). A healthy copy records ok and true. +// +// sequential: replaces the state-backup integrity seam. +func TestTheBackupRecordsIntegrityAndRefusesACorruptedCopy(t *testing.T) { + t.Run("a corrupted copy", func(t *testing.T) { + h, _, second, old, _ := zoneInstalled(t) + zoneStore(t, h) + restore := install.ReplaceIntegrityCheck(func(_ context.Context, dest string, copied []install.BackedUp) (string, error) { + return "Page 5 is never used", errors.New("PRAGMA integrity_check answered a corruption") + }) + defer restore() + o := h.options() + o.StateBackup = backupOf(h, "corrupt") + result, code := install.Install(context.Background(), o, "update", install.Source{From: second}) + if code != install.Refused || at(result, "swapGate", "verdict") != "BLOCKED" || h.pointerTarget(t) != old { + t.Fatalf("a corrupted copy must refuse: exit %d\n%s", code, golden.Canon(at(result, "swapGate"))) + } + backup := backupOf(h, "corrupt") + var manifest struct { + IntegrityCheck string `json:"integrityCheck"` + RestoreCandidate bool `json:"restoreCandidate"` + } + if err := json.Unmarshal(mustRead(t, backup+install.ManifestSuffix), &manifest); err != nil { + t.Fatal(err) + } + if manifest.RestoreCandidate || !strings.Contains(manifest.IntegrityCheck, "never used") { + t.Fatalf("manifest: %+v", manifest) + } + }) + t.Run("a healthy copy", func(t *testing.T) { + h, _, second, _, next := zoneInstalled(t) + zoneStore(t, h) + o := h.options() + o.StateBackup = backupOf(h, "healthy") + result, code := install.Install(context.Background(), o, "update", install.Source{From: second}) + if code != install.OK || at(result, "promoted") != true || h.pointerTarget(t) != next { + t.Fatalf("a healthy copy must pass: exit %d\n%s", code, golden.Canon(at(result, "swapGate"))) + } + backup := backupOf(h, "healthy") + var manifest struct { + IntegrityCheck string `json:"integrityCheck"` + RestoreCandidate bool `json:"restoreCandidate"` + } + if err := json.Unmarshal(mustRead(t, backup+install.ManifestSuffix), &manifest); err != nil { + t.Fatal(err) + } + if !manifest.RestoreCandidate || manifest.IntegrityCheck != "ok" { + t.Fatalf("manifest: %+v", manifest) + } + if !bytes.Equal(mustRead(t, filepath.Join(backup, "relay.sqlite3")), mustRead(t, filepath.Join(h.relayState, "relay.sqlite3"))) { + t.Fatal("the integrity check changed the copy of the store") + } + }) +} + // storeSidecarsOf is the storeSidecars record of a backup's manifest, read from the file. func storeSidecarsOf(t *testing.T, backup string) map[string]string { t.Helper()