fleetd #134: make tool-surface neutralization visible to the daemon and the worker
isolateToolSurface replaced .mcp.json/opencode.json/.autoenv with neutral stubs and
marked them --skip-worktree, but said nothing anywhere. A real worker read a 3-byte
{} stub for opencode.json, where the repo's real file is 30+ lines, and truthfully
(but wrongly) reported a mount key did not exist.
Two readers, two fixes:
- the daemon operator gets one info log per provisioning, naming the denominator,
what was neutralized, and why anything was not (same shape as overlayParity's
fix in #148 point 3).
- the worker gets the same fact recorded in worktree-scoped git config
(fleet.neutralizedConfig / fleet.neutralizedConfigNote), discoverable with
`git config --worktree --get-all fleet.neutralizedConfig` from inside its own
worktree, without asking the lead. Not a working-tree file: this repo already
uses worktree-scoped config for the credential helper and the SSH->HTTPS
rewrite, and it lives under .git/worktrees/<nonce>/ so it can never appear in
`git status` for the worker to trip on or commit.
The neutralization itself (stub content, --skip-worktree marking) is unchanged.
This commit is contained in:
@@ -426,19 +426,75 @@ public final class GitWorktrees implements Worktrees {
|
||||
* neutralized copy from ever showing up as a local modification the worker might commit. A config
|
||||
* the repo does not carry is skipped silently — no stub is invented for a file the repo does not
|
||||
* have, and one missing file must never fail provisioning.
|
||||
*
|
||||
* <p>fleetd #134. The neutralization above is correct and stays unconditional — the defect was
|
||||
* that it was invisible on both sides. Neither the daemon's own log nor the worker sitting in the
|
||||
* worktree could tell a stub from the repo's real file: a real worker read a 3-byte {@code {}}
|
||||
* where the repo's {@code opencode.json} is 30+ lines, and reported — truthfully from what it
|
||||
* could see, and wrongly — that a mount key did not exist. Two fixes, aimed at two different
|
||||
* readers:
|
||||
* <ul>
|
||||
* <li>the daemon operator reads {@link #log}, so the summary below names the denominator, what
|
||||
* was neutralized, and why anything was not — the same shape {@code overlayParity} reports
|
||||
* its own copy in;
|
||||
* <li>the worker reads its own worktree, not the daemon's log, so the same fact is recorded a
|
||||
* second time in worktree-scoped git config ({@code fleet.neutralizedConfig} /
|
||||
* {@code fleet.neutralizedConfigNote}, readable with {@code git config --worktree --get-all
|
||||
* fleet.neutralizedConfig}) rather than as a file in the working tree. A working-tree file
|
||||
* would show up in {@code git status} for the worker to trip on or commit; worktree-scoped
|
||||
* config lives in {@code .git/worktrees/<nonce>/config.worktree} and can never appear there.
|
||||
* This reuses the exact mechanism {@link #configureEnvironmentCredentialHelper} and
|
||||
* {@link #configureHttpsUrlRewriteForSshOrigin} already use for other worktree-local state.
|
||||
* </ul>
|
||||
*/
|
||||
private void isolateToolSurface(String worktreePath) {
|
||||
Path root = Path.of(worktreePath).toAbsolutePath().normalize();
|
||||
List<String> neutralized = new ArrayList<>();
|
||||
List<String> skipped = new ArrayList<>();
|
||||
for (WorktreeHostileConfig cfg : WORKTREE_HOSTILE_CONFIGS) {
|
||||
neutralize(root, worktreePath, cfg);
|
||||
if (neutralize(root, worktreePath, cfg)) {
|
||||
neutralized.add(cfg.file());
|
||||
} else {
|
||||
skipped.add(cfg.file() + " absent");
|
||||
}
|
||||
}
|
||||
String detail = neutralized.isEmpty() ? String.join(", ", skipped)
|
||||
: skipped.isEmpty() ? String.join(", ", neutralized)
|
||||
: String.join(", ", neutralized) + " (" + String.join(", ", skipped) + ")";
|
||||
log.info("tool-surface isolation: neutralized {} of {} configs: {} — the worktree copy is a "
|
||||
+ "stub, not the repo's file; edit the real file in the primary checkout instead",
|
||||
neutralized.size(), WORKTREE_HOSTILE_CONFIGS.size(), detail);
|
||||
recordNeutralizedConfigForWorker(worktreePath, neutralized);
|
||||
}
|
||||
|
||||
private void neutralize(Path root, String worktreePath, WorktreeHostileConfig cfg) {
|
||||
/**
|
||||
* The worker-readable half of fleetd #134: record which files were neutralized where the worker
|
||||
* itself can read it, without a working-tree file that would show up in {@code git status}.
|
||||
* Worktree-scoped git config is per-worktree, lives under {@code .git/worktrees/<nonce>/} rather
|
||||
* than the working tree, and this repo already relies on the same mechanism (and the same
|
||||
* {@code extensions.worktreeConfig} enablement) for the credential helper and the SSH→HTTPS
|
||||
* rewrite — see {@link #configureEnvironmentCredentialHelper}.
|
||||
*/
|
||||
private void recordNeutralizedConfigForWorker(String worktreePath, List<String> neutralized) {
|
||||
if (neutralized.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
exec("git", "-C", worktreePath, "config", "extensions.worktreeConfig", "true");
|
||||
for (String file : neutralized) {
|
||||
exec("git", "-C", worktreePath, "config", "--worktree", "--add", "fleet.neutralizedConfig", file);
|
||||
}
|
||||
exec("git", "-C", worktreePath, "config", "--worktree", "fleet.neutralizedConfigNote",
|
||||
"the worktree copy of each fleet.neutralizedConfig path is a stub, not the repo's "
|
||||
+ "committed file; edit the real file from the primary checkout instead");
|
||||
}
|
||||
|
||||
/** @return true if {@code cfg} was neutralized (present, or created because {@link
|
||||
* WorktreeHostileConfig#createIfAbsent()}); false if the repo does not carry it and it was
|
||||
* left alone. */
|
||||
private boolean neutralize(Path root, String worktreePath, WorktreeHostileConfig cfg) {
|
||||
Path target = root.resolve(cfg.file());
|
||||
if (!Files.exists(target) && !cfg.createIfAbsent()) {
|
||||
log.debug("{} absent in the worktree — skipping (repo does not carry it)", cfg.file());
|
||||
return;
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
Files.writeString(target, cfg.stub());
|
||||
@@ -449,7 +505,7 @@ public final class GitWorktrees implements Worktrees {
|
||||
if (isTracked(root, cfg.file())) {
|
||||
exec("git", "-C", worktreePath, "update-index", "--skip-worktree", cfg.file());
|
||||
}
|
||||
log.debug("neutralized {} — worker tool surface is launcher-mounted only", cfg.file());
|
||||
return true;
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@@ -666,6 +666,103 @@ class GitWorktreesTest {
|
||||
assertEquals("", status(Path.of(wt), ".autoenv"), ".autoenv still shows as modified");
|
||||
}
|
||||
|
||||
// ---- fleetd #134: isolateToolSurface must report what it neutralized (to the daemon operator's
|
||||
// log) and record it where the worker itself can read it (worktree-scoped git config), without
|
||||
// ever showing up in the worker's own `git status`. All drive the real provisioning path,
|
||||
// GitWorktrees#add, per criterion 5 — the whole provisioned worktree is what's under test here. ----
|
||||
|
||||
private static Path initRepoWithAllThreeConfigs(Path dir) throws Exception {
|
||||
Files.createDirectories(dir);
|
||||
git(dir, "init", "-q", "-b", "main");
|
||||
git(dir, "config", "user.email", "test@example.invalid");
|
||||
git(dir, "config", "user.name", "Test");
|
||||
Files.writeString(dir.resolve(".mcp.json"), WITH_SERVERS);
|
||||
Files.writeString(dir.resolve("opencode.json"), OPENCODE_WITH_FILE_REF);
|
||||
Files.writeString(dir.resolve(".autoenv"), AUTOENV_WITH_DIRECTIVE);
|
||||
Files.writeString(dir.resolve("README.md"), "seed\n");
|
||||
git(dir, "add", ".mcp.json", "opencode.json", ".autoenv", "README.md");
|
||||
git(dir, "commit", "-q", "-m", "seed");
|
||||
return dir;
|
||||
}
|
||||
|
||||
/** Criterion 1, all three present: the summary names the denominator and every neutralized file. */
|
||||
@Test
|
||||
void isolateToolSurfaceLogsAllThreeConfigsNeutralized(@TempDir Path tmp) throws Exception {
|
||||
reportingLogger.setLevel(Level.INFO);
|
||||
Path repo = initRepoWithAllThreeConfigs(tmp.resolve("repo"));
|
||||
|
||||
new GitWorktrees(tmp.resolve("wts").toString()).add(repo.toString(), "cb-134-log-all", "HEAD");
|
||||
|
||||
assertTrue(capturedMessages().contains(
|
||||
"tool-surface isolation: neutralized 3 of 3 configs: .mcp.json, opencode.json, "
|
||||
+ ".autoenv — the worktree copy is a stub, not the repo's file; edit the "
|
||||
+ "real file in the primary checkout instead"),
|
||||
"expected the all-neutralized summary line, got:\n" + capturedMessages());
|
||||
}
|
||||
|
||||
/** Criterion 1, two absent: the summary must still name the denominator and say why. */
|
||||
@Test
|
||||
void isolateToolSurfaceLogsAbsentConfigsWithReason(@TempDir Path tmp) throws Exception {
|
||||
reportingLogger.setLevel(Level.INFO);
|
||||
Path repo = initRepo(tmp.resolve("repo")); // only .mcp.json + README committed
|
||||
|
||||
new GitWorktrees(tmp.resolve("wts").toString()).add(repo.toString(), "cb-134-log-partial", "HEAD");
|
||||
|
||||
assertTrue(capturedMessages().contains(
|
||||
"tool-surface isolation: neutralized 1 of 3 configs: .mcp.json (opencode.json "
|
||||
+ "absent, .autoenv absent) — the worktree copy is a stub, not the repo's "
|
||||
+ "file; edit the real file in the primary checkout instead"),
|
||||
"expected the partial summary line, got:\n" + capturedMessages());
|
||||
}
|
||||
|
||||
/**
|
||||
* Criterion 2. The daemon's own log is invisible to the worker process — it never reads fleetd's
|
||||
* stdout. This is the mechanism the worker itself can query, from inside its own worktree, to
|
||||
* learn "this file is neutralized here, the repo's real file differs" instead of trusting what it
|
||||
* just read on disk.
|
||||
*/
|
||||
@Test
|
||||
void aWorkerCanDiscoverNeutralizedConfigsFromWorktreeScopedGitConfig(@TempDir Path tmp) throws Exception {
|
||||
Path repo = initRepoWithAllThreeConfigs(tmp.resolve("repo"));
|
||||
|
||||
String wt = new GitWorktrees(tmp.resolve("wts").toString())
|
||||
.add(repo.toString(), "cb-134-discover", "HEAD");
|
||||
|
||||
String recorded = gitOutput(Path.of(wt), "config", "--worktree", "--get-all", "fleet.neutralizedConfig");
|
||||
Set<String> files = new HashSet<>();
|
||||
for (String line : recorded.split("\\R")) {
|
||||
if (!line.isBlank()) {
|
||||
files.add(line.trim());
|
||||
}
|
||||
}
|
||||
assertEquals(Set.of(".mcp.json", "opencode.json", ".autoenv"), files,
|
||||
"the worker-readable record must name every neutralized file: " + recorded);
|
||||
|
||||
String note = gitOutput(Path.of(wt), "config", "--worktree", "--get", "fleet.neutralizedConfigNote").trim();
|
||||
assertTrue(note.contains("stub"), "note must say the worktree copy is a stub: " + note);
|
||||
assertTrue(note.contains("primary checkout"),
|
||||
"note must state the consequence — where to edit the real file instead: " + note);
|
||||
}
|
||||
|
||||
/**
|
||||
* Criterion 3. Whatever fleetd#134's worker-discovery mechanism writes must never appear as
|
||||
* untracked or modified in the worker's own `git status` — a worker that sees a stray file either
|
||||
* commits it by mistake or burns a turn asking about it. This runs the full porcelain status, not
|
||||
* a single-file check, so any leftover file anywhere in the worktree would fail it.
|
||||
*/
|
||||
@Test
|
||||
void aProvisionedWorktreeHasCleanGitStatusDespiteNeutralizedConfigRecordkeeping(@TempDir Path tmp)
|
||||
throws Exception {
|
||||
Path repo = initRepoWithAllThreeConfigs(tmp.resolve("repo"));
|
||||
|
||||
String wt = new GitWorktrees(tmp.resolve("wts").toString())
|
||||
.add(repo.toString(), "cb-134-clean-status", "HEAD");
|
||||
|
||||
assertEquals("", fullStatus(Path.of(wt)),
|
||||
"a freshly provisioned worktree must show a clean `git status --porcelain`, including "
|
||||
+ "after the worker-readable neutralized-config record was written");
|
||||
}
|
||||
|
||||
/**
|
||||
* CB-578 stage C, acceptance criterion 1. A dirty worktree — a tracked edit plus a brand-new
|
||||
* untracked file, exactly the shape lost in CB-576 — must land in {@code refs/wip/<branch>}'s
|
||||
|
||||
Reference in New Issue
Block a user