mirror of
https://github.com/esengine/DeepSeek-Reasonix.git
synced 2026-11-07 02:36:01 -10:00
fix(worktree): rebuild private index before capturing content
Problem: A candidate snapshot can contain old bytes after the working file changed while preserving its size and mtime. Root cause: Copying the source index carries Git's trusted stat cache and assume-unchanged state into StageAll, so staging can skip the actual bytes. Fix: Preserve the copied index's staged tree, rebuild the private index from that tree, then stage current content. Handle an unmerged private index first. Keep staged ignored inputs, the real conflicted index, HEAD and refs intact. Document concrete Git/path/authority boundaries and the unchanged need for source coordination. Include the inherited documentation-lint prerequisite. Verification: All three cache-policy variants fail against the original function through a compiler overlay and pass with the repair. Full worktree race passes, plus final expanded cache and conflict regressions. Check, lint and root vet pass. Full builtin/boot exits zero (22.243s / 444.076s tests), using a measured 15-minute package alarm with no product-deadline change. The exact-commit full root sweep and hosted checks remain separate evidence. Refs #14025 Refs #14133
This commit is contained in:
1 parent
ef79a2215e
commit
0f943c7f7a
6 files changed
+241
-10
No files matched your search
@@ -0,0 +1,65 @@
|
||||
---
|
||||
owner: @SivanCola
|
||||
backup: @esengine
|
||||
status: active
|
||||
reviewed: 2026-10-24
|
||||
---
|
||||
|
||||
# Worktree content snapshot review / Worktree 内容快照审查
|
||||
|
||||
## Purpose / 目的
|
||||
|
||||
This is the author's security assessment of the content-snapshot slice extracted
|
||||
from #14133 at `467a61668a0781cab171536158536283d931e4d9`. It is independent of
|
||||
the transaction-library PR #14368. It is not independent security approval.
|
||||
|
||||
本批修复已有快照入口受 Git stat 缓存影响的问题,独立于事务基础库。本文是
|
||||
作者审查,不代表独立安全审批,也不替代后续隔离、整合及恢复层的审查。
|
||||
|
||||
## Reference tables / 边界对照
|
||||
|
||||
| Value / 输入 | Owner and use / 所有者及用途 |
|
||||
| --- | --- |
|
||||
| Repository and Git index path | Existing resolved `gitcmd.Repo` and `rev-parse --git-path index`. Existing copy reads that index. No model-provided arbitrary index path is added. |
|
||||
| Private index | Existing `os.MkdirTemp` directory and fixed `index` leaf. `GIT_INDEX_FILE` is passed as environment data to every new Git command. Only this private index is removed/rebuilt. |
|
||||
| Tree identity | `write-tree` output from the private index is passed as one argument to `read-tree`. No shell string concatenation or new shell executable is introduced. |
|
||||
| Workspace bytes | Existing `Repo.StageAll` reads the authorized source through the established Git driver. Git configuration/filter policy is not broadened. |
|
||||
| Source Git state | The real index, HEAD and refs are not updated. Writing unreachable tree/commit objects remains an existing snapshot side effect governed by Git's own object publication. |
|
||||
|
||||
各路径来自现有宿主解析的仓库身份或新建的私有临时目录。新命令仍经现有 Git
|
||||
驱动执行;参数不作为 shell 脚本拼接。真实索引和 refs 不参与私有索引重建。
|
||||
|
||||
## Rules and failure / 规则与失败
|
||||
|
||||
1. The copied index MUST first preserve its staged tree, including explicitly
|
||||
staged ignored files. Rebuild from that tree before staging current bytes,
|
||||
so trusted size/mtime cache entries cannot select stale content.
|
||||
2. An unmerged copied index MUST be staged from working bytes before its tree
|
||||
can be written. This records conflict markers as bytes; it does not resolve
|
||||
or rewrite the user's actual conflicted index.
|
||||
3. Every new Git invocation MUST retain the existing driver and deadline.
|
||||
Failure returns an error. The temporary directory is removed by its existing
|
||||
owner; Git object cleanup remains ordinary Git garbage collection.
|
||||
4. Callers MUST still coordinate the source during capture. This repair does
|
||||
not make a directory scan atomic against concurrent external writers, grant
|
||||
access, isolate execution or issue a verification receipt.
|
||||
5. Existing snapshot/result formats remain unchanged. No metadata field,
|
||||
migration, settings change or automatic-isolation policy is introduced.
|
||||
|
||||
规则保证私有索引重建时保留已暂存的忽略文件,并处理真实冲突状态。它没有把
|
||||
目录扫描变成跨文件原子快照;调用者仍需协调源文件写入。失败必须返回错误,
|
||||
不能把不完整捕获当作有效验证证据。
|
||||
|
||||
## Verification / 验证
|
||||
|
||||
1. `TestSnapshotHashesBytesDespiteTrustedCachedStat` covers trusted size/mtime,
|
||||
`assume-unchanged` and `core.ignorestat`. Old code captures old bytes; fixed
|
||||
code captures new bytes, keeps staged ignored input, and preserves index/HEAD.
|
||||
2. `TestSnapshotRecordsAWorkspaceMidConflict` uses a real merge conflict and
|
||||
checks both captured conflict-marker bytes and unchanged real index bytes.
|
||||
3. The complete worktree race suite covers neighbouring creation, cancellation,
|
||||
filtering, relative roots, application and rollback behavior. Platform and
|
||||
broader assembly results are reported separately in the PR.
|
||||
|
||||
回归先证明旧实现在真实 Git 仓库中捕获旧内容,再验证修复后内容、忽略文件、
|
||||
索引及 HEAD 均符合预期。其他平台和装配层结论按各自实际测试结果记录。
|
||||
@@ -1,13 +1,18 @@
|
||||
---
|
||||
owner: @Oliver-Rui
|
||||
status: proposed
|
||||
owner: @esengine
|
||||
backup: @SivanCola
|
||||
author: @Oliver-Rui
|
||||
status: active
|
||||
reviewed: 2026-10-24
|
||||
---
|
||||
|
||||
# First run without a picked workspace
|
||||
|
||||
Status: proposed. Nothing in this document is shipped unless §8 says so. It lives under `docs/design/` so the product-doc retrieval corpus (`docs/*.md`) cannot present it to a model as a feature.
|
||||
|
||||
Reference: how MiMo Desktop (observed 2026-10-24 on a machine running it) handles the same three first-run decisions — connect a model, pick a workspace, start a session. Its behaviour below is read off installed artifacts (config schema, directory layout, state store), not source; the mapping onto this repository in §6 is grounded in source.
|
||||
Reference: how MiMo Desktop (observed 2026-10-24 on a machine running it) handles the same three first-run decisions — connect a model, pick a workspace, start a session.
|
||||
|
||||
Its behaviour below is read off installed artifacts (config schema, directory layout, state store), not source; the mapping onto this repository in §6 is grounded in source.
|
||||
|
||||
## 1. The problem
|
||||
|
||||
@@ -19,9 +24,13 @@ Studio's first launch asks three things at once, and only one of them is honestl
|
||||
| Start a session | the composer is reachable as soon as the workbench opens | yes |
|
||||
| Pick a workspace | `pickLaunchWorkspace(cwd, appExe, remembered, home)` — with nothing remembered, the first pane's root is the **user's home directory** | no |
|
||||
|
||||
The fallback chain in `cmd/reasonix-studio-host/launchroot.go` is careful about *which* cwd it trusts — a Start-menu launch inherits the install directory, and taking that as a project "puts the app's own folder in the sidebar again after every removal" — but its last resort is the home directory. That folder was never chosen as a project. It is broad, it is sensitive, and it is not represented in the sidebar as anything the user picked, so a session running there reads as a temporary session: not in the user's project, scoped to a folder they did not choose, and removing it does not stick.
|
||||
The fallback chain in `cmd/reasonix-studio-host/launchroot.go` is careful about *which* cwd it trusts — a Start-menu launch inherits the install directory, and taking that as a project "puts the app's own folder in the sidebar again after every removal" — but its last resort is the home directory.
|
||||
|
||||
Everything else is already in place for a decoupled first run. Session transcripts persist in the global store (`SessionDir()` = `<user state>/sessions`), so a session without a chosen project is not lost. Each logical session already owns a private scratch directory (`SessionTemp`, rotated per session, deleted when it ends, surfaced to the model as `$TMPDIR`). Provider setup already refuses to block. The workspace is the last coupled decision.
|
||||
That folder was never chosen as a project. It is broad, it is sensitive, and it is not represented in the sidebar as anything the user picked, so a session running there reads as a temporary session: not in the user's project, scoped to a folder they did not choose, and removing it does not stick.
|
||||
|
||||
Everything else is already in place for a decoupled first run. Session transcripts persist in the global store (`SessionDir()` = `<user state>/sessions`), so a session without a chosen project is not lost.
|
||||
|
||||
Each logical session already owns a private scratch directory (`SessionTemp`, rotated per session, deleted when it ends, surfaced to the model as `$TMPDIR`). Provider setup already refuses to block. The workspace is the last coupled decision.
|
||||
|
||||
## 2. What MiMo does instead
|
||||
|
||||
|
||||
@@ -193,8 +193,8 @@ func RemoveCandidate(ctx context.Context, snap Snapshot, c Candidate) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// workingTree writes repo's working state as a tree through a private copy of
|
||||
// its index, so only changed files are hashed and the real index is untouched.
|
||||
// workingTree hashes actual workspace bytes through a rebuilt private index.
|
||||
// Git's cached stat identity is not a content version; the real index is untouched.
|
||||
func workingTree(ctx context.Context, repo gitcmd.Repo) (string, error) {
|
||||
indexPath, _, err := runGit(ctx, repo, "rev-parse", "--git-path", "index")
|
||||
if err != nil {
|
||||
@@ -216,6 +216,29 @@ func workingTree(ctx context.Context, repo gitcmd.Repo) (string, error) {
|
||||
env := []string{"GIT_INDEX_FILE=" + private}
|
||||
stageCtx, cancel := context.WithTimeout(ctx, gitWorktreeAddTimeout)
|
||||
defer cancel()
|
||||
if _, err := os.Stat(private); err == nil {
|
||||
unmerged, stderr, err := runGitEnv(stageCtx, repo, env, "ls-files", "--unmerged", "-z")
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("read private index: %w%s", err, stderrSuffix(stderr))
|
||||
}
|
||||
if unmerged != "" {
|
||||
if err := repo.StageAll(stageCtx, env...); err != nil {
|
||||
return "", fmt.Errorf("stage conflicted workspace snapshot: %w", err)
|
||||
}
|
||||
}
|
||||
indexTree, stderr, err := runGitEnv(stageCtx, repo, env, "write-tree")
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("record private index: %w%s", err, stderrSuffix(stderr))
|
||||
}
|
||||
if err := os.Remove(private); err != nil {
|
||||
return "", err
|
||||
}
|
||||
if _, stderr, err := runGitEnv(stageCtx, repo, env, "read-tree", strings.TrimSpace(indexTree)); err != nil {
|
||||
return "", fmt.Errorf("rebuild private index: %w%s", err, stderrSuffix(stderr))
|
||||
}
|
||||
} else if !os.IsNotExist(err) {
|
||||
return "", err
|
||||
}
|
||||
if err := repo.StageAll(stageCtx, env...); err != nil {
|
||||
return "", fmt.Errorf("stage workspace snapshot: %w", err)
|
||||
}
|
||||
@@ -257,9 +280,8 @@ func runGitEnv(parent context.Context, repo gitcmd.Repo, env []string, args ...s
|
||||
return outBuf.String(), strings.TrimSpace(errBuf.String()), err
|
||||
}
|
||||
|
||||
// copyFile copies an index keeping its modification time: git re-hashes an
|
||||
// entry written in the index's own second only while the index file's mtime
|
||||
// says so, and a fresh mtime would hide a same-size edit made in that second.
|
||||
// copyFile preserves the index bytes and timestamp. workingTree rebuilds its
|
||||
// cached entries before staging the current workspace content.
|
||||
func copyFile(from, to string) error {
|
||||
in, err := os.Open(from)
|
||||
if err != nil {
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
package worktree
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestSnapshotHashesBytesDespiteTrustedCachedStat(t *testing.T) {
|
||||
for _, policy := range []string{"stat-cache", "assume-unchanged", "ignore-stat"} {
|
||||
t.Run(policy, func(t *testing.T) { testSnapshotCurrentBytes(t, policy) })
|
||||
}
|
||||
}
|
||||
|
||||
func testSnapshotCurrentBytes(t *testing.T, policy string) {
|
||||
t.Helper()
|
||||
requireGit(t)
|
||||
root := t.TempDir()
|
||||
gitIn(t, root, "init", "-q")
|
||||
gitIn(t, root, "config", "core.trustctime", "false")
|
||||
gitIn(t, root, "config", "core.checkstat", "minimal")
|
||||
if policy == "ignore-stat" {
|
||||
gitIn(t, root, "config", "core.ignorestat", "true")
|
||||
}
|
||||
writeRepoFile(t, root, "input", "old\n")
|
||||
writeRepoFile(t, root, ".gitignore", "staged-ignored\n")
|
||||
path := filepath.Join(root, "input")
|
||||
stamp := time.Now().Add(-time.Hour).Truncate(time.Second)
|
||||
if err := os.Chtimes(path, stamp, stamp); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
gitIn(t, root, "add", "-A")
|
||||
gitIn(t, root, "commit", "-qm", "base")
|
||||
if policy == "assume-unchanged" {
|
||||
gitIn(t, root, "update-index", "--assume-unchanged", "input")
|
||||
}
|
||||
writeRepoFile(t, root, "staged-ignored", "must survive\n")
|
||||
gitIn(t, root, "add", "-f", "staged-ignored")
|
||||
index := filepath.Join(root, ".git", "index")
|
||||
before, err := os.ReadFile(index)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
writeRepoFile(t, root, "input", "new\n")
|
||||
if err := os.Chtimes(path, stamp, stamp); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// The ordinary stat cache really does hide this edit; otherwise the test
|
||||
// would not distinguish hashing bytes from copying Git's cached version.
|
||||
if diff := gitIn(t, root, "diff", "--", "input"); diff != "" {
|
||||
t.Fatalf("fixture did not reproduce a trusted stale stat cache: %s", diff)
|
||||
}
|
||||
snapshot, err := TakeSnapshot(t.Context(), opened(t, root))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got := gitIn(t, root, "show", snapshot.Tree+":input"); got != "new\n" {
|
||||
t.Fatalf("snapshot reused stale stat cache: %q", got)
|
||||
}
|
||||
if got := gitIn(t, root, "show", snapshot.Tree+":staged-ignored"); got != "must survive\n" {
|
||||
t.Fatalf("private index rebuild lost staged ignored input: %q", got)
|
||||
}
|
||||
after, err := os.ReadFile(index)
|
||||
if err != nil || !bytes.Equal(before, after) {
|
||||
t.Fatalf("snapshot modified the user's index: %v", err)
|
||||
}
|
||||
if got := strings.TrimSpace(gitIn(t, root, "show", "HEAD:input")); got != "old" {
|
||||
t.Fatalf("snapshot changed HEAD: %s", got)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,60 @@
|
||||
package worktree
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestSnapshotRecordsAWorkspaceMidConflict(t *testing.T) {
|
||||
requireGit(t)
|
||||
repo := initRepo(t)
|
||||
git := func(args ...string) error {
|
||||
cmd := exec.Command("git", append([]string{"-C", repo}, args...)...)
|
||||
cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1",
|
||||
"GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@t", "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@t")
|
||||
return cmd.Run()
|
||||
}
|
||||
write := func(body string) {
|
||||
if err := os.WriteFile(filepath.Join(repo, "README.md"), []byte(body), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
if err := git("checkout", "-q", "-b", "other"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
write("other\n")
|
||||
if err := git("commit", "-qam", "other"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := git("checkout", "-q", "-"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
write("mine\n")
|
||||
if err := git("commit", "-qam", "mine"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := git("merge", "-q", "other"); err == nil {
|
||||
t.Fatal("fixture merge did not conflict")
|
||||
}
|
||||
index := filepath.Join(repo, ".git", "index")
|
||||
before, err := os.ReadFile(index)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
snap, err := TakeSnapshot(t.Context(), opened(t, repo))
|
||||
if err != nil {
|
||||
t.Fatalf("a conflicted index blocked the snapshot: %v", err)
|
||||
}
|
||||
out, err := exec.Command("git", "-C", repo, "cat-file", "-p", snap.Tree+":README.md").Output()
|
||||
if err != nil || !strings.Contains(string(out), "<<<<<<<") {
|
||||
t.Fatalf("snapshot did not record the working file: %q %v", out, err)
|
||||
}
|
||||
after, err := os.ReadFile(index)
|
||||
if err != nil || !bytes.Equal(before, after) {
|
||||
t.Fatalf("snapshot rewrote the user's conflicted index: %v", err)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,2 @@
|
||||
- 修复文件大小和修改时间未变时,候选快照可能复用 Git 缓存中的旧内容;同时覆盖 `assume-unchanged` 和 `core.ignorestat` 设置。关联 #14025。
|
||||
- 私有索引重建保留用户暂存区、已暂存的忽略文件和未解决的合并冲突现场。关联 #14025。
|
||||
Reference in new issue
Block a user