diff --git a/cmd/releasekit/recovery.go b/cmd/releasekit/recovery.go index 55b9cfc..dc32383 100644 --- a/cmd/releasekit/recovery.go +++ b/cmd/releasekit/recovery.go @@ -93,7 +93,11 @@ func runRecovery(ctx context.Context, invocation cli.Invocation, stdout, stderr return runReconcile(ctx, invocation, stdout, stderr) } listed, err := releasekit.ListPublicationAttempts(ctx, releasekit.PublicationAttemptsRequest{RepositoryDir: invocation.Repository, RecordDir: invocation.RecordDir}) - if invocation.JSON { + return reportAttempts(listed, err, invocation.JSON, stdout, stderr) +} + +func reportAttempts(listed releasekit.PublicationAttemptsResult, err error, jsonOutput bool, stdout, stderr io.Writer) int { + if jsonOutput { attempts := make([]attemptDocument, 0, len(listed.Attempts)) for _, item := range listed.Attempts { attempts = append(attempts, attemptDocument{RecordPath: git.DisplayDiagnostic(item.RecordPath), Evidence: safe(item.Evidence)}) diff --git a/cmd/releasekit/recovery_test.go b/cmd/releasekit/recovery_test.go index 916c110..19dd5d7 100644 --- a/cmd/releasekit/recovery_test.go +++ b/cmd/releasekit/recovery_test.go @@ -50,6 +50,32 @@ func TestRecoveryCommandsReportJSONWriteFailure(t *testing.T) { } } +func TestDiscoveryReportsPartialAttemptsAndEnumerationFailure(t *testing.T) { + listed := releasekit.PublicationAttemptsResult{ + RecordDir: "records", + Attempts: []releasekit.PublicationAttempt{{RecordPath: "good.json", Evidence: releasekit.PublicationEvidence{Tag: "v1.2.3"}}}, + Diagnostics: []*releasekit.PublicationRecoveryError{{Kind: "record_format", RecordPath: "broken.json"}}, + } + failure := &releasekit.PublicationRecoveryError{Kind: "storage_enumeration"} + for _, jsonOutput := range []bool{false, true} { + var stdout, stderr bytes.Buffer + if status := reportAttempts(listed, failure, jsonOutput, &stdout, &stderr); status != 1 { + t.Fatalf("partial discovery status=%d", status) + } + if jsonOutput { + var document attemptsDocument + if err := json.Unmarshal(stdout.Bytes(), &document); err != nil { + t.Fatal(err) + } + if len(document.Attempts) != 1 || document.Attempts[0].RecordPath != "good.json" || len(document.Diagnostics) != 1 || document.Diagnostics[0].RecordPath != "broken.json" || document.Diagnostic == nil || document.Diagnostic.Kind != "storage_enumeration" { + t.Fatalf("partial JSON lost data: %s", &stdout) + } + } else if !strings.Contains(stdout.String(), "good.json") || !strings.Contains(stderr.String(), "broken.json") || !strings.Contains(stderr.String(), "storage_enumeration") { + t.Fatalf("partial human output lost data: %s%s", &stdout, &stderr) + } + } +} + func TestRecoveryDisplaysRedactCredentialPathsAndMetadata(t *testing.T) { secret := "audit-secret" raw := "https://audit-user:" + secret + "@example.invalid/attempt.json" diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 64e486d..8712bd5 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,7 +1,12 @@ # Publication reconciliation remediation -Status: open findings from review of commits `4dff808` through `599ad81`, -against planning baseline `447b42a`. +Status: complete. R01–R13 are addressed and individually verified. The full +offline suite (`go test ./...`) and compiled facade/dependency checks +(`go run ./internal/apicheck`) passed with Go 1.26.8 after remediation. + +The findings describe commits `4dff808` through `599ad81`, against planning +baseline `447b42a`. Audit evidence and defect descriptions below refer to that +reviewed baseline; each completion marker records the implemented correction. The [feature roadmap](publication-reconciliation.md) defines the requested behavior; the [implementation plan](implementation.md) supplies the intended @@ -538,6 +543,12 @@ caps or public input restrictions are needed. ## R13: Retain entries returned with a directory-enumeration error — P2 +**Completed:** Discovery loads returned record paths before reporting enumeration +failure. Fault-injected entries-plus-error and zero-entry tests passed, preserving +readable records, per-record diagnostics, ordering, and the original cause. +Facade conversion and CLI human/JSON tests confirm partial results survive with +a non-success status. The full offline suite and API/dependency checks passed. + **Relevant code:** `internal/publish/attempts.go:41` through its immediate error return at lines 42–44; Stage 5's partial-result requirement in the implementation plan. diff --git a/internal/publish/attempts.go b/internal/publish/attempts.go index a7a64aa..f9732e3 100644 --- a/internal/publish/attempts.go +++ b/internal/publish/attempts.go @@ -22,6 +22,10 @@ type AttemptsResult struct { } func ListAttempts(ctx context.Context, repositoryDir, override string) (AttemptsResult, error) { + return listAttempts(ctx, repositoryDir, override, recordPaths) +} + +func listAttempts(ctx context.Context, repositoryDir, override string, enumerate func(context.Context, string) ([]string, error)) (AttemptsResult, error) { if repositoryDir == "" { repositoryDir = "." } @@ -41,10 +45,7 @@ func ListAttempts(ctx context.Context, repositoryDir, override string) (Attempts if err := ctx.Err(); err != nil { return result, &RecoveryError{Kind: "canceled", cause: err} } - paths, err := recordPaths(ctx, directory) - if err != nil { - return result, recoveryFailure(ctx, "storage_enumeration", fmt.Errorf("enumerate records in %q: %w", directory, err)) - } + paths, enumerationErr := enumerate(ctx, directory) sort.Strings(paths) for _, path := range paths { if err := ctx.Err(); err != nil { @@ -65,6 +66,9 @@ func ListAttempts(ctx context.Context, repositoryDir, override string) (Attempts } result.Attempts = append(result.Attempts, Attempt{RecordPath: absolute, Evidence: evidence}) } + if enumerationErr != nil { + return result, recoveryFailure(ctx, "storage_enumeration", fmt.Errorf("enumerate records in %q: %w", directory, enumerationErr)) + } return result, nil } @@ -75,12 +79,16 @@ func recordPaths(ctx context.Context, directory string) ([]string, error) { return nil, err } defer file.Close() + return readRecordPaths(ctx, directory, file.ReadDir) +} + +func readRecordPaths(ctx context.Context, directory string, read func(int) ([]os.DirEntry, error)) ([]string, error) { var paths []string for { if err := ctx.Err(); err != nil { return paths, err } - entries, err := file.ReadDir(128) + entries, err := read(128) for _, entry := range entries { if !entry.IsDir() && strings.HasSuffix(entry.Name(), ".json") { paths = append(paths, filepath.Join(directory, entry.Name())) diff --git a/internal/publish/record_test.go b/internal/publish/record_test.go index 0e21e23..fd38570 100644 --- a/internal/publish/record_test.go +++ b/internal/publish/record_test.go @@ -431,6 +431,47 @@ func TestListAttemptsFiltersMixedDirectoryAcrossBatches(t *testing.T) { } } +func TestListAttemptsRetainsEntriesWithEnumerationFailure(t *testing.T) { + directory := t.TempDir() + for name, contents := range map[string]string{ + "z.json": `{"format_version":1,"evidence":{"tag":"v2.0.0"}}`, + "a.json": `{"format_version":1,"evidence":{"tag":"v1.0.0"}}`, + "broken.json": "{", "unrelated.txt": "ignored", + } { + if err := os.WriteFile(filepath.Join(directory, name), []byte(contents), 0600); err != nil { + t.Fatal(err) + } + } + entries, err := os.ReadDir(directory) + if err != nil { + t.Fatal(err) + } + failure := errors.New("controlled directory read failure") + for _, empty := range []bool{false, true} { + calls := 0 + listed, err := listAttempts(t.Context(), "", directory, func(ctx context.Context, dir string) ([]string, error) { + return readRecordPaths(ctx, dir, func(int) ([]os.DirEntry, error) { + calls++ + if empty { + return nil, failure + } + return entries, failure + }) + }) + var recovery *RecoveryError + if !errors.As(err, &recovery) || recovery.Kind != "storage_enumeration" || !errors.Is(err, failure) || calls != 1 { + t.Fatalf("enumeration failure lost: %#v, %v; reads=%d", listed, err, calls) + } + if empty { + if len(listed.Attempts) != 0 || len(listed.Diagnostics) != 0 { + t.Fatalf("empty read: %#v", listed) + } + } else if len(listed.Attempts) != 2 || len(listed.Diagnostics) != 1 || filepath.Base(listed.Attempts[0].RecordPath) != "a.json" || filepath.Base(listed.Attempts[1].RecordPath) != "z.json" || listed.Diagnostics[0].Kind != "record_format" { + t.Fatalf("partial read dropped/reordered records: %#v", listed) + } + } +} + func TestListAttemptsDistinguishesEmptyMissingAndCanceledStorage(t *testing.T) { empty := t.TempDir() listed, err := ListAttempts(t.Context(), "", empty) diff --git a/recovery.go b/recovery.go index b77d55b..6e63c1b 100644 --- a/recovery.go +++ b/recovery.go @@ -111,6 +111,10 @@ type PublicationAttemptsResult struct { // remote or inferring which attempt should be reconciled. func ListPublicationAttempts(ctx context.Context, request PublicationAttemptsRequest) (PublicationAttemptsResult, error) { listed, err := publish.ListAttempts(ctx, request.RepositoryDir, request.RecordDir) + return publicAttemptsResult(listed, err) +} + +func publicAttemptsResult(listed publish.AttemptsResult, err error) (PublicationAttemptsResult, error) { result := PublicationAttemptsResult{RecordDir: listed.RecordDir, Attempts: make([]PublicationAttempt, 0, len(listed.Attempts)), Diagnostics: make([]*PublicationRecoveryError, 0, len(listed.Diagnostics))} for _, attempt := range listed.Attempts { result.Attempts = append(result.Attempts, PublicationAttempt{RecordPath: attempt.RecordPath, Evidence: publicEvidence(attempt.Evidence)}) diff --git a/recovery_conversion_test.go b/recovery_conversion_test.go new file mode 100644 index 0000000..93f9d57 --- /dev/null +++ b/recovery_conversion_test.go @@ -0,0 +1,21 @@ +package releasekit + +import ( + "errors" + "testing" + + "go.maximumdirect.net/releasekit/internal/publish" +) + +func TestPublicAttemptsRetainsPartialResultAndFailure(t *testing.T) { + failure := &publish.RecoveryError{Kind: "storage_enumeration"} + diagnostic := &publish.RecoveryError{Kind: "record_format", RecordPath: "broken.json"} + result, err := publicAttemptsResult(publish.AttemptsResult{ + RecordDir: "records", Attempts: []publish.Attempt{{RecordPath: "good.json", Evidence: publish.Evidence{Tag: "v1.2.3"}}}, + Diagnostics: []*publish.RecoveryError{diagnostic}, + }, failure) + var typed *PublicationRecoveryError + if !errors.As(err, &typed) || typed.Kind != "storage_enumeration" || !errors.Is(err, failure) || len(result.Attempts) != 1 || result.Attempts[0].RecordPath != "good.json" || result.Attempts[0].Evidence.Tag != "v1.2.3" || len(result.Diagnostics) != 1 || !errors.Is(result.Diagnostics[0], diagnostic) { + t.Fatalf("facade lost partial results or cause: %#v, %v", result, err) + } +}