mirror of
https://github.com/esengine/DeepSeek-Reasonix.git
synced 2026-11-07 10:45:59 -10:00
fix(completion): read build caches from their own source
Two defects, both found by CI on Windows and both in this PR's own new code.
`underScratchOrCache` asked `hostwrite.Dirs()` for the toolchain caches. That
function answers what a jail grants a command, and Windows jails nothing, so it
returns nil there: every build cache write was reported as out of sight on the
one platform where the list is empty. `hostwrite.Caches()` answers the question
actually being asked — where does a build write through — and is defined on every
platform. The two questions stay apart because conflating them is what made a
Windows build produce a caveat about its own module cache.
The one-off directory `sessiontemp` hands a turn is recognised by its position,
not by its name. It is a direct child of the host's temporary root, so that root
is read from the host and the first component below it is compared against the
prefix. The previous version walked every component, which exempted any folder
deeper in the tree that happened to spell the prefix, and it assumed the root was
named `/tmp`, which is not where Windows puts it.
The tests that pinned these edges set `HOME` and `TMPDIR` and then read the
result, which is why they agreed with the code on Linux and disagreed with it
everywhere else. `HOME` and `TMPDIR` are POSIX-only names: on Windows the
temporary root comes from `TEMP`, so `t.Setenv("TMPDIR", …)` changed nothing and
the assertions ran against the real root. They now call `disposable` with the
roots as arguments, and build paths from the host's own temp root, so each
assertion is about the rule rather than about an environment.
Tested: the thirteen out-of-scope tests, the packages this PR touches, and
`GOOS=windows go vet` on `internal/runtime/agent` and `internal/base/hostwrite`.
This commit is contained in:
1 parent
a440f83b0a
commit
f9de73aeda
5 files changed
+94
-55
No files matched your search
@@ -16,6 +16,24 @@ func Dirs() []string {
|
||||
return dirsFor(runtime.GOOS)
|
||||
}
|
||||
|
||||
// Caches lists the toolchain caches a build writes through, on every platform.
|
||||
// A check that must not trust what a cache holds reads this rather than Dirs:
|
||||
// Dirs is what a jail grants, and a host that jails nothing grants nothing.
|
||||
func Caches() []string {
|
||||
var dirs []string
|
||||
if dir, err := os.UserCacheDir(); err == nil && dir != "" {
|
||||
dirs = append(dirs, dir)
|
||||
}
|
||||
home, err := os.UserHomeDir()
|
||||
if err != nil {
|
||||
return dirs
|
||||
}
|
||||
for _, sub := range []string{".cargo", ".npm", "go"} {
|
||||
dirs = append(dirs, filepath.Join(home, sub))
|
||||
}
|
||||
return dirs
|
||||
}
|
||||
|
||||
func dirsFor(goos string) []string {
|
||||
home, homeErr := os.UserHomeDir()
|
||||
switch goos {
|
||||
|
||||
@@ -20,3 +20,21 @@ func TestDirsNameTheTemporaryAndCacheDirectoriesPerPlatform(t *testing.T) {
|
||||
t.Fatalf("linux = %v, want the cache directory", dirsFor("linux"))
|
||||
}
|
||||
}
|
||||
|
||||
// Caches is read by checks that must not trust what a cache holds, so it names
|
||||
// the build caches on every platform — unlike Dirs, which is what a jail grants
|
||||
// and is empty where nothing is jailed.
|
||||
func TestCachesNameTheBuildCachesOnEveryPlatform(t *testing.T) {
|
||||
if got := Caches(); len(got) == 0 {
|
||||
t.Fatal("Caches named nothing: a check reading it would trust every cache")
|
||||
}
|
||||
for _, want := range []string{"go", ".cargo", ".npm"} {
|
||||
if !slices.ContainsFunc(Caches(), func(d string) bool { return filepath.Base(d) == want }) {
|
||||
t.Fatalf("Caches = %v, want %s", Caches(), want)
|
||||
}
|
||||
}
|
||||
// The two answers are not the same question, and Windows is where they part.
|
||||
if got := dirsFor("windows"); got != nil {
|
||||
t.Fatalf("dirsFor(windows) = %v, want none: nothing is jailed there", got)
|
||||
}
|
||||
}
|
||||
@@ -40,7 +40,8 @@ func TestTurnWhoseWritesFallOutsideTheWorkspaceIsNotReportedAsFinished(t *testin
|
||||
workspace := t.TempDir()
|
||||
// Deliberately not t.TempDir(): a temp directory is exempt from this rule,
|
||||
// and the case being pinned is a write to a place the person meant to keep.
|
||||
target := filepath.Join(string(filepath.Separator), "outside-watch", "Reasonix CLI.desktop")
|
||||
// The volume comes from the host, so this is absolute on Windows too.
|
||||
target := filepath.Join(filepath.VolumeName(workspace)+string(filepath.Separator), "outside-watch", "Reasonix CLI.desktop")
|
||||
|
||||
reg := tool.NewRegistry()
|
||||
reg.Add(readProbe{})
|
||||
|
||||
@@ -35,21 +35,28 @@ func (a *Agent) outsideObservedWrites() []string {
|
||||
return a.relativeUnique(outside)
|
||||
}
|
||||
|
||||
// underScratchOrCache reports a path the host owns and disposes of: inside the
|
||||
// session's own scratch directory, or inside a toolchain cache. A public
|
||||
// temporary root is not exempt on its own — a person who asked for a file there
|
||||
// asked for something to keep, and nothing the host watches says it arrived.
|
||||
// underScratchOrCache reports a path the host disposes of on its own: inside the
|
||||
// directory this session was handed for temporary work, or inside a toolchain
|
||||
// cache. A shared temporary root is neither — a person who asked for a file
|
||||
// there asked for something to keep, and nothing the host watches says it
|
||||
// arrived.
|
||||
func underScratchOrCache(path string) bool {
|
||||
return disposable(path, os.TempDir(), hostwrite.Caches())
|
||||
}
|
||||
|
||||
// disposable is the rule above with the host's roots passed in, so its edges are
|
||||
// decidable without an environment that happens to sit where the rule looks.
|
||||
func disposable(path, tempRoot string, caches []string) bool {
|
||||
if !filepath.IsAbs(path) {
|
||||
return false
|
||||
}
|
||||
target := filepath.Clean(path)
|
||||
if withoutSessionScratch(target) != target {
|
||||
if insideSessionScratch(target, tempRoot) {
|
||||
return true
|
||||
}
|
||||
for _, dir := range hostwrite.Dirs() {
|
||||
for _, dir := range caches {
|
||||
dir = strings.TrimSpace(dir)
|
||||
if dir == "" || hostTempRoot(dir) {
|
||||
if dir == "" {
|
||||
continue
|
||||
}
|
||||
clean := filepath.Clean(dir)
|
||||
@@ -66,31 +73,18 @@ func underScratchOrCache(path string) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
// withoutSessionScratch drops the prefix of path through the session's scratch
|
||||
// directory, or returns path unchanged when it does not lie inside one. A
|
||||
// directory earns that name only as a direct child of a root the host uses for
|
||||
// temporary files: any directory could otherwise claim the name.
|
||||
func withoutSessionScratch(path string) string {
|
||||
prefix := sessiontemp.Prefix()
|
||||
for dir := path; ; dir = filepath.Dir(dir) {
|
||||
if strings.HasPrefix(filepath.Base(dir), prefix) && hostTempRoot(filepath.Dir(dir)) {
|
||||
return filepath.Dir(dir)
|
||||
}
|
||||
if parent := filepath.Dir(dir); parent == dir {
|
||||
return path
|
||||
}
|
||||
// insideSessionScratch reports a path under a directory sessiontemp creates:
|
||||
// directly beneath the temporary root, named with the scratch prefix. Only the
|
||||
// first component below that root is read, so a folder deeper in the tree cannot
|
||||
// earn an exemption by spelling the prefix.
|
||||
func insideSessionScratch(path, tempRoot string) bool {
|
||||
if tempRoot == "" {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// hostTempRoot reports a directory that is merely where temporary files go.
|
||||
func hostTempRoot(dir string) bool {
|
||||
clean := filepath.Clean(dir)
|
||||
if temp := os.TempDir(); temp != "" && clean == filepath.Clean(temp) {
|
||||
return true
|
||||
rel, err := filepath.Rel(filepath.Clean(tempRoot), path)
|
||||
if err != nil || rel == "." || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
|
||||
return false
|
||||
}
|
||||
switch clean {
|
||||
case "/tmp", "/private/tmp", "/private/var/folders":
|
||||
return true
|
||||
}
|
||||
return false
|
||||
first, _, _ := strings.Cut(rel, string(filepath.Separator))
|
||||
return strings.HasPrefix(first, sessiontemp.Prefix())
|
||||
}
|
||||
@@ -3,6 +3,7 @@ package agent
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"slices"
|
||||
"strings"
|
||||
@@ -20,9 +21,12 @@ import (
|
||||
|
||||
// outsideTarget is a path outside every root this host watches and outside the
|
||||
// temp directory, which is exempt from the rule under test. It is never
|
||||
// written to: the stub writer reports the write instead of performing one.
|
||||
// written to: the stub writer reports the write instead of performing one. The
|
||||
// volume comes from the host so the path is absolute on Windows too, where a
|
||||
// leading separator alone names no drive and the path would read as relative.
|
||||
func outsideTarget(name string) string {
|
||||
return filepath.Join(string(filepath.Separator), "outside-watch", name)
|
||||
vol := filepath.VolumeName(os.TempDir())
|
||||
return filepath.Join(vol+string(filepath.Separator), "outside-watch", name)
|
||||
}
|
||||
|
||||
// ordinaryTurnAgent is a workspace with a read tool and a stub writer, so a
|
||||
@@ -87,12 +91,10 @@ func TestAReadOnlyInvestigationEndsClean(t *testing.T) {
|
||||
// exemption is what keeps every investigation that used a probe from ending
|
||||
// under a caveat.
|
||||
func TestAScratchWriteCarriesNoCaveat(t *testing.T) {
|
||||
// Named explicitly rather than left to t.TempDir(): which root the suite
|
||||
// runs under is an environment fact, and a test whose meaning depends on it
|
||||
// passes here and fails everywhere else.
|
||||
tempRoot := t.TempDir()
|
||||
t.Setenv("TMPDIR", tempRoot)
|
||||
scratch := filepath.Join(tempRoot, sessiontemp.Prefix()+"probe", "probe.go")
|
||||
// Built from the host's own temp root rather than a variable this test sets:
|
||||
// which names that root goes by is the host's fact on every platform, and
|
||||
// the directory sessiontemp creates is always a direct child of it.
|
||||
scratch := filepath.Join(os.TempDir(), sessiontemp.Prefix()+"probe", "probe.go")
|
||||
a, _ := ordinaryTurnAgent(t,
|
||||
testutil.Turn{ToolCalls: []provider.ToolCall{{ID: "w1", Name: "write_file", Arguments: fmt.Sprintf(`{"path":%q}`, scratch)}}},
|
||||
testutil.Turn{Text: "探针跑完了,结论见上面。"},
|
||||
@@ -193,10 +195,9 @@ func TestOutsideWriteDetailIsBounded(t *testing.T) {
|
||||
// path. A person who asks for a file under the public temp directory asked for
|
||||
// something the host cannot verify afterwards, and that has to show.
|
||||
func TestADeliverableInThePublicTempDirectoryIsNotExempt(t *testing.T) {
|
||||
// The suite runs with TMPDIR pointed at the session scratch directory, so
|
||||
// the public temp root is named here rather than read from the environment.
|
||||
t.Setenv("TMPDIR", "/tmp")
|
||||
target := filepath.Join("/tmp", "deliverable.txt")
|
||||
// Beside the scratch directories, not inside one: the exemption names the
|
||||
// directory sessiontemp hands out, not the root it sits in.
|
||||
target := filepath.Join(os.TempDir(), "deliverable.txt")
|
||||
if underScratchOrCache(target) {
|
||||
t.Fatalf("%s is exempt; a path the host cannot verify afterwards must not be", target)
|
||||
}
|
||||
@@ -242,26 +243,33 @@ func TestTheSameReceiptsGoFromDoneToPartial(t *testing.T) {
|
||||
// resembles one. A folder a person could create under their own project must
|
||||
// not buy silence, and neither must an installed executable.
|
||||
func TestTheExemptionIsNarrowerThanALookalikePath(t *testing.T) {
|
||||
home := t.TempDir()
|
||||
t.Setenv("HOME", home)
|
||||
t.Setenv("TMPDIR", filepath.Join(home, "tmp"))
|
||||
// The host's roots are stated rather than steered through variables, which
|
||||
// each platform spells differently; the rule under test takes them as input.
|
||||
root := t.TempDir()
|
||||
tempRoot := filepath.Join(root, "tmp")
|
||||
caches := []string{filepath.Join(root, "go")}
|
||||
|
||||
for _, dir := range []string{
|
||||
filepath.Join(home, "projects", "reasonix-session-tmp-backup"),
|
||||
filepath.Join(home, "work", "deep", "reasonix-session-tmp-x"),
|
||||
filepath.Join(root, "projects", "reasonix-session-tmp-backup"),
|
||||
filepath.Join(tempRoot, "deep", "reasonix-session-tmp-x"),
|
||||
} {
|
||||
if underScratchOrCache(filepath.Join(dir, "notes.md")) {
|
||||
if disposable(filepath.Join(dir, "notes.md"), tempRoot, caches) {
|
||||
t.Fatalf("%s was exempt: a directory can claim the scratch name by spelling it", dir)
|
||||
}
|
||||
}
|
||||
|
||||
installed := filepath.Join(home, "go", "bin", "mytool")
|
||||
if underScratchOrCache(installed) {
|
||||
installed := filepath.Join(root, "go", "bin", "mytool")
|
||||
if disposable(installed, tempRoot, caches) {
|
||||
t.Fatalf("%s was exempt: go install puts the user's own executable there", installed)
|
||||
}
|
||||
cached := filepath.Join(home, "go", "pkg", "mod", "cache", "x.go")
|
||||
if !underScratchOrCache(cached) {
|
||||
cached := filepath.Join(root, "go", "pkg", "mod", "cache", "x.go")
|
||||
if !disposable(cached, tempRoot, caches) {
|
||||
t.Fatalf("%s was not exempt: a module cache is what the build writes through", cached)
|
||||
}
|
||||
scratch := filepath.Join(tempRoot, sessiontemp.Prefix()+"run", "probe.go")
|
||||
if !disposable(scratch, tempRoot, caches) {
|
||||
t.Fatalf("%s was not exempt: the directory sessiontemp hands out is the host's to spend", scratch)
|
||||
}
|
||||
}
|
||||
|
||||
// A turn the host cannot judge still says what it saw. Dropping the gap at the
|
||||
|
||||
Reference in new issue
Block a user