Pin the MCP project tools' refusal of a present-but-unloadable jpack.json

Read the project.Present call sites in internal/mcp/tools.go and the related tests in internal/mcp/project_test.go; derive the test cases from those call sites, as the issue specifies. Check project.Load and each surface's toolError path for the loader and absent-configuration diagnostics, then run the relevant MCP tests. Done means every deciding surface is covered, the missing-path control succeeds, and the specified one-site-at-a-time mutations fail at the corresponding cases.

Goal

Add one test to internal/mcp/project_test.go that holds the presence guard every deciding MCP project tool runs before it loads a configuration, so a jpack.json that is demonstrably there and cannot be loaded refuses the call instead of being answered as a project that does not use the convention.

Why

internal/project/project.go ships two presence questions and names the difference between them:

// Exists reports whether a readable regular configuration file is at path. It is
// how a surface tells "this project does not use the convention" — an answer —
// from "the configuration is broken", which is a failure.
func Exists(configPath string) bool {
	info, err := os.Lstat(configPath)
	return err == nil && info.Mode().IsRegular()
}

// Present reports whether anything at all is at path, whatever it is.
//
// It is the narrower question's opposite, and the two are not interchangeable.
// Exists asks whether a *readable regular file* is there, so a symlink, a
// directory, or a special file all answer "no" — which a surface then reads as
// "this project does not use the convention". On a surface that decides, that
// reading is a fail-open: the configuration is demonstrably there, it would
// demonstrably fail to load, and the run would report a decision anyway,
// unrecorded, for a project that asked to be recorded. So a deciding surface
// asks this question instead, and anything present-but-unloadable reaches Load
// and is refused with Load's own diagnostic. An error that is not "nothing is
// there" also answers yes, for the same fail-closed reason.
func Present(configPath string) bool {
	_, err := os.Lstat(configPath)
	return !errors.Is(err, os.ErrNotExist)
}

toolExperimentalTestPacks' own doc comment (internal/mcp/tools.go:559) makes the choice load-bearing rather than incidental:

// Presence is tested with project.Present rather than Exists, so a
// configuration that is demonstrably there and unloadable refuses the call
// instead of answering as a project that does not use the convention.

and toolExperimentalEvaluate restates it at the point where the decision is actually at stake (internal/mcp/tools.go:934):

	// The project is resolved for every call this tool serves, not only for the
	// ones naming a pack by id: whether an evaluation is recorded is the
	// configuration's to say (ADR-0018), and a pack passed as text is still
	// evaluated in the project this server was launched in. A configuration that
	// is there and cannot be read refuses the call either way — and "there" is
	// mere presence, not a readable regular file, so nothing unloadable can be
	// mistaken for a project that does not use the convention.
	configPath := project.Locate("")
	present := project.Present(configPath)

Nothing on the MCP surface holds any of that today. Measured on 86250f4, one call site at a time, each reverted before the next:

$ sed -i '942s/project.Present(configPath)/project.Exists(configPath)/' internal/mcp/tools.go
$ git diff --unified=0 internal/mcp/tools.go
@@ -942 +942 @@ func (s *Server) toolExperimentalEvaluate(rawArgs json.RawMessage) any {
-	present := project.Present(configPath)
+	present := project.Exists(configPath)
$ env GO111MODULE=on go test ./... 2>&1 | grep -vE '^ok|no test files'; echo "TEST-EXIT=${PIPESTATUS[0]}"
TEST-EXIT=0

The same one-identifier relaxation applied at the experimental_test_packs site (line 613) and at the experimental_test_graphs site (line 775) each printed TEST-EXIT=0 as well — the entire suite passes with any of the three swapped.

What that buys, at the experimental_evaluate site, is a decision nobody records. With JPACK_CONFIG pointing at a directory named jpack.json and a conformant pack passed as text, the shipping code refuses:

No project configuration could be read as one bounded regular file at <path>. Pass --config, set JPACK_CONFIG, or add a jpack.json at the project root.

With Exists in its place the same call returns "isError": false, "status": "evaluated", and a real "disposition": {"kind":"unresolved", ...}. loaded stays nil, and (*project.Project).AuditWriter returns nil for a nil receiver (internal/project/project.go:997), so no audit record is appended and no reviewed set is consulted. That is exactly the fail-open ADR-0018 and ADR-0019 exist to prevent: a project that asked to be recorded gets an unrecorded decision.

Which layer refuses at the two matrix sites — and why an isError assertion is dead there. At experimental_test_packs and experimental_test_graphs the relaxed code still refuses. It just refuses for the wrong reason, and sends the operator looking for a file that is sitting right there. Measured under the line-613 swap, on a directory named jpack.json:

No project configuration was found at <path>/jpack.json, so there is no declared matrix to run. The convention is optional: a jpack.json at the project root, or the JPACK_CONFIG environment variable, declares packs and their matrices.

So a test that asserts only isError cannot discriminate at either matrix site. The refusal text is the whole assertion there.

No MCP fixture can tell the two apart. Every missing-configuration fixture on this surface uses a path where nothing exists, and on such a path Present and Exists agree:

$ sed -n '909p;910p' internal/mcp/project_test.go   # in TestExperimentalTestPacksWithNoConfigurationIsAToolError
	missing := filepath.Join(t.TempDir(), project.DefaultConfigName)
	t.Setenv(project.ConfigEnv, missing)
$ sed -n '1455p' internal/mcp/project_test.go       # in TestExperimentalTestGraphsWithNoConfigurationIsAToolError
	t.Setenv(project.ConfigEnv, filepath.Join(t.TempDir(), "absent.json"))
$ grep -rn "Mkdir" --include=*_test.go . | grep -i "jpack.json\|DefaultConfigName"
$ 

The same clause is already held — on the other surface. The CLI runs the identical guard at internal/cli/app.go:210, and one test holds it:

$ grep -rn "bounded regular file" --include=*_test.go .
internal/cli/audit_test.go:268:	if !strings.Contains(stderr, "bounded regular file") {

That is TestAConfigurationThatIsThereButUnloadableRefusesRatherThanBeingIgnored (internal/cli/audit_test.go:251), and it earns its keep: swapping project.Present(configFile) for project.Exists(configFile) at internal/cli/app.go:210 fails it at audit_test.go:266, printing the evaluation the CLI would otherwise have reported unrecorded. It builds its present-but-unloadable path with a symlink, which is why it opens with if runtime.GOOS == "windows" { t.Skip(...) }. So the behaviour is held on one of the four surfaces that run the guard, by a test that does not run on a quarter of CI, and unheld on the three MCP ones. This issue closes the MCP three, on a fixture that needs no skip.

The opposite direction is held, unevenly. Relaxing the guard the other way (present := true at the evaluate site) fails TestExperimentalEvaluateTool at server_test.go:283 and TestExperimentalEvaluateDistinguishesOmittedFromEmptyDocuments at server_test.go:411; deleting the guard at the experimental_test_packs site (if !project.Present(configPath) { → if false {) fails TestExperimentalTestPacksWithNoConfigurationIsAToolError at project_test.go:916, on its text assertion. Doing the same at the experimental_test_graphs site printed TEST-EXIT=0 for the whole suite, because that tool's absent-configuration test asserts only isError, never which of the two diagnoses came back. So experimental_test_graphs' configuration guard is unheld in both directions, and closing it is a one-line addition to a test that already exists.

Scope

• Change internal/mcp/project_test.go only.
• Build the present-but-unloadable fixture with os.Mkdir on the configuration path, plus t.Setenv(project.ConfigEnv, …). A directory is the one unloadable shape that behaves identically on all four CI legs (.github/workflows/ci.yml: ubuntu stable, ubuntu 1.24.x, macos, windows) — and not by luck: fssecure.Root.Open fstats the handle it just opened and refuses !Mode().IsRegular() in Go code (internal/fssecure/root.go:258), so the refusal never depends on what read(2) does to a directory descriptor on a given platform. A symlink or a special file would need a runtime.GOOS guard, as the existing fixtures at internal/project/project_test.go:248 and internal/mcp/project_test.go:319 do.
• Do not change internal/mcp/tools.go, internal/project/project.go, Present, Exists, or Load.
• Do not change any refusal string, or the toolError calls that produce them. If a message reads wrong to you, say so in the pull request; this issue is about holding what ships.
• Do not add a runtime.GOOS skip, and do not reach for t.Skip at all.
• Do not touch internal/cli or the CLI's own --config handling. The CLI runs the same guard at internal/cli/app.go:210, but it is already held by internal/cli/audit_test.go:251; leave it alone.
• Do not add a new fixture helper to internal/project.

Acceptance criteria

• The set of surfaces the test covers is derived from the source, not from this issue: grep -n 'project\.Present(' internal/mcp/tools.go names every deciding surface that runs the guard, and the test has a case for each one the grep returns. Anchor on the open paren — the unanchored spelling also matches the doc comment that explains the choice, which is prose, not a call site. If the grep returns more sites than this issue mentions, cover them and say so.
• Each case has exactly one obstacle: a directory stands where the configuration file belongs, and nothing else about the project or the call is wrong. No case combines the unloadable configuration with a second fault — a malformed pack, a missing facts, an unknown pack_id — because such a case refuses for the other fault and passes without the clause under test ever running.
• The experimental_evaluate case passes a pack as text that would evaluate successfully on its own (the bundled internal/evaluation/testdata/data-request-intake-triage.json with the file's own projectFacts constant does). A pack that would be refused anyway makes the assertion pass on broken code as well as correct code, and discriminates nothing.
• Every case asserts isError is true, asserts the refusal carries the loader's own diagnostic, and asserts it does not carry the absent-configuration phrasing. Derive both strings by reading project.Load / loadThrough and the surfaces' toolError calls — do not paste literals out of this issue. The two text assertions are the ones that do the work: at the two matrix sites the relaxed code still returns isError true, so an isError-only case is dead there.
• A control case in the same test points JPACK_CONFIG at a path where nothing exists and asserts the same experimental_evaluate call succeeds (isError false). This makes the refusal above attributable to the configuration's file type rather than to the fixture's pack. Note in the pull request that this control is for attribution: the opposite relaxation is already held by TestExperimentalEvaluateTool, so the control is not expected to kill a mutation of its own.
• TestExperimentalTestGraphsWithNoConfigurationIsAToolError additionally asserts which diagnosis it got, not only that the call refused.
• Mutation check, run locally and reported in the pull request, naming the exact relaxation applied. Replace project.Present(configPath) with project.Exists(configPath) at each site the grep named, one site at a time, and confirm the new test fails at each — and that the failure names which surface fell open. Then, separately, replace the experimental_test_graphs guard's condition (if !project.Present(configPath) { → if false {) and confirm the strengthened absent-configuration assertion fails. After each, restore with git checkout -- internal/mcp/tools.go and confirm git status --porcelain prints nothing. If any relaxation leaves the suite green, say so in the pull request rather than dropping it quietly — a relaxation nothing catches is the finding.
• env GO111MODULE=on go fmt ./... leaves git status clean.
• env GO111MODULE=on go vet ./... is clean.
• env GO111MODULE=on go test ./... passes.
• env GO111MODULE=on CGO_ENABLED=0 go build -trimpath ./cmd/jpack succeeds; delete the resulting ./jpack binary and confirm git status is clean.
• The pull request carries its one-line Material-decision impact: declaration (tests only, no material-decision change).
• Every commit includes a DCO sign-off created with git commit -s.

Contributor learning

The difference between failing open and failing closed, in two four-line Go functions that answer almost the same question — "is anything there" versus "is a readable regular file there" — and why a surface that decides has to ask the wider one. Along the way: how to tell a test fixture that reaches a branch from one that holds it; why a fixture on which correct and broken code produce identical output cannot discriminate no matter how strong the assertion; how to find out which layer actually refused an input before crediting a test with catching it — here, two of the three relaxations still return isError true, so the obvious assertion is the dead one; and how to pick a fixture whose refusal comes from a check you can point at in Go rather than from platform behaviour you would otherwise have to skip a CI leg to avoid.