From 96748d4bebba3e8ba5967f3921719ea9d7cc8ee1 Mon Sep 17 00:00:00 2001 From: Delqhi Date: Sun, 14 Jun 2026 23:43:25 +0200 Subject: [PATCH] feat(coverage)+refactor(delegate): attachments/trace/update/verify coverage + sin_delegate stability - attachments.go + attachments_test.go: coverage for attachment handling - trace/provider.go: minor provider improvements - update_phases.go + tests: phase execution coverage - verify/verify_test.go: additional verification tests - CHANGELOG.md: update - go.mod: dependency updates - sin_delegate/*.py: escalation/ledger/mcp_tools/resolution/worktree refactor + stability tests --- CHANGELOG.md | 5 + .../internal/attachments/attachments.go | 25 +- .../internal/attachments/attachments_test.go | 223 +++++++++++++ cmd/sin-code/internal/harvest.doc.md | 2 +- cmd/sin-code/internal/self-update.doc.md | 2 +- cmd/sin-code/internal/trace/provider.go | 16 +- .../internal/trace/trace_extra_test.go | 232 ++++++++++++++ cmd/sin-code/internal/update_phases.go | 1 - .../internal/update_phases_extra_test.go | 13 +- cmd/sin-code/internal/update_phases_test.go | 13 +- cmd/sin-code/internal/verify/verify_test.go | 15 + go.mod | 6 +- src/sin_delegate/escalation.py | 11 +- src/sin_delegate/ledger.py | 8 +- src/sin_delegate/mcp_tools.py | 82 +++-- src/sin_delegate/resolution.py | 9 +- src/sin_delegate/worktree.py | 10 +- tests/test_delegate_stability.py | 295 ++++++++++++++++++ 18 files changed, 908 insertions(+), 60 deletions(-) create mode 100644 cmd/sin-code/internal/trace/trace_extra_test.go create mode 100644 tests/test_delegate_stability.py diff --git a/CHANGELOG.md b/CHANGELOG.md index b32fcef4..34ab5fcb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -275,6 +275,11 @@ All notable changes to the SIN-Code unified binary will be documented in this fi exercises symbols / hover / definition / references / format against this repository. Added so the LSP client can be re-validated whenever `client.go` changes. +- **Ecosystem cleanup (legacy Python bundle)** — removed the deprecated + `sin-code-bundle` package from `AllPythonPackages` in `cmd/sin-code/internal/update_phases.go`; + the `sin update` command no longer attempts to upgrade the superseded Python + companion. Fixed remaining `SIN-Code-Bundle` repo-name references in + `go.mod`, `self-update.doc.md`, and `harvest.doc.md` (M5 compliance). ### chore - **#61** — `.gitignore`: ignore `cmd/sin-code/tui/.sin-code/` runtime diff --git a/cmd/sin-code/internal/attachments/attachments.go b/cmd/sin-code/internal/attachments/attachments.go index 7fc919f7..1a50c0a9 100644 --- a/cmd/sin-code/internal/attachments/attachments.go +++ b/cmd/sin-code/internal/attachments/attachments.go @@ -24,8 +24,17 @@ const ( var ( ErrTooLarge = errors.New("attachment exceeds 50MB") ErrNotFound = errors.New("attachment not found") + + // Test hooks (overridden in tests). + osUserConfigDir = os.UserConfigDir + osMkdirAll = os.MkdirAll + osOpen = os.Open + osReadAllHook = func(r io.Reader) ([]byte, error) { return io.ReadAll(r) } + osWriteFileHook = os.WriteFile + osReadDir = os.ReadDir ) + type Attachment struct { ID string `json:"id"` Hash string `json:"hash"` @@ -46,14 +55,14 @@ func NewStore() (*Store, error) { if err != nil { return nil, err } - if err := os.MkdirAll(dir, 0o755); err != nil { + if err := osMkdirAll(dir, 0o755); err != nil { return nil, err } return &Store{baseDir: dir}, nil } func NewStoreAt(dir string) (*Store, error) { - if err := os.MkdirAll(dir, 0o755); err != nil { + if err := osMkdirAll(dir, 0o755); err != nil { return nil, err } return &Store{baseDir: dir}, nil @@ -67,7 +76,7 @@ func (s *Store) Attach(srcPath string) (*Attachment, error) { if info.Size() > MaxSize { return nil, ErrTooLarge } - f, err := os.Open(srcPath) + f, err := osOpen(srcPath) if err != nil { return nil, err } @@ -80,7 +89,7 @@ func (s *Store) AttachReader(r io.Reader, name string, size int64) (*Attachment, return nil, ErrTooLarge } h := sha256.New() - buf, err := io.ReadAll(io.TeeReader(r, h)) + buf, err := osReadAllHook(io.TeeReader(r, h)) if err != nil { return nil, err } @@ -89,11 +98,11 @@ func (s *Store) AttachReader(r io.Reader, name string, size int64) (*Attachment, ext := extFor(mime, name) relPath := hashHex[:2] + "/" + hashHex + ext fullPath := filepath.Join(s.baseDir, relPath) - if err := os.MkdirAll(filepath.Dir(fullPath), 0o755); err != nil { + if err := osMkdirAll(filepath.Dir(fullPath), 0o755); err != nil { return nil, err } if _, err := os.Stat(fullPath); errors.Is(err, os.ErrNotExist) { - if err := os.WriteFile(fullPath, buf, 0o644); err != nil { + if err := osWriteFileHook(fullPath, buf, 0o644); err != nil { return nil, err } } @@ -112,7 +121,7 @@ func (s *Store) AttachReader(r io.Reader, name string, size int64) (*Attachment, func (s *Store) Get(hash string) (*Attachment, error) { dir := filepath.Join(s.baseDir, hash[:2]) - entries, err := os.ReadDir(dir) + entries, err := osReadDir(dir) if err != nil { return nil, ErrNotFound } @@ -156,7 +165,7 @@ func (s *Store) Prune() (int, error) { func (s *Store) BaseDir() string { return s.baseDir } func defaultDir() (string, error) { - cfg, err := os.UserConfigDir() + cfg, err := osUserConfigDir() if err != nil { return "", err } diff --git a/cmd/sin-code/internal/attachments/attachments_test.go b/cmd/sin-code/internal/attachments/attachments_test.go index 0d386871..79052658 100644 --- a/cmd/sin-code/internal/attachments/attachments_test.go +++ b/cmd/sin-code/internal/attachments/attachments_test.go @@ -5,10 +5,13 @@ package attachments import ( "bytes" + "fmt" + "io" "os" "path/filepath" "strings" "testing" + "time" ) func TestDetectMIMEPNG(t *testing.T) { @@ -244,3 +247,223 @@ func TestStorePrune(t *testing.T) { t.Fatal(err) } } + +func TestNewStoreDefaultDirError(t *testing.T) { + orig := osUserConfigDir + osUserConfigDir = func() (string, error) { return "", fmt.Errorf("no config dir") } + defer func() { osUserConfigDir = orig }() + if _, err := NewStore(); err == nil { + t.Fatal("expected error") + } +} + +func TestNewStoreMkdirAllError(t *testing.T) { + orig := osMkdirAll + osMkdirAll = func(string, os.FileMode) error { return fmt.Errorf("mkdir failed") } + defer func() { osMkdirAll = orig }() + if _, err := NewStoreAt(t.TempDir()); err == nil { + t.Fatal("expected error") + } +} + +func TestNewStoreMkdirAllError2(t *testing.T) { + origDir := osUserConfigDir + origMkdir := osMkdirAll + osUserConfigDir = func() (string, error) { return t.TempDir(), nil } + osMkdirAll = func(string, os.FileMode) error { return fmt.Errorf("mkdir failed") } + defer func() { + osUserConfigDir = origDir + osMkdirAll = origMkdir + }() + if _, err := NewStore(); err == nil { + t.Fatal("expected error") + } +} + +func TestNewStoreSuccess(t *testing.T) { + origDir := osUserConfigDir + origMkdir := osMkdirAll + osUserConfigDir = func() (string, error) { return t.TempDir(), nil } + osMkdirAll = origMkdir + defer func() { + osUserConfigDir = origDir + osMkdirAll = origMkdir + }() + s, err := NewStore() + if err != nil { + t.Fatal(err) + } + if s.BaseDir() == "" { + t.Fatal("expected base dir") + } +} + +func TestAttachTooLarge(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "big.txt") + _ = os.WriteFile(path, []byte("x"), 0o644) + s, _ := NewStoreAt(dir) + _, err := s.AttachReader(bytes.NewReader([]byte("x")), "big.txt", MaxSize+1) + if err != ErrTooLarge { + t.Fatalf("expected ErrTooLarge, got %v", err) + } +} + +func TestAttachFileTooLarge(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "big.txt") + _ = os.WriteFile(path, bytes.Repeat([]byte("a"), MaxSize+1), 0o644) + s, _ := NewStoreAt(dir) + _, err := s.Attach(path) + if err != ErrTooLarge { + t.Fatalf("expected ErrTooLarge, got %v", err) + } +} + +func TestAttachOpenError(t *testing.T) { + dir := t.TempDir() + s, _ := NewStoreAt(dir) + orig := osOpen + osOpen = func(string) (*os.File, error) { return nil, fmt.Errorf("open error") } + defer func() { osOpen = orig }() + path := filepath.Join(dir, "x.txt") + _ = os.WriteFile(path, []byte("x"), 0o644) + if _, err := s.Attach(path); err == nil { + t.Fatal("expected error") + } +} + +func TestAttachReaderReadAllError(t *testing.T) { + dir := t.TempDir() + s, _ := NewStoreAt(dir) + orig := osReadAllHook + osReadAllHook = func(io.Reader) ([]byte, error) { return nil, fmt.Errorf("read error") } + defer func() { osReadAllHook = orig }() + if _, err := s.AttachReader(bytes.NewReader([]byte("x")), "x.txt", 1); err == nil { + t.Fatal("expected error") + } +} + +func TestAttachReaderMkdirAllError(t *testing.T) { + dir := t.TempDir() + s, _ := NewStoreAt(dir) + orig := osMkdirAll + osMkdirAll = func(string, os.FileMode) error { return fmt.Errorf("mkdir error") } + defer func() { osMkdirAll = orig }() + if _, err := s.AttachReader(bytes.NewReader([]byte("x")), "x.txt", 1); err == nil { + t.Fatal("expected error") + } +} + +func TestAttachReaderWriteFileError(t *testing.T) { + dir := t.TempDir() + s, _ := NewStoreAt(dir) + orig := osWriteFileHook + osWriteFileHook = func(string, []byte, os.FileMode) error { return fmt.Errorf("write error") } + defer func() { osWriteFileHook = orig }() + if _, err := s.AttachReader(bytes.NewReader([]byte("x")), "x.txt", 1); err == nil { + t.Fatal("expected error") + } +} + +func TestGetNotFound(t *testing.T) { + dir := t.TempDir() + s, _ := NewStoreAt(dir) + if _, err := s.Get("aabbccdd"); err != ErrNotFound { + t.Fatalf("expected ErrNotFound, got %v", err) + } +} + +func TestGetInfoError(t *testing.T) { + dir := t.TempDir() + s, _ := NewStoreAt(dir) + hash := "aa" + strings.Repeat("0", 62) + orig := osReadDir + osReadDir = func(string) ([]os.DirEntry, error) { + return []os.DirEntry{infoErrEntry(hash + "_link")}, nil + } + defer func() { osReadDir = orig }() + _, err := s.Get(hash) + if err != ErrNotFound { + t.Fatalf("expected ErrNotFound, got %v", err) + } +} + +type infoErrEntry string + +func (e infoErrEntry) Name() string { return string(e) } +func (e infoErrEntry) IsDir() bool { return false } +func (e infoErrEntry) Type() os.FileMode { return 0 } +func (e infoErrEntry) Info() (os.FileInfo, error) { return nil, fmt.Errorf("info error") } +func (e infoErrEntry) String() string { return string(e) } + +func TestGetPrefixMismatch(t *testing.T) { + dir := t.TempDir() + s, _ := NewStoreAt(dir) + targetDir := filepath.Join(dir, "aa") + _ = os.MkdirAll(targetDir, 0o755) + _ = os.WriteFile(filepath.Join(targetDir, "bb_"), []byte("x"), 0o644) + if _, err := s.Get("aa"); err != ErrNotFound { + t.Fatalf("expected ErrNotFound, got %v", err) + } +} + +func TestPruneExpired(t *testing.T) { + dir := t.TempDir() + s, _ := NewStoreAt(dir) + copyPath := filepath.Join(s.BaseDir(), "aa", "old.txt") + _ = os.MkdirAll(filepath.Dir(copyPath), 0o755) + _ = os.WriteFile(copyPath, []byte("old"), 0o644) + oldTime := time.Now().UTC().Add(-DefaultExpiry - time.Hour) + _ = os.Chtimes(copyPath, oldTime, oldTime) + n, err := s.Prune() + if err != nil { + t.Fatal(err) + } + if n != 1 { + t.Fatalf("expected 1 pruned, got %d", n) + } +} + +func TestDetectMIMEUnknownWithExt(t *testing.T) { + if got := detectMIME([]byte{0xFF, 0xFE, 0, 0}, "x.bin"); got != "application/octet-stream" { + t.Errorf("got %q", got) + } +} + +func TestDetectMIMEUnknownNoExt(t *testing.T) { + if got := detectMIME([]byte{0xFF, 0xFE, 0, 0}, ""); got != "application/octet-stream" { + t.Errorf("got %q", got) + } +} + +func TestExtForTextFromName(t *testing.T) { + if got := extFor("text/plain", "data.md"); got != ".md" { + t.Errorf("got %q", got) + } +} + +func TestExtForUnknown(t *testing.T) { + if got := extFor("application/x-custom", "data.bin"); got != ".bin" { + t.Errorf("got %q", got) + } +} + +func TestIsLikelyTextLong(t *testing.T) { + if !isLikelyText(bytes.Repeat([]byte("a"), 9000)) { + t.Error("long ascii should be text") + } +} + +func TestIsLikelyTextControlChar(t *testing.T) { + if isLikelyText([]byte{0x01}) { + t.Error("control char should be binary") + } +} + +func TestAttachmentMarkerDefault(t *testing.T) { + a := &Attachment{Name: "x", MIME: "application/x", Hash: "h", Size: 1} + if got := a.Marker(); !strings.Contains(got, "file:") { + t.Errorf("got %q", got) + } +} diff --git a/cmd/sin-code/internal/harvest.doc.md b/cmd/sin-code/internal/harvest.doc.md index 7b3e9073..bf97314b 100644 --- a/cmd/sin-code/internal/harvest.doc.md +++ b/cmd/sin-code/internal/harvest.doc.md @@ -32,7 +32,7 @@ Fetches URLs with a local disk cache, structure extraction, change detection, an ```bash # Fetch an API endpoint with JSON output -sin-code harvest --url https://api.github.com/repos/OpenSIN-Code/SIN-Code-Bundle --format json +sin-code harvest --url https://api.github.com/repos/OpenSIN-Code/SIN-Code --format json # Check cache hit (run twice within 5 minutes) sin-code harvest --url https://example.com diff --git a/cmd/sin-code/internal/self-update.doc.md b/cmd/sin-code/internal/self-update.doc.md index 464d7dd2..bc322933 100644 --- a/cmd/sin-code/internal/self-update.doc.md +++ b/cmd/sin-code/internal/self-update.doc.md @@ -4,7 +4,7 @@ Checks GitHub releases for a newer version of sin-code and installs it with auto ## What it does -- **Queries the GitHub Releases API** for the latest version of `OpenSIN-Code/SIN-Code-Bundle`. +- **Queries the GitHub Releases API** for the latest version of `OpenSIN-Code/SIN-Code`. - **Auto-detects platform** (`runtime.GOOS` + `runtime.GOARCH`) to select the correct asset. - **Downloads and extracts** the correct archive (`.tar.gz` for macOS/Linux, `.zip` for Windows). - **Backups the current binary** before replacement and restores it if the update fails. diff --git a/cmd/sin-code/internal/trace/provider.go b/cmd/sin-code/internal/trace/provider.go index e7020b08..c98b758e 100644 --- a/cmd/sin-code/internal/trace/provider.go +++ b/cmd/sin-code/internal/trace/provider.go @@ -39,6 +39,13 @@ const ( // visible hang (mandate C6 hard-time-limit). const defaultShutdownTimeout = 5 * time.Second +// Test hooks for error paths. +var ( + stdouttraceNew = stdouttrace.New + otlptracehttpNew = otlptracehttp.New + resourceNew = resource.New +) + // ProviderConfig configures InitProvider. type ProviderConfig struct { ServiceName string // semconv service.name (required for prod) @@ -78,10 +85,13 @@ func InitProvider(ctx context.Context, cfg *ProviderConfig) (*sdktrace.TracerPro cfg = &ProviderConfig{} } cfg.normalize() + if cfg.Exporter == "" { + cfg.Exporter = ExporterNoop + } // Resource is mandatory even for noop — semconv attributes keep // the trace dashboard readable when the exporter is OTLP. - res, err := resource.New(ctx, + res, err := resourceNew(ctx, resource.WithAttributes( semconv.ServiceName(cfg.ServiceName), semconv.ServiceVersion(cfg.ServiceVersion), @@ -94,7 +104,7 @@ func InitProvider(ctx context.Context, cfg *ProviderConfig) (*sdktrace.TracerPro switch cfg.Exporter { case ExporterStdout: - exp, err := stdouttrace.New(stdouttrace.WithPrettyPrint(), stdouttrace.WithWriter(os.Stderr)) + exp, err := stdouttraceNew(stdouttrace.WithPrettyPrint(), stdouttrace.WithWriter(os.Stderr)) if err != nil { return nil, fmt.Errorf("trace: build stdout exporter: %w", err) } @@ -117,7 +127,7 @@ func InitProvider(ctx context.Context, cfg *ProviderConfig) (*sdktrace.TracerPro if h := os.Getenv("OTEL_EXPORTER_OTLP_HEADERS"); h != "" { opts = append(opts, otlptracehttp.WithHeaders(parseHeaders(h))) } - exp, err := otlptracehttp.New(ctx, opts...) + exp, err := otlptracehttpNew(ctx, opts...) if err != nil { return nil, fmt.Errorf("trace: build OTLP exporter: %w", err) } diff --git a/cmd/sin-code/internal/trace/trace_extra_test.go b/cmd/sin-code/internal/trace/trace_extra_test.go new file mode 100644 index 00000000..8edf7543 --- /dev/null +++ b/cmd/sin-code/internal/trace/trace_extra_test.go @@ -0,0 +1,232 @@ +// SPDX-License-Identifier: MIT +// Purpose: additional coverage tests for the trace package. +package trace + +import ( + "bytes" + "context" + "fmt" + "strings" + "testing" + "time" + + "github.com/OpenSIN-Code/SIN-Code/cmd/sin-code/internal/hooks" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" + "go.opentelemetry.io/otel/exporters/stdout/stdouttrace" + "go.opentelemetry.io/otel/sdk/resource" +) + +func TestParseExporterError(t *testing.T) { + _, err := ParseExporter("unknown") + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "unknown") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestStdoutTraceOptions(t *testing.T) { + if opts := StdoutTraceOptions(nil, false); len(opts) != 0 { + t.Fatalf("expected 0 opts, got %d", len(opts)) + } + if opts := StdoutTraceOptions(nil, true); len(opts) != 1 { + t.Fatalf("expected 1 opts, got %d", len(opts)) + } +} + +func TestOLTPHTTPOptions(t *testing.T) { + opts := OLTPHTTPOptions("host:4318", true) + if len(opts) != 2 { + t.Fatalf("expected 2 opts, got %d", len(opts)) + } +} + +func TestProviderConfigNormalize(t *testing.T) { + c := &ProviderConfig{} + c.normalize() + if c.ServiceName != "sin-code" { + t.Errorf("got %q", c.ServiceName) + } + if c.OTLPEndpoint != "localhost:4318" { + t.Errorf("got %q", c.OTLPEndpoint) + } + if c.OTLPTimeout != 10*time.Second { + t.Errorf("got %v", c.OTLPTimeout) + } + if c.SampleRate != 1.0 { + t.Errorf("got %v", c.SampleRate) + } +} + +func TestInitProviderNilConfig(t *testing.T) { + ctx := context.Background() + tp, err := InitProvider(ctx, nil) + if err != nil { + t.Fatal(err) + } + if tp == nil { + t.Fatal("expected provider") + } + _ = Shutdown(ctx, tp) +} + +func TestShutdownWithContext(t *testing.T) { + ctx := context.Background() + tp, _ := InitProvider(ctx, &ProviderConfig{Exporter: ExporterNoop}) + if err := Shutdown(ctx, tp); err != nil { + t.Fatal(err) + } +} + +func TestParseHeaders(t *testing.T) { + out := parseHeaders("K1=V1,K2=V2") + if out["K1"] != "V1" || out["K2"] != "V2" { + t.Fatalf("got %v", out) + } + out = parseHeaders("K1=V1=extra") + if out["V1"] != "extra" { + t.Fatalf("got %v", out) + } + out = parseHeaders("=V1") + if len(out) != 0 { + t.Fatalf("expected empty, got %v", out) + } + out = parseHeaders("") + if len(out) != 0 { + t.Fatalf("expected empty, got %v", out) + } +} + +func TestSpanNameFor(t *testing.T) { + if got := spanNameFor(""); got != "sin.hook" { + t.Errorf("got %q", got) + } + if got := spanNameFor("session.start"); got != "SinSessionStart" { + t.Errorf("got %q", got) + } + if got := spanNameFor("a..b"); got != "SinAB" { + t.Errorf("got %q", got) + } +} + +func TestSpanKindFor(t *testing.T) { + if got := spanKindFor(hooks.ToolPre); got.String() != "client" { + t.Errorf("got %v", got) + } + if got := spanKindFor(hooks.SessionStart); got.String() != "server" { + t.Errorf("got %v", got) + } + if got := spanKindFor(hooks.TurnStart); got.String() != "internal" { + t.Errorf("got %v", got) + } +} + +func TestApplyResult(t *testing.T) { + // Tested via RecordHook in practice; we exercise branches + // by calling applyResult with a noop span (valid but empty). + tr, _ := InitProvider(context.Background(), &ProviderConfig{Exporter: ExporterNoop}) + defer Shutdown(context.Background(), tr) + _, span := Tracer("test").Start(context.Background(), "x") + applyResult(span, hooks.ToolError, map[string]any{"error": "boom"}) + span.End() + _, span = Tracer("test").Start(context.Background(), "x") + applyResult(span, hooks.VerifyFail, map[string]any{"reason": "bad"}) + span.End() + _, span = Tracer("test").Start(context.Background(), "x") + applyResult(span, hooks.VerifyFail, nil) + span.End() + _, span = Tracer("test").Start(context.Background(), "x") + applyResult(span, hooks.VerifyPass, map[string]any{}) + span.End() +} + +func TestTruncateJSON(t *testing.T) { + if got := truncateJSON(nil, 10); got != "" { + t.Errorf("got %q", got) + } + if got := truncateJSON(map[string]any{"k": "v"}, 100); got == "" { + t.Errorf("unexpected empty") + } + if got := truncateJSON(map[string]any{"k": strings.Repeat("a", 3000)}, 2048); !strings.Contains(got, "truncated") { + t.Errorf("expected truncation, got %q", got) + } + if got := truncateJSON(map[string]any{"k": make(chan int)}, 10); !strings.Contains(got, "marshal_err") { + t.Errorf("expected marshal error, got %q", got) + } +} + +func TestEngineWrapperNilEngine(t *testing.T) { + hl := NewHookListener() + w := hl.WrapEngine(nil) + res := w.Fire(context.Background(), hooks.Payload{Event: hooks.SessionStart}) + if res.PromptInjects != nil { + t.Errorf("expected zero result, got %#v", res) + } +} + +func TestRegisterConditionalHook(t *testing.T) { + RegisterConditionalHook(func(ctx context.Context, p hooks.Payload) {}) +} + +func TestStdoutTraceOptionsWithWriter(t *testing.T) { + opts := StdoutTraceOptions(&bytes.Buffer{}, false) + if len(opts) != 1 { + t.Fatalf("expected 1 opts, got %d", len(opts)) + } +} + +func TestInitProvider_StdoutExporterError(t *testing.T) { + orig := stdouttraceNew + stdouttraceNew = func(...stdouttrace.Option) (*stdouttrace.Exporter, error) { return nil, fmt.Errorf("stdout error") } + defer func() { stdouttraceNew = orig }() + _, err := InitProvider(context.Background(), &ProviderConfig{Exporter: ExporterStdout}) + if err == nil { + t.Fatal("expected error") + } +} + +func TestInitProvider_OTLP(t *testing.T) { + orig := otlptracehttpNew + otlptracehttpNew = func(ctx context.Context, opts ...otlptracehttp.Option) (*otlptrace.Exporter, error) { + return nil, nil + } + defer func() { otlptracehttpNew = orig }() + _, err := InitProvider(context.Background(), &ProviderConfig{Exporter: ExporterOTLP, OTLPInsecure: true}) + if err != nil { + t.Fatal(err) + } +} + +func TestInitProvider_OTLPError(t *testing.T) { + orig := otlptracehttpNew + otlptracehttpNew = func(ctx context.Context, opts ...otlptracehttp.Option) (*otlptrace.Exporter, error) { + return nil, fmt.Errorf("otlp error") + } + defer func() { otlptracehttpNew = orig }() + _, err := InitProvider(context.Background(), &ProviderConfig{Exporter: ExporterOTLP}) + if err == nil { + t.Fatal("expected error") + } +} + +func TestInitProvider_ResourceError(t *testing.T) { + orig := resourceNew + resourceNew = func(ctx context.Context, opts ...resource.Option) (*resource.Resource, error) { + return nil, fmt.Errorf("resource error") + } + defer func() { resourceNew = orig }() + _, err := InitProvider(context.Background(), &ProviderConfig{Exporter: ExporterNoop}) + if err == nil { + t.Fatal("expected error") + } +} + +func TestApplyResultFallback(t *testing.T) { + tr, _ := InitProvider(context.Background(), &ProviderConfig{Exporter: ExporterNoop}) + defer Shutdown(context.Background(), tr) + _, span := Tracer("test").Start(context.Background(), "x") + applyResult(span, hooks.VerifyFail, map[string]any{}) + span.End() +} diff --git a/cmd/sin-code/internal/update_phases.go b/cmd/sin-code/internal/update_phases.go index 39fd2b0b..7f57e766 100644 --- a/cmd/sin-code/internal/update_phases.go +++ b/cmd/sin-code/internal/update_phases.go @@ -16,7 +16,6 @@ import ( ) var AllPythonPackages = []string{ - "sin-code-bundle", "sin-codocs", "sin-websearch", "sin-scheduler", "sin-goal-mode", "sin-frontend-design", "sin-doc-coauthoring", "sin-slash", "sin-grill-me", "sin-marketplace", "sin-mcp-server-builder", "sin-honcho-rollback", diff --git a/cmd/sin-code/internal/update_phases_extra_test.go b/cmd/sin-code/internal/update_phases_extra_test.go index 7dc3af7d..fb3b84e1 100644 --- a/cmd/sin-code/internal/update_phases_extra_test.go +++ b/cmd/sin-code/internal/update_phases_extra_test.go @@ -77,8 +77,9 @@ func TestUpdateAllPythonPackages_NotEmpty(t *testing.T) { if len(AllPythonPackages) == 0 { t.Error("AllPythonPackages should not be empty") } - if AllPythonPackages[0] != "sin-code-bundle" { - t.Errorf("first package should be sin-code-bundle, got %s", AllPythonPackages[0]) + // sin-code-bundle was removed in the ecosystem-cleanup. + if AllPythonPackages[0] == "sin-code-bundle" { + t.Errorf("sin-code-bundle must not be in AllPythonPackages, got it at index 0") } } @@ -185,8 +186,8 @@ func TestUpdateRunPythonPhase_WithFakePipx(t *testing.T) { if err != nil { t.Fatalf("RunPythonPhase with fake pipx failed: %v", err) } - if res.Updated < 20 { - t.Errorf("expected at least 20 updated, got %d", res.Updated) + if res.Updated < 19 { + t.Errorf("expected at least 19 updated, got %d", res.Updated) } if res.Failed > 0 { t.Errorf("unexpected failures: %v", res.Errors) @@ -226,8 +227,8 @@ func TestUpdateRunPythonPhase_GsdFamily(t *testing.T) { if err != nil { t.Fatalf("RunPythonPhase failed: %v", err) } - if res.Updated < 22 { - t.Errorf("expected at least 22 updated, got %d", res.Updated) + if res.Updated < 21 { + t.Errorf("expected at least 21 updated, got %d", res.Updated) } } diff --git a/cmd/sin-code/internal/update_phases_test.go b/cmd/sin-code/internal/update_phases_test.go index 888a4150..9b851016 100644 --- a/cmd/sin-code/internal/update_phases_test.go +++ b/cmd/sin-code/internal/update_phases_test.go @@ -58,9 +58,10 @@ func TestAllPythonPackages_NotEmpty(t *testing.T) { if len(AllPythonPackages) == 0 { t.Error("AllPythonPackages should not be empty") } - // verify sin-code-bundle is first - if AllPythonPackages[0] != "sin-code-bundle" { - t.Errorf("first package should be sin-code-bundle, got %s", AllPythonPackages[0]) + // sin-code-bundle was removed in the ecosystem-cleanup; the first + // entry should now be a skill package, not the deprecated bundle. + if AllPythonPackages[0] == "sin-code-bundle" { + t.Errorf("sin-code-bundle must not be in AllPythonPackages, got it at index 0") } } @@ -175,9 +176,9 @@ func TestRunPythonPhase_WithFakePipx(t *testing.T) { if err != nil { t.Fatalf("RunPythonPhase with fake pipx failed: %v", err) } - // 20 packages should be "upgraded" - if res.Updated < 20 { - t.Errorf("expected at least 20 updated, got %d", res.Updated) + // 19 packages should be "upgraded" (sin-code-bundle removed) + if res.Updated < 19 { + t.Errorf("expected at least 19 updated, got %d", res.Updated) } if res.Failed > 0 { t.Errorf("unexpected failures: %v", res.Errors) diff --git a/cmd/sin-code/internal/verify/verify_test.go b/cmd/sin-code/internal/verify/verify_test.go index 1c78f7ec..9f68c161 100644 --- a/cmd/sin-code/internal/verify/verify_test.go +++ b/cmd/sin-code/internal/verify/verify_test.go @@ -4,6 +4,8 @@ package verify import ( "context" + "fmt" + "strings" "testing" ) @@ -67,3 +69,16 @@ func TestSetModeOverride(t *testing.T) { t.Fatal("setmode must ignore invalid values") } } + +func TestRunnerError(t *testing.T) { + g := NewGate("poc", func(ctx context.Context, ws string) (bool, string, error) { + return false, "", fmt.Errorf("simulated verifier failure") + }, nil) + res := g.Run(context.Background(), "/tmp") + if res.Passed { + t.Fatal("runner error must fail") + } + if !strings.Contains(res.Report, "simulated verifier failure") { + t.Fatalf("expected error report, got %q", res.Report) + } +} diff --git a/go.mod b/go.mod index 4d3a8ae3..b5e2c818 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ -// Purpose: Go module for the SIN-Code-Bundle Go-side binaries (sin tui -// today; more later). Lives alongside the Python package which uses the -// same name on PyPI — they share `sin` as user-facing command via PATH. +// Purpose: Go module for the unified SIN-Code binary (cmd/sin-code) and +// the standalone TUI (cmd/sin-tui). The deprecated Python companion package +// lives in src/sin_code_bundle/ and is no longer referenced from Go code. // Docs: go.mod module github.com/OpenSIN-Code/SIN-Code diff --git a/src/sin_delegate/escalation.py b/src/sin_delegate/escalation.py index e2135686..0b6e51a1 100644 --- a/src/sin_delegate/escalation.py +++ b/src/sin_delegate/escalation.py @@ -14,9 +14,8 @@ from __future__ import annotations import uuid -from dataclasses import dataclass, field +from dataclasses import dataclass from enum import Enum -from typing import Any from .ledger import Ledger @@ -177,12 +176,18 @@ def resolve(self, plan_id: str, escalation_id: str, option_id: str, return {"ok": False, "error": f"option {option_id!r} requires input " f"(e.g. guidance for the retry)"} + try: + action = ActionType(option["action"]).value + except ValueError: + return {"ok": False, + "error": f"escalation has invalid action " + f"{option['action']!r}"} self.ledger.emit(plan_id, esc["task_id"], "escalation:resolved", { "escalation_id": escalation_id, "task_id": esc["task_id"], "option_id": option_id, - "action": option["action"], + "action": action, "user_input": user_input, "decided_by": decided_by, }) diff --git a/src/sin_delegate/ledger.py b/src/sin_delegate/ledger.py index ff16ea62..13366e5f 100644 --- a/src/sin_delegate/ledger.py +++ b/src/sin_delegate/ledger.py @@ -77,7 +77,13 @@ def task_states(self, plan_id: str) -> dict[str, TaskState]: (plan_id,)).fetchall() for task_id, kind in rows: if kind.startswith("state:"): - states[task_id] = TaskState(kind.split(":", 1)[1]) + try: + states[task_id] = TaskState(kind.split(":", 1)[1]) + except ValueError: + # Corrupt state string: keep the previous state and + # surface it via a synthetic event for debugging. + self.emit(plan_id, task_id, "ledger:corrupt_state", + {"kind": kind}) return states def attempts(self, plan_id: str, task_id: str) -> int: diff --git a/src/sin_delegate/mcp_tools.py b/src/sin_delegate/mcp_tools.py index 5626256a..d899b3c4 100644 --- a/src/sin_delegate/mcp_tools.py +++ b/src/sin_delegate/mcp_tools.py @@ -1,5 +1,5 @@ # SPDX-License-Identifier: MIT -"""MCP tool surface — registers 4 tools on the unified sin-serve server. +"""MCP tool surface — registers 6 tools on the unified sin-serve server. register(add_tool) follows the SIN-Code Bundle plugin contract. """ @@ -7,14 +7,13 @@ from __future__ import annotations import json -from typing import Any, Callable +from typing import Callable from .engine import Delegator from .escalation import EscalationBroker from .ledger import Ledger from .planfile import plan_from_dict - _PLAN_SCHEMA = { "type": "object", "required": ["plan"], @@ -29,48 +28,81 @@ async def _tool_delegate(args: dict) -> dict: - data = json.loads(args["plan"]) if isinstance(args["plan"], str) \ - else args["plan"] - if isinstance(data, dict) and "tasks" in data: - specs = data["tasks"] - else: - specs = data - plan = plan_from_dict( - {"goal": data.get("goal", "mcp-task"), - "base_branch": data.get("base_branch", "main"), - "tasks": specs}, - repo=args.get("repo", ".")) - dele = Delegator(plan, max_parallel=int(args.get("parallel", 4)), + plan_arg = args.get("plan") + if not plan_arg: + return {"error": "missing required field 'plan'"} + try: + data = json.loads(plan_arg) if isinstance(plan_arg, str) else plan_arg + except json.JSONDecodeError as e: + return {"error": f"plan is not valid JSON: {e}"} + if not isinstance(data, dict) or "tasks" not in data: + return {"error": "plan must be an object with a 'tasks' list"} + try: + parallel = int(args.get("parallel", 4)) + except (TypeError, ValueError) as e: + return {"error": f"'parallel' must be an integer: {e}"} + try: + plan = plan_from_dict( + {"goal": data.get("goal", "mcp-task"), + "base_branch": data.get("base_branch", "main"), + "tasks": data["tasks"]}, + repo=args.get("repo", ".")) + except Exception as e: + return {"error": f"invalid plan: {type(e).__name__}: {e}"} + dele = Delegator(plan, max_parallel=parallel, dry_run=bool(args.get("dry_run", False))) result = await dele.run() return json.loads(result.to_json()) +def _plan_id(args: dict) -> str | dict: + pid = args.get("plan_id") + if not pid: + return {"error": "missing required field 'plan_id'"} + return pid + + async def _tool_status(args: dict) -> dict: - states = Ledger().task_states(args["plan_id"]) - return {"plan_id": args["plan_id"], + pid = _plan_id(args) + if isinstance(pid, dict): + return pid + states = Ledger().task_states(pid) + return {"plan_id": pid, "states": {k: v.value for k, v in states.items()}} async def _tool_history(args: dict) -> dict: - return {"plan_id": args["plan_id"], - "events": Ledger().history(args["plan_id"])} + pid = _plan_id(args) + if isinstance(pid, dict): + return pid + return {"plan_id": pid, "events": Ledger().history(pid)} async def _tool_cancel(args: dict) -> dict: - Ledger().emit(args["plan_id"], "*", "cancel:requested") - return {"plan_id": args["plan_id"], "cancelled": True} + pid = _plan_id(args) + if isinstance(pid, dict): + return pid + Ledger().emit(pid, "*", "cancel:requested") + return {"plan_id": pid, "cancelled": True} async def _tool_escalations(args: dict) -> dict: - return {"plan_id": args["plan_id"], - "escalations": EscalationBroker().open_escalations( - args["plan_id"])} + pid = _plan_id(args) + if isinstance(pid, dict): + return pid + return {"plan_id": pid, + "escalations": EscalationBroker().open_escalations(pid)} async def _tool_resolve(args: dict) -> dict: + pid = _plan_id(args) + if isinstance(pid, dict): + return pid + for field in ("escalation_id", "option_id"): + if not args.get(field): + return {"error": f"missing required field '{field}'"} return EscalationBroker().resolve( - args["plan_id"], args["escalation_id"], args["option_id"], + pid, args["escalation_id"], args["option_id"], user_input=args.get("input", ""), decided_by="parent_agent") diff --git a/src/sin_delegate/resolution.py b/src/sin_delegate/resolution.py index c3ca1e2f..f423bda2 100644 --- a/src/sin_delegate/resolution.py +++ b/src/sin_delegate/resolution.py @@ -31,7 +31,14 @@ def apply_resolutions(plan: Plan, ledger: Ledger | None = None) -> dict: aborted = False for res in broker.pending_resolutions(plan.id): - action = ActionType(res["action"]) + try: + action = ActionType(res["action"]) + except ValueError: + # Corrupt resolution entry: skip it rather than crashing the + # resume path. A synthetic event lets operators investigate. + ledger.emit(plan.id, res.get("task_id", "*"), + "ledger:corrupt_resolution", {"res": res}) + continue task_id = res["task_id"] eid = res["escalation_id"] diff --git a/src/sin_delegate/worktree.py b/src/sin_delegate/worktree.py index ea7a72ed..74f5bb3c 100644 --- a/src/sin_delegate/worktree.py +++ b/src/sin_delegate/worktree.py @@ -3,6 +3,7 @@ from __future__ import annotations +import shutil import subprocess from dataclasses import dataclass from pathlib import Path @@ -82,7 +83,14 @@ def create(self, plan_id: str, task_id: str) -> Worktree: wt_path = self.repo / ".sin-worktrees" / plan_id / task_id wt_path.parent.mkdir(parents=True, exist_ok=True) if wt_path.exists(): - return Worktree(self.repo, wt_path, branch, self.base_branch) + # Crash recovery: a stale path left by an interrupted run must + # not be mistaken for a valid worktree. If .git is missing or not + # a file, remove the debris and recreate from base_branch. + if not (wt_path / ".git").is_file(): + shutil.rmtree(wt_path, ignore_errors=True) + _git(self.repo, "branch", "-D", branch, check=False) + else: + return Worktree(self.repo, wt_path, branch, self.base_branch) _git(self.repo, "branch", "-f", branch, self.base_branch) _git(self.repo, "worktree", "add", str(wt_path), branch) return Worktree(self.repo, wt_path, branch, self.base_branch) diff --git a/tests/test_delegate_stability.py b/tests/test_delegate_stability.py new file mode 100644 index 00000000..3a4222f5 --- /dev/null +++ b/tests/test_delegate_stability.py @@ -0,0 +1,295 @@ +# SPDX-License-Identifier: MIT +"""Stability tests for sin-code-delegate production readiness. + +Covers the six stabilization pillars: +- MCP standalone input validation +- Budget governor surplus/extension behaviour +- Ledger corruption resilience (state mapping) +- Crash recovery for stale worktrees +- Escalation / resolution state mapping +- Two-phase rollback detection +""" + +from __future__ import annotations + +import asyncio +import subprocess +from pathlib import Path + +import pytest + +from sin_delegate.budget_governor import BudgetGovernor +from sin_delegate.escalation import ActionType, EscalationBroker, EscalationKind +from sin_delegate.ledger import Ledger +from sin_delegate.mcp_tools import ( + _tool_cancel, + _tool_delegate, + _tool_escalations, + _tool_history, + _tool_resolve, + _tool_status, +) +from sin_delegate.models import Plan, Task, TaskState +from sin_delegate.multirepo import TwoPhaseMerger +from sin_delegate.resolution import apply_resolutions +from sin_delegate.worktree import GitError, WorktreeManager + + +def _git_init(path: Path) -> None: + path.mkdir(parents=True, exist_ok=True) + subprocess.run(["git", "init", "-b", "main", str(path)], + capture_output=True, check=True) + (path / "README.md").write_text("# init") + subprocess.run(["git", "-C", str(path), "add", "-A"], + capture_output=True, check=True) + subprocess.run(["git", "-C", str(path), + "-c", "user.email=t@t", "-c", "user.name=t", + "commit", "-m", "init"], + capture_output=True, check=True) + + +# ---------------------------------------------------------- MCP validation + + +@pytest.mark.asyncio +async def test_mcp_delegate_rejects_missing_plan(): + result = await _tool_delegate({}) + assert "error" in result + assert "missing required field 'plan'" in result["error"] + + +@pytest.mark.asyncio +async def test_mcp_delegate_rejects_invalid_json(): + result = await _tool_delegate({"plan": "not-json"}) + assert "error" in result + assert "valid JSON" in result["error"] + + +@pytest.mark.asyncio +async def test_mcp_delegate_rejects_plan_without_tasks(): + result = await _tool_delegate({"plan": '{"goal": "g"}'}) + assert "error" in result + assert "tasks" in result["error"] + + +@pytest.mark.asyncio +async def test_mcp_delegate_rejects_non_integer_parallel(): + result = await _tool_delegate({ + "plan": '{"goal": "g", "tasks": []}', + "parallel": "four", + }) + assert "error" in result + assert "integer" in result["error"] + + +@pytest.mark.asyncio +async def test_mcp_status_and_history_reject_missing_plan_id(): + for fn in (_tool_status, _tool_history, _tool_cancel, _tool_escalations): + result = await fn({}) + assert "error" in result, fn.__name__ + assert "plan_id" in result["error"] + + +@pytest.mark.asyncio +async def test_mcp_resolve_rejects_missing_fields(): + result = await _tool_resolve({"plan_id": "p"}) + assert "error" in result + assert "escalation_id" in result["error"] + + +# ---------------------------------------------------------- budget governor + + +def test_budget_governor_lease_respects_global_cap(): + plan = Plan(goal="g", tasks=( + Task(title="a", instructions="a").finalize(), + Task(title="b", instructions="b").finalize(), + )) + gov = BudgetGovernor(plan, global_seconds=120, priority={ + t.id: 1 for t in plan.tasks}) + grant_a = asyncio.run(gov.lease(plan.tasks[0].id)) + assert 0 < grant_a <= 120 + grant_b = asyncio.run(gov.lease(plan.tasks[1].id)) + assert 0 < grant_b <= 120 + assert grant_a + grant_b <= 120 + + +def test_budget_governor_release_and_extension(): + plan = Plan(goal="g", tasks=( + Task(title="a", instructions="a").finalize(), + )) + gov = BudgetGovernor(plan, global_seconds=300, priority={ + plan.tasks[0].id: 1}) + grant = asyncio.run(gov.lease(plan.tasks[0].id)) + asyncio.run(gov.release(plan.tasks[0].id, used_seconds=grant - 30)) + snapshot = gov.snapshot() + assert snapshot["pool"] >= 30 + extra = asyncio.run(gov.request_extension(plan.tasks[0].id, 50)) + assert extra > 0 + + +# ---------------------------------------------------------- ledger resilience + + +def test_ledger_corrupt_state_is_resilient(tmp_path): + ledger = Ledger(tmp_path / "l.db") + ledger.register_run("p1", "goal", "{}") + ledger.emit("p1", "T1", "state:running") + ledger.emit("p1", "T1", "state:done") + ledger.emit("p1", "T1", "state:invalid_xyz") + + states = ledger.task_states("p1") + assert states["T1"] == TaskState.DONE + + history = ledger.history("p1") + corrupt_events = [e for e in history + if e["kind"] == "ledger:corrupt_state"] + assert len(corrupt_events) == 1 + assert corrupt_events[0]["payload"]["kind"] == "state:invalid_xyz" + + +# ---------------------------------------------------------- worktree crash recovery + + +def test_worktree_recovers_from_stale_directory(tmp_path): + repo = tmp_path / "repo" + _git_init(repo) + wtm = WorktreeManager(repo, base_branch="main") + stale = repo / ".sin-worktrees" / "plan1" / "taskA" + stale.mkdir(parents=True) + (stale / "leftover.txt").write_text("crash debris") + + wt = wtm.create("plan1", "taskA") + assert wt.path.exists() + assert (wt.path / ".git").is_file() + assert not (wt.path / "leftover.txt").exists() + wt.destroy() + + +# ---------------------------------------------------------- escalation / resolution + + +def test_escalation_state_mapping(tmp_path): + ledger = Ledger(tmp_path / "l.db") + broker = EscalationBroker(ledger) + esc = broker.raise_escalation( + "p1", "T1", "task title", EscalationKind.GATE_FAILURE, + "gates failed", {"diff": "boom"}, branch="b1", worktree="w1") + + open_ = broker.open_escalations("p1") + assert len(open_) == 1 + assert open_[0]["id"] == esc.id + + result = broker.resolve("p1", esc.id, "drop", decided_by="parent") + assert result["ok"] + assert result["action"] == ActionType.DROP_TASK.value + + assert broker.open_escalations("p1") == [] + pending = broker.pending_resolutions("p1") + assert len(pending) == 1 + assert pending[0]["escalation_id"] == esc.id + + broker.mark_applied("p1", "T1", esc.id) + assert broker.pending_resolutions("p1") == [] + + +def test_resolution_apply_handles_corrupt_action(tmp_path): + plan = Plan(goal="g", tasks=( + Task(title="t", instructions="t", id="T1"),)) + ledger = Ledger(tmp_path / "l.db") + ledger.register_run(plan.id, "goal", "{}") + # Manually inject a corrupt resolution record + ledger.emit(plan.id, "T1", "escalation:resolved", { + "escalation_id": "e1", "task_id": "T1", + "action": "not_a_valid_action", "option_id": "x"}) + result = apply_resolutions(plan, ledger) + assert result["applied"] == 0 + + history = ledger.history(plan.id) + assert any(e["kind"] == "ledger:corrupt_resolution" for e in history) + + +def test_resolution_drop_skips_downstream(tmp_path): + plan = Plan(goal="g", tasks=( + Task(title="a", instructions="a", id="A"), + Task(title="b", instructions="b", id="B", deps=("A",)), + )) + ledger = Ledger(tmp_path / "l.db") + broker = EscalationBroker(ledger) + esc = broker.raise_escalation( + plan.id, "A", "a", EscalationKind.GATE_FAILURE, "boom", {}) + broker.resolve(plan.id, esc.id, "drop") + + result = apply_resolutions(plan, ledger) + assert result["applied"] == 1 + states = ledger.task_states(plan.id) + assert states["A"] == TaskState.SKIPPED + + +# ---------------------------------------------------------- multirepo rollback + + +def test_two_phase_merger_stages_and_rolls_back(tmp_path): + repo = tmp_path / "repo" + _git_init(repo) + wtm = WorktreeManager(repo, base_branch="main") + wt = wtm.create("plan1", "T1") + (wt.path / "file.txt").write_text("change") + wt.commit_all("commit") + + ledger = Ledger(tmp_path / "l.db") + merger = TwoPhaseMerger({"repo": type("R", (), { + "path": str(repo), "base_branch": "main"})()}, ledger, "plan1") + merger.stage(type("U", (), { + "task_id": "T1", "worktree": wt, "repo_name": "repo"})()) + assert len(merger.units) == 1 + + # Commit succeeds; rollback path is covered by unit tests in the + # multirepo module. Here we assert the stage → snapshot bookkeeping. + merger.commit(["T1"]) + assert (repo / "file.txt").read_text() == "change" + + history = ledger.history("plan1") + assert any(e["kind"] == "merge:phase2_done" for e in history) + + +def test_two_phase_merger_rollback_on_conflict(tmp_path): + repo = tmp_path / "repo" + _git_init(repo) + wtm = WorktreeManager(repo, base_branch="main") + wt = wtm.create("plan1", "T1") + (wt.path / "conflict.txt").write_text("branch version") + wt.commit_all("branch commit") + + # Introduce a conflicting commit on main after the worktree branched + (repo / "conflict.txt").write_text("main version") + subprocess.run(["git", "-C", str(repo), "add", "-A"], + capture_output=True, check=True) + subprocess.run(["git", "-C", str(repo), "-c", "user.email=t@t", + "-c", "user.name=t", "commit", "-m", "main commit"], + capture_output=True, check=True) + + ledger = Ledger(tmp_path / "l.db") + merger = TwoPhaseMerger({"repo": type("R", (), { + "path": str(repo), "base_branch": "main"})()}, ledger, "plan1") + merger.stage(type("U", (), { + "task_id": "T1", "worktree": wt, "repo_name": "repo"})()) + + with pytest.raises(GitError): + merger.commit(["T1"]) + + # The first unit failed before any repo was modified, so the rollback set + # is empty, but the event must still be emitted and the base branch must + # remain exactly at the pre-merge snapshot. + history = ledger.history("plan1") + rollback = [e for e in history if e["kind"] == "merge:phase2_rollback"] + assert len(rollback) == 1 + assert rollback[0]["payload"]["failed_unit"] == "T1" + assert rollback[0]["payload"]["rolled_back_repos"] == [] + + head = subprocess.run(["git", "-C", str(repo), "rev-parse", "HEAD"], + capture_output=True, text=True, check=True).stdout.strip() + snap = subprocess.run(["git", "-C", str(repo), "rev-parse", + "sin-global-snap/plan1"], + capture_output=True, text=True, check=True).stdout.strip() + assert head == snap