fix(isolation): retain published results on recovery inspection errors

Recovery conflated a failed Git inspection with an empty result and erased
the durable marker before attempting worktree removal. Return inspection
errors without cleanup and report the existing unavailable refusal instead
of forgetting a valid result ID.

Exercise apply path discovery, Apply and Discard against real retained
worktrees after restarting the store and temporarily hiding Git. Preserve
marker bytes and work, then retry the same ID after Git returns. Keep age
expiration and pre-publication Finish recovery as separate lifecycle units.
This commit is contained in:
SivanCola committed 2026-10-25 01:40:02 +08:00
1 parent 0dd1a41153
commit be47298166
5 files changed
+170 -8

No files matched your search

+63
View File
@@ -0,0 +1,63 @@
---
owner: @SivanCola
backup: @esengine
status: active
reviewed: 2026-10-25
---
# Isolated result inspection / 隔离结果检查
## Cause and repair / 根因与修复
A restarted store reconstructs a pending result by asking Git for its candidate
tree and diff statistics. Recovery previously treated an inspection error and an
empty result as the same cleanup condition. Cleanup forgot the durable marker
before attempting worktree removal, losing the result's discoverable identity.
重启后的存储通过 Git 重建候选树和变更统计。此前检查失败与无修改共用清理分支,
先删除恢复标记再尝试移除工作树。即使 Git 只是暂时不可用,原结果 ID 也会丢失。
Recovery now returns inspection errors before any cleanup. The lookup owner
reports the existing `isolation.unavailable` refusal with the result ID and
retained-files guidance. A caller can retry the same ID when Git is available.
Genuinely unknown IDs retain their existing refusal.
修复将检查错误直接返回,禁止错误触发清理。共用查询入口使用已有的不可用错误码,
说明结果 ID 和文件保留情况。Git 恢复后可用同一 ID 重试;不存在的 ID 仍按原规则拒绝。
## Inputs, authority and effects / 输入、权限与执行
- Result IDs and durable markers retain their existing workspace scope. This
change adds no path grant, root, Git command or model-controlled execution.
- All three callers share the fix: apply path discovery, apply execution and
explicit discard. An inspection error mutates neither the marker nor the
store's recovered-entry map and does not start apply/discard cleanup.
- Inspection can still invoke the existing Git snapshot machinery. This is not
a promise that arbitrary Git internals, external writers or commands are
confined, atomic or side-effect-free.
- Error details come from the existing local Git operations. The durable JSON
shape, tool schema, refusal vocabulary and compatibility format are unchanged.
本批不增加路径授权、Git 命令或模型可控执行点。三个调用入口均复用同一修复;
失败时不删除标记、不登记错误重建结果,也不开始应用或丢弃。检查仍使用原 Git
快照流程,不宣称其具备系统沙箱、全局原子性或消除外部写入竞争的能力。
## Evidence and remaining bounds / 证据与未完成边界
Regressions create real disposable Git worktrees, restart the store and remove
Git from the test process's PATH. Every caller must preserve marker bytes,
candidate content, workspace content and pending identity. Restoring PATH must
allow the same operation to complete using the original ID.
回归使用真实临时 Git 工作树,在存储重建后暂时移除测试进程的 Git 搜索路径。
测试要求恢复标记逐字节保留,候选内容和主工作区不变,原 ID 仍可发现;恢复路径
后同一操作必须成功。没有修改用户 Git 配置或运行生产资源。
Age-based expiration, successful-empty inspection policy, interrupted Finish,
versioned lifecycle records, recovery cancellation, journaled application and
cross-process settlement remain separate review units. This repair does not
claim to preserve results from an older process still using the old policy.
按年龄清理、成功但为空的判定、Finish 中断、版本化生命周期、恢复取消、整合日志
与跨进程整合互斥仍须分批处理。本批不能约束仍使用旧清理策略的旧程序,也不代表
完整恢复层已经验收。作者说明不能替代独立安全审查。
+11 -7
View File
@@ -3,6 +3,7 @@ package isolation
import (
"context"
"encoding/json"
"fmt"
"os"
"path/filepath"
"time"
@@ -70,9 +71,9 @@ func (s *Store) durableIDs() []string {
return ids
}
// recover rebuilds a result from its worktree. One whose worktree no longer
// holds a change is removed rather than offered.
func (s *Store) recover(id string) (*Entry, bool) {
// recover rebuilds a result from its worktree. Inspection errors retain the
// result for retry; a successfully inspected empty result is removed.
func (s *Store) recover(id string) (*Entry, error) {
for _, d := range s.durable() {
if d.ID != id {
continue
@@ -83,17 +84,20 @@ func (s *Store) recover(id string) (*Entry, bool) {
if err == nil && len(changes) > 0 {
e.Stats, err = worktree.DiffStat(ctx, e.Snap, tree, changes)
}
if err != nil || len(changes) == 0 {
if err != nil {
return nil, fmt.Errorf("inspect isolated result %s; files retained: %w", id, err)
}
if len(changes) == 0 {
s.remove(ctx, e)
return nil, false
return nil, nil
}
e.Tree, e.Changes = tree, changes
s.mu.Lock()
s.entries[e.ID] = e
s.mu.Unlock()
return e, true
return e, nil
}
return nil, false
return nil, nil
}
// SweepExpired removes this workspace's results nobody settled within maxAge.
@@ -0,0 +1,89 @@
package isolation
import (
"bytes"
"errors"
"os"
"path/filepath"
"slices"
"strings"
"testing"
"reasonix/internal/base/testenv"
"reasonix/internal/contract/tool"
)
func TestRecoveryInspectionFailureRetainsResultAndCanRetry(t *testing.T) {
for _, operation := range []string{"paths", "apply", "discard"} {
t.Run(operation, func(t *testing.T) {
repo := gitRepo(t)
managed := testenv.TempDir(t)
store := NewStore(managed, repo, opened(t, repo))
report, err := store.Execute(t.Context(), writes(map[string]string{"a.txt": "isolated work\n"}), repo, "change a.txt", "")
if err != nil {
t.Fatal(err)
}
id := idOf(t, report)
e, err := store.lookup(id)
if err != nil {
t.Fatal(err)
}
marker := entryPath(e.Cand)
before, err := os.ReadFile(marker)
if err != nil {
t.Fatal(err)
}
restarted := NewStore(managed, repo, opened(t, repo))
invoke := func() error {
switch operation {
case "paths":
_, err := NewApplyTool(restarted).DeclaredWritePaths(t.Context(), args(id))
return err
case "apply":
_, err := NewApplyTool(restarted).Execute(t.Context(), args(id))
return err
default:
_, err := NewDiscardTool(restarted).Execute(t.Context(), args(id))
return err
}
}
// No Git process can inspect or clean the worktree while PATH is empty.
// This is a temporary tool failure, not an empty or discarded result.
previousPath := os.Getenv("PATH")
t.Setenv("PATH", testenv.TempDir(t))
err = invoke()
after, readErr := os.ReadFile(marker)
if readErr != nil || !bytes.Equal(after, before) {
t.Fatalf("inspection failure lost or changed the recovery marker: %v", readErr)
}
if got := read(t, e.Cand.WorkspaceRoot, "a.txt"); got != "isolated work\n" {
t.Fatalf("candidate work was changed: %q", got)
}
if got := read(t, repo, "a.txt"); got != "one\n" {
t.Fatalf("failed recovery changed workspace: %q", got)
}
var refusal tool.Refusal
if !errors.As(err, &refusal) || refusal.Code != CodeUnavailable || !strings.Contains(refusal.Message, id) || !strings.Contains(refusal.Message, "retained") {
t.Fatalf("inspection error = %v, want unavailable result with retained-files guidance", err)
}
if !slices.Contains(restarted.Pending(), id) {
t.Fatal("pending result became undiscoverable")
}
t.Setenv("PATH", previousPath)
if err := invoke(); err != nil {
t.Fatalf("same result could not retry after Git returned: %v", err)
}
if operation == "apply" && read(t, repo, "a.txt") != "isolated work\n" {
t.Fatal("retried apply did not integrate retained work")
}
if operation == "paths" {
if _, err := NewDiscardTool(restarted).Execute(t.Context(), args(id)); err != nil {
t.Fatal(err)
}
}
if _, err := os.Stat(filepath.Join(filepath.Dir(e.Cand.WorktreeRoot), entryFile)); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("settled result marker remains: %v", err)
}
})
}
}
+5 -1
View File
@@ -170,7 +170,11 @@ func (s *Store) lookup(id string) (*Entry, error) {
e, ok := s.entries[id]
s.mu.Unlock()
if !ok {
e, ok = s.recover(id)
recovered, err := s.recover(id)
if err != nil {
return nil, tool.Refusal{Code: CodeUnavailable, Message: err.Error()}
}
e, ok = recovered, recovered != nil
}
if !ok {
pending := s.Pending()
@@ -0,0 +1,2 @@
- Fixed: temporary Git inspection failures after restart retain an isolated result's files and ID, so path discovery, apply or discard can retry when Git returns. Refs #14025, #14133.
- 修复:重启后 Git 暂时不可用时保留隔离结果的文件和 ID;Git 恢复后可重新查询、应用或丢弃。关联 #14025、#14133。