Skip to content

Commit 187bb98

Browse files
committed
fix(artifact): recover interrupted folder markers
A crash before marker installation can leave an otherwise empty target permanently unopenable, while fallback write failures can strand a partial final marker. Recover only bounded, exact marker temporaries from an unmarked target and remove fallback markers that never became durable, preserving fail-closed rejection for every other nonempty directory.
1 parent e077e52 commit 187bb98

2 files changed

Lines changed: 171 additions & 7 deletions

File tree

internal/artifact/transport_folder.go

Lines changed: 112 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const (
2121
folderFormatName = "agentsview-normalized-artifacts"
2222
folderFormatVersion = 3
2323
folderMarkerTempPrefix = ".agentsview-artifacts.tmp-"
24+
folderMarkerMaxTemps = 128
2425
folderExchangeLockName = ".agentsview-artifacts.lock"
2526
folderExchangeMaxObjects = 128
2627
folderExchangeMaxBytes = int64(64 << 20)
@@ -225,10 +226,16 @@ func (t *folderTransport) prepareMarker() error {
225226
return err
226227
}
227228
if !empty {
228-
return fmt.Errorf(
229-
"%w: target is not an agentsview artifact target",
230-
ErrArtifactInvalid,
231-
)
229+
recovered, err := recoverFolderMarkerTemporaries(t.root)
230+
if err != nil {
231+
return err
232+
}
233+
if !recovered {
234+
return fmt.Errorf(
235+
"%w: target is not an agentsview artifact target",
236+
ErrArtifactInvalid,
237+
)
238+
}
232239
}
233240
if err := createFolderMarker(t.root); err != nil {
234241
return fmt.Errorf("initializing agentsview artifact target: %w", err)
@@ -406,6 +413,64 @@ func folderRootEmpty(root *os.Root) (bool, error) {
406413
return len(entries) == 0, nil
407414
}
408415

416+
func recoverFolderMarkerTemporaries(root *os.Root) (_ bool, retErr error) {
417+
directory, err := root.Open(".")
418+
if err != nil {
419+
return false, fmt.Errorf("opening artifact target directory: %w", err)
420+
}
421+
defer func() { retErr = errors.Join(retErr, directory.Close()) }()
422+
423+
names := make([]string, 0, 1)
424+
for {
425+
entries, err := directory.ReadDir(1)
426+
if errors.Is(err, io.EOF) {
427+
break
428+
}
429+
if err != nil {
430+
return false, fmt.Errorf("reading artifact target directory: %w", err)
431+
}
432+
name := entries[0].Name()
433+
if !isFolderMarkerTemporaryName(name) ||
434+
len(names) >= folderMarkerMaxTemps {
435+
return false, nil
436+
}
437+
info, err := root.Lstat(name)
438+
if err != nil {
439+
return false, fmt.Errorf("stating marker temporary: %w", err)
440+
}
441+
if !info.Mode().IsRegular() {
442+
return false, nil
443+
}
444+
names = append(names, name)
445+
}
446+
if len(names) == 0 {
447+
return true, nil
448+
}
449+
for _, name := range names {
450+
if err := removeFolderFile(root, name); err != nil {
451+
return false, fmt.Errorf("removing marker temporary: %w", err)
452+
}
453+
}
454+
if err := syncFolderDirectory(root); err != nil {
455+
return false, fmt.Errorf("syncing recovered artifact target: %w", err)
456+
}
457+
return true, nil
458+
}
459+
460+
func isFolderMarkerTemporaryName(name string) bool {
461+
suffix, found := strings.CutPrefix(name, folderMarkerTempPrefix)
462+
if !found || len(suffix) != 16 {
463+
return false
464+
}
465+
for _, character := range suffix {
466+
if (character < '0' || character > '9') &&
467+
(character < 'a' || character > 'f') {
468+
return false
469+
}
470+
}
471+
return true
472+
}
473+
409474
func validateFolderMarker(root *os.Root) error {
410475
_, err := readFolderMarker(root)
411476
return err
@@ -494,6 +559,20 @@ func createFolderMarker(root *os.Root) (retErr error) {
494559
}
495560

496561
func createFolderMarkerExclusive(root *os.Root, body []byte) error {
562+
return createFolderMarkerExclusiveWithWriter(
563+
root,
564+
body,
565+
func(file *os.File, body []byte) (int, error) {
566+
return file.Write(body)
567+
},
568+
)
569+
}
570+
571+
func createFolderMarkerExclusiveWithWriter(
572+
root *os.Root,
573+
body []byte,
574+
write func(*os.File, []byte) (int, error),
575+
) (retErr error) {
497576
file, err := root.OpenFile(
498577
folderMarkerName,
499578
os.O_WRONLY|os.O_CREATE|os.O_EXCL,
@@ -505,10 +584,36 @@ func createFolderMarkerExclusive(root *os.Root, body []byte) error {
505584
}
506585
return err
507586
}
508-
if _, err := file.Write(body); err != nil {
509-
return errors.Join(err, file.Close())
587+
closed := false
588+
keep := false
589+
defer func() {
590+
if !closed {
591+
retErr = errors.Join(retErr, file.Close())
592+
}
593+
if !keep {
594+
retErr = errors.Join(
595+
retErr,
596+
removeFolderFile(root, folderMarkerName),
597+
)
598+
}
599+
}()
600+
written, err := write(file, body)
601+
if err != nil {
602+
return err
603+
}
604+
if written != len(body) {
605+
return io.ErrShortWrite
510606
}
511-
return errors.Join(file.Sync(), file.Close())
607+
if err := file.Sync(); err != nil {
608+
return err
609+
}
610+
closeErr := file.Close()
611+
closed = true
612+
if closeErr != nil {
613+
return closeErr
614+
}
615+
keep = true
616+
return nil
512617
}
513618

514619
type folderTransportPersistedState struct {

internal/artifact/transport_folder_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,65 @@ func TestOpenFolderTransportReopensMarkedTarget(t *testing.T) {
7272
require.NoError(t, transport.Prepare(t.Context(), nil))
7373
}
7474

75+
func TestOpenFolderTransportRecoversInterruptedMarkerTemporary(t *testing.T) {
76+
t.Parallel()
77+
78+
target := t.TempDir()
79+
temporary := filepath.Join(
80+
target,
81+
folderMarkerTempPrefix+"0123456789abcdef",
82+
)
83+
require.NoError(t, os.WriteFile(temporary, []byte(`{"format":`), 0o600))
84+
85+
transport, err := OpenFolderTransport(target, FolderTransportOptions{})
86+
require.NoError(t, err)
87+
t.Cleanup(func() { require.NoError(t, transport.Close()) })
88+
89+
assert.NoFileExists(t, temporary)
90+
marker, err := os.ReadFile(filepath.Join(target, folderMarkerName))
91+
require.NoError(t, err)
92+
assert.Contains(t, string(marker), folderFormatName)
93+
}
94+
95+
func TestOpenFolderTransportRejectsMarkerTempLookalike(t *testing.T) {
96+
t.Parallel()
97+
98+
target := t.TempDir()
99+
lookalike := filepath.Join(target, folderMarkerTempPrefix+"not-generated")
100+
require.NoError(t, os.WriteFile(lookalike, []byte("keep"), 0o600))
101+
102+
transport, err := OpenFolderTransport(target, FolderTransportOptions{})
103+
require.Error(t, err)
104+
assert.Nil(t, transport)
105+
assert.ErrorContains(t, err, "not an agentsview artifact target")
106+
assert.FileExists(t, lookalike)
107+
assert.NoFileExists(t, filepath.Join(target, folderMarkerName))
108+
}
109+
110+
func TestCreateFolderMarkerExclusiveRemovesPartialFinal(t *testing.T) {
111+
t.Parallel()
112+
113+
target := t.TempDir()
114+
root, err := os.OpenRoot(target)
115+
require.NoError(t, err)
116+
t.Cleanup(func() { require.NoError(t, root.Close()) })
117+
body := []byte(`{"format":"agentsview-normalized-artifacts"}`)
118+
partialWrite := errors.New("partial marker write")
119+
120+
err = createFolderMarkerExclusiveWithWriter(
121+
root,
122+
body,
123+
func(file *os.File, body []byte) (int, error) {
124+
written, writeErr := file.Write(body[:8])
125+
require.NoError(t, writeErr)
126+
return written, partialWrite
127+
},
128+
)
129+
130+
require.ErrorIs(t, err, partialWrite)
131+
assert.NoFileExists(t, filepath.Join(target, folderMarkerName))
132+
}
133+
75134
func TestOpenFolderTransportRefusesUnmarkedNonemptyTarget(t *testing.T) {
76135
t.Parallel()
77136

0 commit comments

Comments
 (0)