fleetd #134: make tool-surface neutralization visible to the daemon and the worker
isolateToolSurface replaces .mcp.json, opencode.json and .autoenv with stubs in every provisioned worktree and marks them --skip-worktree. That neutralisation is correct and is unchanged here — the committed files would mount the primary's credentials. The problem was that it was invisible. A worker told to edit opencode.json read a 3-byte stub and reported, truthfully and wrongly, that the mount key did not exist. A missing file would have prompted a question; a plausible stub did not. Two changes, both visibility only. The daemon now logs one info summary per provisioning with the denominator, the files neutralised, and the consequence. And the list is recorded in worktree-scoped git config (fleet.neutralizedConfig / fleet.neutralizedConfigNote) so a worker can discover it from inside its own worktree with 'git config --worktree --get-all fleet.neutralizedConfig'. Worktree-scoped config was chosen over a file in the working tree because it lives in .git/worktrees/<nonce>/config.worktree and so can never appear in git status, and because configureEnvironmentCredentialHelper already uses the same mechanism in the same add() call. Verified by the lead: baseline 1172 tests, 0 failures. Reverting the summary to log.debug goes red (2 tests), and recording into --local rather than --worktree — which would leak the record into the shared repo config — goes red too. Both with 0 compile errors.
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
|
* 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
|
* 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.
|
* 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) {
|
private void isolateToolSurface(String worktreePath) {
|
||||||
Path root = Path.of(worktreePath).toAbsolutePath().normalize();
|
Path root = Path.of(worktreePath).toAbsolutePath().normalize();
|
||||||
|
List<String> neutralized = new ArrayList<>();
|
||||||
|
List<String> skipped = new ArrayList<>();
|
||||||
for (WorktreeHostileConfig cfg : WORKTREE_HOSTILE_CONFIGS) {
|
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());
|
Path target = root.resolve(cfg.file());
|
||||||
if (!Files.exists(target) && !cfg.createIfAbsent()) {
|
if (!Files.exists(target) && !cfg.createIfAbsent()) {
|
||||||
log.debug("{} absent in the worktree — skipping (repo does not carry it)", cfg.file());
|
return false;
|
||||||
return;
|
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
Files.writeString(target, cfg.stub());
|
Files.writeString(target, cfg.stub());
|
||||||
@@ -449,7 +505,7 @@ public final class GitWorktrees implements Worktrees {
|
|||||||
if (isTracked(root, cfg.file())) {
|
if (isTracked(root, cfg.file())) {
|
||||||
exec("git", "-C", worktreePath, "update-index", "--skip-worktree", 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
|
@Override
|
||||||
|
|||||||
@@ -666,6 +666,103 @@ class GitWorktreesTest {
|
|||||||
assertEquals("", status(Path.of(wt), ".autoenv"), ".autoenv still shows as modified");
|
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
|
* 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
|
* untracked file, exactly the shape lost in CB-576 — must land in {@code refs/wip/<branch>}'s
|
||||||
|
|||||||
Reference in New Issue
Block a user