test: usage's and provider's TestMains clear the agents' variables too, and a new agent's variable left out of the list now fails (yetone, reviewing #543: internal/usage/main_test.go's TestMain still clears no agent variables, so a CLAUDE_CONFIG_DIR left in the shell reaches the real ~/.claude there too; nothing fails yet when a new agent's variable is left out of Vars) — both TestMains gave their package a home of its own and cleared no agent variable, the provider one as much as the usage one that was named. A CLAUDE_CONFIG_DIR left in the shell reached the real ~/.claude through the ledger's sessions.Calls (usage/ledger.go:122): with a canary Claude folder holding three of the sessions package's fixtures, six usage tests fail — TestLedger, TestSubscriptionListPrice, TestLedgerUsesTheStatedPrice, TestLedgerConfiguredPricesAndRefresh and the two Antigravity ledger ones — and pass with the clearing; provider passes with the same canary either way, but its Accounts read what an agent keeps on the machine too. #543 missed these by how it looked for its sites: by the clearing loops a sandbox already had, which one with no loop at all has no way to answer. TestFolderVarsAreListed reads the sources of the four packages an agent's folder variable is read in — this one, sessions, provider and library, the rest of the repository reading only magpie's own variables, the process' and the desktop's — and fails on a name that is neither in agentenv.Vars nor excused here with a reason, so a new agent's variable no longer waits for a developer's shell to meet one. It reads them as files, since a test in sessions or provider cannot import this package, which imports them; it asks for one name each way its pattern matches, so a change in how a variable is read empties the scan loudly instead of passing it, and for every name excused, so no excuse outlives its variable. What it cannot see is a name the sources build at runtime, as omoDir reads its two out of a slice (omo.go:33); both are listed, and a third taken the same way would not be caught. The package doc's VarsTest is TestVarsAreRead. TestFolderVarsAreListed fails without the change when KIMI_CODE_HOME is taken out of Vars, or when a pretend NEWAGENT_HOME is read in agents.go, and names the file that reads it; the six usage tests fail without theirs. On Windows 11 the whole suite answers as it did before the change: 31 packages pass, and the same 15 tests in internal/library, internal/gateway and internal/agent fail for what they failed for before. go vet ./..., go build -tags nogui (windows, GOOS=linux, GOOS=darwin), go vet -tags nogui for linux and darwin over every package touched, and gofmt pass. (#549)
This commit is contained in:
1 parent
34538b0a4b
commit
7586df4400
4 files changed
+126
-4
No files matched your search
@@ -0,0 +1,107 @@
|
||||
package agent
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"slices"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/yetone/magpie/internal/agentenv"
|
||||
)
|
||||
|
||||
// readsEnv matches the two ways these packages name a variable they read to
|
||||
// find an agent's folder: as the argument of a Getenv, a LookupEnv or a
|
||||
// place's getenv, and as the env of a table entry, which is how Qoder's two
|
||||
// builds are named (read through at.getenv(b.env)).
|
||||
var readsEnv = regexp.MustCompile(`(?:Getenv|LookupEnv|getenv)\("([A-Z][A-Z_0-9]*)"\)|env:\s*"([A-Z][A-Z_0-9]*)"`)
|
||||
|
||||
// notAnAgent are the variables the same sources read that are not an agent's
|
||||
// folder, so no sandbox clears them through agentenv.Vars.
|
||||
var notAnAgent = map[string]string{
|
||||
"APPDATA": "Windows' own folder, which a sandbox sets to one of its own rather than clears",
|
||||
"LOCALAPPDATA": "the same",
|
||||
"PATH": "the process' own",
|
||||
"TZ": "the process' own",
|
||||
"USER": "the process' own",
|
||||
"XDG_CONFIG_HOME": "magpie's own folder's, which appdir decides",
|
||||
"XDG_DATA_HOME": "the same",
|
||||
"MAGPIE_ADDR": "magpie's own",
|
||||
}
|
||||
|
||||
// TestFolderVarsAreListed says every variable the packages that find an
|
||||
// agent's folder read is one a sandbox clears. Nothing said so before, so a
|
||||
// new agent could take a variable of its own and every sandbox would go on
|
||||
// meeting the agent a developer really has installed — which is how #522 came
|
||||
// about, four sandboxes clearing lists of their own that had each drifted.
|
||||
//
|
||||
// The four packages are this one, sessions, provider and library: every
|
||||
// agent's folder variable magpie reads is read in one of them, and the rest
|
||||
// of the repository reads only magpie's own variables, the process' and the
|
||||
// desktop's. Their sources are read as files rather than imported, since a
|
||||
// test in sessions or provider cannot import this package, which imports
|
||||
// them; what the walk looks at is what they say, not what they do.
|
||||
//
|
||||
// What this cannot see is a variable read through a name the sources build at
|
||||
// runtime, as omoDir reads OMO_CODING_AGENT_DIR and SENPI_CODING_AGENT_DIR
|
||||
// out of a slice. Both are in agentenv.Vars; a third taken the same way would
|
||||
// not be caught here.
|
||||
func TestFolderVarsAreListed(t *testing.T) {
|
||||
listed := make(map[string]bool, len(agentenv.Vars))
|
||||
for _, v := range agentenv.Vars {
|
||||
listed[v] = true
|
||||
}
|
||||
dirs := []string{".", filepath.Join("..", "sessions"), filepath.Join("..", "provider"), filepath.Join("..", "library")}
|
||||
found := map[string]string{}
|
||||
for _, dir := range dirs {
|
||||
entries, err := os.ReadDir(dir)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, e := range entries {
|
||||
n := e.Name()
|
||||
if e.IsDir() || !strings.HasSuffix(n, ".go") || strings.HasSuffix(n, "_test.go") {
|
||||
continue
|
||||
}
|
||||
b, err := os.ReadFile(filepath.Join(dir, n))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, m := range readsEnv.FindAllStringSubmatch(string(b), -1) {
|
||||
name := m[1]
|
||||
if name == "" {
|
||||
name = m[2]
|
||||
}
|
||||
if _, ok := found[name]; !ok {
|
||||
found[name] = filepath.Join(dir, n)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
// One name read each way the pattern matches, so that a change in how a
|
||||
// variable is read empties the scan loudly instead of leaving the test
|
||||
// nothing to complain of.
|
||||
for _, canary := range []string{"CODEX_HOME", "QODER_CONFIG_DIR"} {
|
||||
if _, ok := found[canary]; !ok {
|
||||
t.Errorf("%s was not found in these sources, so readsEnv no longer matches the way a variable is read", canary)
|
||||
}
|
||||
}
|
||||
// And no excuse outlives the variable it was for.
|
||||
for n := range notAnAgent {
|
||||
if _, ok := found[n]; !ok {
|
||||
t.Errorf("%s is in notAnAgent, but nothing in these sources reads it any more", n)
|
||||
}
|
||||
}
|
||||
names := make([]string, 0, len(found))
|
||||
for n := range found {
|
||||
names = append(names, n)
|
||||
}
|
||||
slices.Sort(names)
|
||||
for _, n := range names {
|
||||
if listed[n] || notAnAgent[n] != "" {
|
||||
continue
|
||||
}
|
||||
t.Errorf("%s, read by %s, is not in agentenv.Vars, so no sandbox clears it and a test meets the agent a developer really has installed: add it to Vars, or to notAnAgent here with a reason if it is not an agent's folder", n, found[n])
|
||||
}
|
||||
}
|
||||
@@ -15,10 +15,10 @@
|
||||
// Each package keeps its own sandbox, because a test in internal/sessions
|
||||
// cannot import internal/agent, which imports it, to ask which variables
|
||||
// there are; this is the list they all clear. A name here is one magpie
|
||||
// reads, and VarsTest says so, so a variable magpie stopped reading does not
|
||||
// linger — that is how GEMINI_CLI_HOME and OPENCODE_CONFIG came to be
|
||||
// cleared by a sandbox while OPENCODE_CONFIG_DIR, the one magpie reads, was
|
||||
// not. What is deliberately not here: APPDATA and LOCALAPPDATA, Windows'
|
||||
// reads, and TestVarsAreRead says so, so a variable magpie stopped reading
|
||||
// does not linger — that is how GEMINI_CLI_HOME and OPENCODE_CONFIG came to
|
||||
// be cleared by a sandbox while OPENCODE_CONFIG_DIR, the one magpie reads,
|
||||
// was not. What is deliberately not here: APPDATA and LOCALAPPDATA, Windows'
|
||||
// folders rather than an agent's, which a sandbox sets to a folder of its
|
||||
// own instead of clearing; and CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC, a
|
||||
// behaviour flag whose one test sets it on purpose to read what magpie then
|
||||
|
||||
@@ -4,6 +4,8 @@ import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/yetone/magpie/internal/agentenv"
|
||||
)
|
||||
|
||||
// TestMain keeps the package's tests off the Kiro sign-ins of whoever runs
|
||||
@@ -25,6 +27,11 @@ func TestMain(m *testing.M) {
|
||||
} {
|
||||
os.Setenv(k, v)
|
||||
}
|
||||
// Accounts here read what an agent keeps on this machine: a variable left
|
||||
// in the shell would point a test at the runner's real one (#522).
|
||||
for _, k := range agentenv.Vars {
|
||||
os.Unsetenv(k)
|
||||
}
|
||||
kiroCLIDB = func() string { return filepath.Join(dir, "kiro-cli", "data.sqlite3") }
|
||||
kiroIDEDir = func() string { return filepath.Join(dir, "sso") }
|
||||
askKiroIdentity = func(string, string) (string, string) { return "", "" }
|
||||
|
||||
@@ -4,6 +4,8 @@ import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/yetone/magpie/internal/agentenv"
|
||||
)
|
||||
|
||||
// TestMain gives the package a home of its own: a test that doesn't set one
|
||||
@@ -21,6 +23,12 @@ func TestMain(m *testing.M) {
|
||||
} {
|
||||
os.Setenv(k, v)
|
||||
}
|
||||
// The package's ledger reads what sessions finds, which asks each agent's
|
||||
// own variable before its folder in the home: one left in the shell would
|
||||
// point a test at the runner's real agent (#522).
|
||||
for _, k := range agentenv.Vars {
|
||||
os.Unsetenv(k)
|
||||
}
|
||||
code := m.Run()
|
||||
os.RemoveAll(home)
|
||||
os.Exit(code)
|
||||
|
||||
Reference in new issue
Block a user