mirror of
https://github.com/lukaszraczylo/harness-sync.git
synced 2026-10-12 00:45:22 +00:00
lukaszraczyloandGitHub
e01f3419ea
Three robustness fixes for the apply pipeline:
1. Silent read errors on state/target files treated 'file absent' as
'file empty' and fast-forward-overwrote user edits. Add
common.ReadExistingFile that returns (nil,nil) only on
os.IsNotExist; other I/O errors propagate. Use it from apply.Run
and from every adapter's Render (claudecode, crush, kilo,
opencode, goose, zed).
2. apply.Run required callers to pre-construct *gitx.Repo; passing
nil silently skipped the commit. Run now auto-opens a repo from
Bundle.Root, gracefully no-ops when the root is not a git repo,
and uses a new gitx.HasStagedChanges() check to skip the no-op
'nothing to commit' that triggered the original silent-skip.
3. Each of 7 adapter packages re-implemented the same 25-line
Option/WithHome/New constructor. Hoist into adapter.Base +
adapter.BaseOption + NewBase. Each adapter embeds *Base and
reduces to a 9-line constructor. Public New(WithHome(h)) API
preserved; a.home becomes a.Home.
Tests:
- apply: 5 new (propagates-read-error, auto-opens-repo,
no-repo-skips-commit, skips-no-op-commit, commit-message-counts)
- gitx: 2 new (HasStagedChanges empty/after-add)
- adapter/common: 3 new (ReadExistingFile missing/content/perm)
- adapter: 4 new (NewBase default/WithHome/order/typed)
- existing adapter tests unchanged — verify a.Home still honoured.
65 lines
1.9 KiB
Go
65 lines
1.9 KiB
Go
package common_test
|
|
|
|
import (
|
|
"errors"
|
|
"os"
|
|
"path/filepath"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
|
|
"github.com/lukaszraczylo/harness-sync/internal/adapter/common"
|
|
)
|
|
|
|
// ReadExistingFile: missing file returns (nil, nil). Permission errors and
|
|
// other I/O failures must NOT be silently swallowed — those would let
|
|
// read-modify-write code clobber the user's on-disk state.
|
|
|
|
func TestReadExistingFileMissing(t *testing.T) {
|
|
got, err := common.ReadExistingFile(filepath.Join(t.TempDir(), "absent"))
|
|
require.NoError(t, err)
|
|
assert.Nil(t, got)
|
|
}
|
|
|
|
func TestReadExistingFileContent(t *testing.T) {
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "ok.txt")
|
|
require.NoError(t, os.WriteFile(path, []byte("body\n"), 0o600))
|
|
|
|
got, err := common.ReadExistingFile(path)
|
|
require.NoError(t, err)
|
|
assert.Equal(t, []byte("body\n"), got)
|
|
}
|
|
|
|
func TestReadExistingFilePropagatesError(t *testing.T) {
|
|
// Create a directory the OS will refuse us read access to.
|
|
if os.Geteuid() == 0 {
|
|
t.Skip("running as root; cannot test permission denial")
|
|
}
|
|
dir := t.TempDir()
|
|
require.NoError(t, os.Mkdir(filepath.Join(dir, "locked"), 0o000))
|
|
t.Cleanup(func() { _ = os.Chmod(filepath.Join(dir, "locked"), 0o755) })
|
|
|
|
_, err := common.ReadExistingFile(filepath.Join(dir, "locked", "file"))
|
|
require.Error(t, err)
|
|
assert.False(t, os.IsNotExist(err), "permission denied must not be reported as not-exist")
|
|
assert.True(t, errors.Is(err, os.ErrPermission) || isPermission(err),
|
|
"expected permission error, got %v", err)
|
|
}
|
|
|
|
func isPermission(err error) bool {
|
|
// Some platforms wrap the error in *os.PathError. Check the string too
|
|
// for portability — it's a coarse signal but adequate here.
|
|
return err != nil && (contains(err.Error(), "permission") || contains(err.Error(), "access denied"))
|
|
}
|
|
|
|
func contains(s, sub string) bool {
|
|
for i := 0; i+len(sub) <= len(s); i++ {
|
|
if s[i:i+len(sub)] == sub {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|