fix: stage declared untracked task files

This commit is contained in:
Mikael Hugo 2026-04-29 20:15:35 +02:00
parent 9398c7000d
commit db41f92812
3 changed files with 108 additions and 6 deletions

View file

@ -577,7 +577,7 @@ export async function postUnitPreVerification(
// git history object is only created once the unit is confirmed complete.
try {
const git = createGitService(s.basePath);
const staged = git.stageOnly();
const staged = git.stageOnly([], taskContext?.keyFiles ?? []);
if (staged) {
s.stagedPendingCommit = true;
s.pendingCommitTaskContext = taskContext ?? null;

View file

@ -16,7 +16,7 @@ import {
readFileSync,
writeFileSync,
} from "node:fs";
import { join } from "node:path";
import { isAbsolute, join, normalize } from "node:path";
import {
QUICK_BRANCH_RE,
SLICE_BRANCH_RE,
@ -24,10 +24,12 @@ import {
} from "./branch-patterns.js";
import { getErrorMessage } from "./error-utils.js";
import { SF_GIT_ERROR, SF_MERGE_CONFLICT, SFError } from "./errors.js";
import { normalizePlannedFileReference } from "./files.js";
import { GIT_NO_PROMPT_ENV } from "./git-constants.js";
import {
_resetHasChangesCache,
nativeAddAllWithExclusions,
nativeAddPaths,
nativeBranchExists,
nativeCommit,
nativeCommitSubject,
@ -247,6 +249,40 @@ export const RUNTIME_EXCLUSION_PATHS: readonly string[] = [
".sf/DISCUSSION-MANIFEST.json",
];
function isPathExcluded(path: string, exclusions: readonly string[]): boolean {
const normalized = path.replace(/\\/g, "/").replace(/^\.\//, "");
return exclusions.some((rawExclusion) => {
const exclusion = rawExclusion.replace(/\\/g, "/").replace(/^\.\//, "");
if (!exclusion) return false;
if (exclusion.includes("*")) {
const prefix = exclusion.slice(0, exclusion.indexOf("*"));
return normalized.startsWith(prefix);
}
if (exclusion.endsWith("/")) return normalized.startsWith(exclusion);
return normalized === exclusion || normalized.startsWith(`${exclusion}/`);
});
}
function normalizeExplicitStagePath(path: string): string | null {
const normalized = normalize(
normalizePlannedFileReference(path).replace(/\\/g, "/"),
)
.replace(/\\/g, "/")
.replace(/^\.\//, "");
if (
!normalized ||
normalized === "." ||
normalized.includes("\0") ||
isAbsolute(normalized) ||
/^[A-Za-z]:\//.test(normalized) ||
normalized === ".." ||
normalized.startsWith("../")
) {
return null;
}
return normalized;
}
// ─── Integration Branch Metadata ───────────────────────────────────────────
/**
@ -541,7 +577,10 @@ export class GitServiceImpl {
* Falls back to plain `git add -A` if the exclusion pathspec fails.
* @param extraExclusions Additional pathspec exclusions beyond RUNTIME_EXCLUSION_PATHS.
*/
private smartStage(extraExclusions: readonly string[] = []): void {
private smartStage(
extraExclusions: readonly string[] = [],
explicitIncludePaths: readonly string[] = [],
): void {
// One-time cleanup: if runtime files are already tracked in the index
// (from older versions where the fallback bug staged them), untrack them
// in a dedicated commit. This must happen as a separate commit because
@ -607,6 +646,25 @@ export class GitServiceImpl {
}
nativeAddAllWithExclusions(this.basePath, allExclusions);
this.stageExplicitIncludePaths(explicitIncludePaths, allExclusions);
}
private stageExplicitIncludePaths(
paths: readonly string[],
exclusions: readonly string[],
): void {
const seen = new Set<string>();
const safePaths = paths
.map(normalizeExplicitStagePath)
.filter((path): path is string => path !== null)
.filter((path) => !isPathExcluded(path, exclusions))
.filter((path) => {
if (seen.has(path)) return false;
seen.add(path);
return true;
});
if (safePaths.length === 0) return;
nativeAddPaths(this.basePath, safePaths);
}
/** Tracks whether runtime file cleanup has run this session. */
@ -650,7 +708,7 @@ export class GitServiceImpl {
// Native path uses libgit2 (single syscall), fallback spawns git.
if (!nativeHasChanges(this.basePath)) return null;
this.smartStage(extraExclusions);
this.smartStage(extraExclusions, taskContext?.keyFiles ?? []);
// After smart staging, check if anything was actually staged
// (all changes might have been runtime files that got excluded)
@ -850,10 +908,15 @@ export class GitServiceImpl {
* postUnitPostVerification once verification has passed.
*
* @param extraExclusions Additional paths to exclude from staging.
* @param explicitIncludePaths Task-declared files to stage even when the
* symlinked .sf fallback must avoid broad untracked traversal.
*/
stageOnly(extraExclusions: readonly string[] = []): boolean {
stageOnly(
extraExclusions: readonly string[] = [],
explicitIncludePaths: readonly string[] = [],
): boolean {
if (!nativeHasChanges(this.basePath)) return false;
this.smartStage(extraExclusions);
this.smartStage(extraExclusions, explicitIncludePaths);
return nativeHasStagedChanges(this.basePath);
}

View file

@ -1740,6 +1740,45 @@ describe("git-service", async () => {
rmSync(externalGsd, { recursive: true, force: true });
});
test("GitServiceImpl: symlinked .sf stages explicit untracked task files", () => {
const repo = initTempRepo();
const externalGsd = mkdtempSync(join(tmpdir(), "sf-external-"));
mkdirSync(join(externalGsd, "activity"), { recursive: true });
symlinkSync(externalGsd, join(repo, ".sf"));
writeFileSync(join(repo, ".gitignore"), ".sf\n");
createFile(repo, "cmd/installer/main.go", "package main\n");
run("git add -A", repo);
run('git commit -m "add installer"', repo);
writeFileSync(
join(repo, "cmd/installer/main.go"),
"package main\n// edit\n",
);
createFile(repo, "cmd/installer/main_test.go", "package main\n");
createFile(repo, "data/large-model.bin", "pretend this is 10GB");
const svc = new GitServiceImpl(repo);
assert.equal(
svc.stageOnly([], ["cmd/installer/main_test.go"]),
true,
"explicit task output should make stageOnly see staged changes",
);
const staged = run("git diff --cached --name-only", repo);
assert.ok(staged.includes("cmd/installer/main.go"));
assert.ok(
staged.includes("cmd/installer/main_test.go"),
"explicit untracked task file should be staged despite symlink fallback",
);
assert.ok(
!staged.includes("data/large-model.bin"),
"unlisted untracked files stay unstaged",
);
rmSync(repo, { recursive: true, force: true });
rmSync(externalGsd, { recursive: true, force: true });
});
// ─── nativeAddAllWithExclusions: non-symlinked .sf still works ───────
test("nativeAddAllWithExclusions: non-symlinked .sf still works", () => {