mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 14:06:50 +01:00
E4: fingerprint under a read-only .git via a private object dir owned by review-log
Codex's sandbox mounts .git read-only, so gstack-wtree's git add -A could not write blobs and /review lost its start capture. gstack-review-log now creates a private 0700 object directory under TMPDIR, passes it to gstack-wtree (which writes new objects there with the common-dir object store, resolved before exporting, as an alternate, so linked worktrees work), exposes it to the bindReview/checkSharedLibsReuse consumers that read the tree and blobs, and removes it on EXIT/INT/TERM after they finish. gstack-wtree prints one stderr line and exits 1 on a failed write or unwritable TMPDIR instead of printing a partial hash; review-log no longer swallows that line. Standalone gstack-wtree keeps writing to the repository store so git diff between saved snapshots (/ship) still works.
This commit is contained in:
1 parent
c58cd80317
commit
337d34fe16
3 files changed
+170
-10
No files matched your search
+16
-1
@@ -40,7 +40,22 @@ WTREE=""
|
||||
DIRTY=""
|
||||
if [ -n "$COMMIT_FULL" ]; then
|
||||
TREE=$(git rev-parse 'HEAD^{tree}' 2>/dev/null || true)
|
||||
WTREE=$("$SCRIPT_DIR/gstack-wtree" 2>/dev/null || true)
|
||||
# E4: the fingerprint's new objects go to a private 0700 directory this
|
||||
# operation owns (a read-only .git still fingerprints), and stay readable
|
||||
# until the consumers below (bindReview / checkSharedLibsReuse read the tree
|
||||
# and blobs) finish; removed on EXIT/INT/TERM. gstack-wtree's one-line
|
||||
# diagnostic reaches stderr; stdout stays the hash.
|
||||
if WTREE_OBJECTS=$(mktemp -d "${TMPDIR:-/tmp}/gstack-wtree-objects-XXXXXX" 2>/dev/null); then
|
||||
case "$(uname -s)" in
|
||||
MINGW*|MSYS*|CYGWIN*) command -v cygpath >/dev/null 2>&1 && WTREE_OBJECTS="$(cygpath -m "$WTREE_OBJECTS")" ;;
|
||||
esac
|
||||
trap 'rm -rf "$WTREE_OBJECTS"' EXIT
|
||||
trap 'exit 130' INT TERM
|
||||
WTREE=$(GSTACK_WTREE_OBJECT_DIR="$WTREE_OBJECTS" "$SCRIPT_DIR/gstack-wtree" || true)
|
||||
export GIT_ALTERNATE_OBJECT_DIRECTORIES="$WTREE_OBJECTS${GIT_ALTERNATE_OBJECT_DIRECTORIES:+:$GIT_ALTERNATE_OBJECT_DIRECTORIES}"
|
||||
else
|
||||
echo "gstack-review-log: cannot create a private object directory under ${TMPDIR:-/tmp}; no working-tree fingerprint. Set TMPDIR to a writable directory." >&2
|
||||
fi
|
||||
if [ -n "$(git status --porcelain -uno 2>/dev/null | head -1)" ]; then
|
||||
DIRTY="true"
|
||||
else
|
||||
|
||||
+40
-7
@@ -21,11 +21,17 @@
|
||||
# produce the identical write-tree hash. Fallback: `read-tree HEAD` when the
|
||||
# index copy is unavailable (fresh repo, exotic index).
|
||||
#
|
||||
# The real repo index is never touched. Staged blobs land in the object store
|
||||
# as unreachable objects and get gc'd like stash churn (note: this means the
|
||||
# The real repo index is never touched. Run standalone, staged blobs land in
|
||||
# the object store as unreachable objects and get gc'd like stash churn (the
|
||||
# CONTENT of untracked, non-ignored files enters .git/objects until gc — the
|
||||
# same property `git stash -u` has). Exit 1 outside a git repo or in a repo
|
||||
# with no commits — callers treat that as "no fingerprint".
|
||||
# same property `git stash -u` has), so `git diff <tree> <tree>` between saved
|
||||
# snapshots works. With GSTACK_WTREE_OBJECT_DIR set (gstack-review-log, E4),
|
||||
# new objects go to that private directory instead, with the repository's
|
||||
# object store as an alternate: the fingerprint works under a read-only .git
|
||||
# (Codex's sandbox), and the caller owns the directory's lifetime because its
|
||||
# consumers read the tree afterwards. Exit 1 outside a git repo, in a repo with
|
||||
# no commits, or when a write fails (one stderr line names the cause) — callers
|
||||
# treat that as "no fingerprint"; a partial hash is never printed.
|
||||
set -euo pipefail
|
||||
|
||||
TOP=$(git rev-parse --show-toplevel 2>/dev/null) || exit 1
|
||||
@@ -33,14 +39,34 @@ TOP=$(git rev-parse --show-toplevel 2>/dev/null) || exit 1
|
||||
# var set, `git rev-parse --git-path index` returns the temp index itself and
|
||||
# the stat-cache seed silently self-copies into a dead fast path.
|
||||
REAL_INDEX=$(git -C "$TOP" rev-parse --git-path index 2>/dev/null || true)
|
||||
TMPIDX=$(mktemp "${TMPDIR:-/tmp}/gstack-wtree-XXXXXX")
|
||||
# Same for the object store: the common dir's objects (linked worktrees share
|
||||
# it), resolved to an absolute path before GIT_OBJECT_DIRECTORY is exported.
|
||||
REAL_OBJECTS=$(git -C "$TOP" rev-parse --git-path objects 2>/dev/null || true)
|
||||
OBJ_DIR="${GSTACK_WTREE_OBJECT_DIR:-}"
|
||||
if [ -n "$OBJ_DIR" ] && [ ! -d "$OBJ_DIR" ]; then
|
||||
echo "gstack-wtree: private object directory $OBJ_DIR does not exist" >&2
|
||||
exit 1
|
||||
fi
|
||||
TMPIDX=$(mktemp "${TMPDIR:-/tmp}/gstack-wtree-XXXXXX" 2>/dev/null) || {
|
||||
echo "gstack-wtree: cannot create a temp index under ${TMPDIR:-/tmp}; set TMPDIR to a writable directory" >&2
|
||||
exit 1
|
||||
}
|
||||
trap 'rm -f "$TMPIDX"' EXIT
|
||||
trap 'exit 130' INT TERM
|
||||
export GIT_INDEX_FILE="$TMPIDX"
|
||||
# Resolve relative --git-path output against the repo root.
|
||||
case "$REAL_INDEX" in
|
||||
""|/*) ;;
|
||||
*) REAL_INDEX="$TOP/$REAL_INDEX" ;;
|
||||
esac
|
||||
case "$REAL_OBJECTS" in
|
||||
""|/*) ;;
|
||||
*) REAL_OBJECTS="$TOP/$REAL_OBJECTS" ;;
|
||||
esac
|
||||
if [ -n "$OBJ_DIR" ]; then
|
||||
export GIT_OBJECT_DIRECTORY="$OBJ_DIR"
|
||||
export GIT_ALTERNATE_OBJECT_DIRECTORIES="$REAL_OBJECTS${GIT_ALTERNATE_OBJECT_DIRECTORIES:+:$GIT_ALTERNATE_OBJECT_DIRECTORIES}"
|
||||
fi
|
||||
if [ -n "$REAL_INDEX" ] && [ -f "$REAL_INDEX" ] && cp "$REAL_INDEX" "$TMPIDX" 2>/dev/null; then
|
||||
# Carry the real index's mtime onto the copy. Git's racy-git protection
|
||||
# re-hashes any entry whose cached mtime is not older than the index file
|
||||
@@ -58,5 +84,12 @@ if [ -n "$REAL_INDEX" ] && [ -f "$REAL_INDEX" ] && cp "$REAL_INDEX" "$TMPIDX" 2>
|
||||
else
|
||||
git -C "$TOP" read-tree HEAD 2>/dev/null || exit 1
|
||||
fi
|
||||
git -C "$TOP" add -A 2>/dev/null || exit 1
|
||||
git -C "$TOP" write-tree 2>/dev/null
|
||||
if ! ERR=$(git -C "$TOP" add -A 2>&1 >/dev/null); then
|
||||
echo "gstack-wtree: cannot stage the working tree: $(printf '%s' "$ERR" | head -1)" >&2
|
||||
exit 1
|
||||
fi
|
||||
if ! TREE=$(git -C "$TOP" write-tree 2>&1) || ! printf '%s' "$TREE" | grep -Eqx '[0-9a-f]{40}|[0-9a-f]{64}'; then
|
||||
echo "gstack-wtree: cannot write the tree: $(printf '%s' "$TREE" | head -1)" >&2
|
||||
exit 1
|
||||
fi
|
||||
printf '%s\n' "$TREE"
|
||||
@@ -1,6 +1,6 @@
|
||||
import { afterEach, beforeEach, describe, expect, test } from 'bun:test';
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { execFileSync, spawnSync } from 'node:child_process';
|
||||
import { chmodSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join, resolve } from 'node:path';
|
||||
import { findFilesBySuffix, gitArgvIn } from './helpers/scratch-repo';
|
||||
@@ -241,3 +241,115 @@ describe('review start/end binding (#2803)', () => {
|
||||
expect(row.review_freshness.reason).toContain('telemetry');
|
||||
});
|
||||
});
|
||||
|
||||
// E4: Codex's sandbox mounts .git read-only, so `git add -A` could not write
|
||||
// the fingerprint's blobs and /review lost its start capture. gstack-wtree now
|
||||
// writes new objects to a private directory owned by the review-log operation,
|
||||
// which keeps it until bindReview/checkSharedLibsReuse have read the tree.
|
||||
const asRoot = typeof process.getuid === 'function' && process.getuid() === 0;
|
||||
describe.skipIf(process.platform === 'win32' || asRoot)('review fingerprint under a read-only .git (E4)', () => {
|
||||
let scratch: string;
|
||||
beforeEach(() => { scratch = mkdtempSync(join(tmpdir(), 'review-ro-tmp-')); });
|
||||
afterEach(() => {
|
||||
spawnSync('chmod', ['-R', 'u+w', join(repo, '.git')], { timeout: 10_000 });
|
||||
rmSync(scratch, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const run = (name: string, args: string[], cwd: string, opts: { input?: string; env?: Record<string, string> } = {}) =>
|
||||
spawnSync(join(ROOT, 'bin', name), args, {
|
||||
cwd, input: opts.input, encoding: 'utf8', timeout: 20_000,
|
||||
env: { ...process.env, GSTACK_HOME: home, TMPDIR: scratch, ...opts.env },
|
||||
});
|
||||
const finding = {
|
||||
advisory: true, severity: 'INFORMATIONAL', action: 'skipped',
|
||||
evidence_paths: ['source.ts', 'untracked.ts'], helper_target: { path: 'lib/shared.ts', symbol: 'readValue' },
|
||||
};
|
||||
const readOnlyGit = () => {
|
||||
const r = spawnSync('chmod', ['-R', 'a-w', join(repo, '.git')], { timeout: 10_000 });
|
||||
expect(r.status).toBe(0);
|
||||
};
|
||||
|
||||
for (const where of ['main checkout', 'linked worktree']) {
|
||||
test(`--start -> --finish -> shared-libs reuse with dirty and untracked files (${where})`, () => {
|
||||
let cwd = repo;
|
||||
if (where === 'linked worktree') {
|
||||
cwd = join(scratch, 'linked wt');
|
||||
git('worktree', 'add', '-q', '-b', 'linked', cwd);
|
||||
}
|
||||
writeFileSync(join(cwd, 'source.ts'), 'export const value = 42;\n');
|
||||
writeFileSync(join(cwd, 'untracked.ts'), 'export const fresh = true;\n');
|
||||
readOnlyGit();
|
||||
const before = spawnSync('find', [join(repo, '.git', 'objects'), '-type', 'f'], { encoding: 'utf8', timeout: 10_000 }).stdout;
|
||||
|
||||
const start = run('gstack-review-log', ['--start', 'review'], cwd);
|
||||
expect(start.stderr).toBe('');
|
||||
expect(start.status).toBe(0);
|
||||
const token = start.stdout.trim();
|
||||
expect(token).toMatch(/^[0-9a-f-]{36}$/);
|
||||
const record = { skill: 'review', status: 'clean', timestamp: new Date().toISOString(), completed: true, converged: true, findings: [finding] };
|
||||
const finish = run('gstack-review-log', [JSON.stringify(record), '--finish', token], cwd);
|
||||
expect(finish.status).toBe(0);
|
||||
const logged = JSON.parse(run('gstack-review-read', [], cwd).stdout.split('---CONFIG---')[0].trim().split('\n').at(-1)!);
|
||||
expect(logged.review_binding.state).toBe('verified');
|
||||
expect(logged.wtree).toMatch(/^[0-9a-f]{40}$/);
|
||||
// The consumer read the private tree and blobs before cleanup.
|
||||
expect(logged.findings[0].snapshot_covered_paths).toEqual(['source.ts', 'untracked.ts']);
|
||||
|
||||
const again = run('gstack-review-log', ['--start', 'review'], cwd).stdout.trim();
|
||||
const reuse = run('gstack-review-log', ['--check-shared-libs', again], cwd, { input: JSON.stringify(finding) });
|
||||
expect(reuse.status).toBe(0);
|
||||
expect(JSON.parse(reuse.stdout).reusable).toBe(true);
|
||||
|
||||
expect(spawnSync('find', [join(repo, '.git', 'objects'), '-type', 'f'], { encoding: 'utf8', timeout: 10_000 }).stdout).toBe(before);
|
||||
expect(readdirSync(scratch).filter((name) => name.startsWith('gstack-wtree'))).toEqual([]);
|
||||
|
||||
// Same fingerprint as a writable .git.
|
||||
spawnSync('chmod', ['-R', 'u+w', join(repo, '.git')], { timeout: 10_000 });
|
||||
expect(run('gstack-wtree', [], cwd).stdout.trim()).toBe(logged.wtree);
|
||||
}, 60_000);
|
||||
}
|
||||
|
||||
test('the private directory is 0700 and alive while the consumer runs, then removed', () => {
|
||||
writeFileSync(join(repo, 'untracked.ts'), 'export const fresh = true;\n');
|
||||
readOnlyGit();
|
||||
const shim = join(scratch, 'shim');
|
||||
const probe = join(scratch, 'probe.log');
|
||||
mkdirSync(shim);
|
||||
writeFileSync(join(shim, 'bun'), [
|
||||
'#!/usr/bin/env bash',
|
||||
'first="${GIT_ALTERNATE_OBJECT_DIRECTORIES%%:*}"',
|
||||
`[ -n "$first" ] && ls -ld "$first" | cut -c1-10 >> '${probe}'`,
|
||||
`exec '${process.execPath}' "$@"`,
|
||||
].join('\n') + '\n', { mode: 0o755 });
|
||||
const env = { PATH: `${shim}:${process.env.PATH}` };
|
||||
const token = run('gstack-review-log', ['--start', 'review'], repo, { env }).stdout.trim();
|
||||
const record = { skill: 'review', status: 'clean', timestamp: new Date().toISOString(), completed: true, converged: true };
|
||||
expect(run('gstack-review-log', [JSON.stringify(record), '--finish', token], repo, { env }).status).toBe(0);
|
||||
expect(readFileSync(probe, 'utf8').trim().split('\n')).toEqual(['drwx------', 'drwx------']);
|
||||
expect(readdirSync(scratch).filter((name) => name.startsWith('gstack-wtree'))).toEqual([]);
|
||||
}, 60_000);
|
||||
|
||||
test('a failed object write exits 1 with one stderr line and no partial hash', () => {
|
||||
writeFileSync(join(repo, 'untracked.ts'), 'export const fresh = true;\n');
|
||||
const objects = join(scratch, 'objects');
|
||||
mkdirSync(objects, { mode: 0o500 });
|
||||
const r = run('gstack-wtree', [], repo, { env: { GSTACK_WTREE_OBJECT_DIR: objects } });
|
||||
chmodSync(objects, 0o700);
|
||||
expect(r.status).toBe(1);
|
||||
expect(r.stdout).toBe('');
|
||||
expect(r.stderr.trim().split('\n')).toHaveLength(1);
|
||||
expect(r.stderr).toStartWith('gstack-wtree: cannot stage the working tree: ');
|
||||
});
|
||||
|
||||
test('an unwritable TMPDIR is named, and review-log reports it on stderr while stdout stays clean', () => {
|
||||
const missing = join(scratch, 'no such dir');
|
||||
const wtree = run('gstack-wtree', [], repo, { env: { TMPDIR: missing } });
|
||||
expect(wtree.status).toBe(1);
|
||||
expect(wtree.stdout).toBe('');
|
||||
expect(wtree.stderr.trim()).toBe(`gstack-wtree: cannot create a temp index under ${missing}; set TMPDIR to a writable directory`);
|
||||
const start = run('gstack-review-log', ['--start', 'review'], repo, { env: { TMPDIR: missing } });
|
||||
expect(start.status).not.toBe(0);
|
||||
expect(start.stdout).toBe('');
|
||||
expect(start.stderr).toContain(`gstack-review-log: cannot create a private object directory under ${missing}; no working-tree fingerprint.`);
|
||||
}, 30_000);
|
||||
});
|
||||
Reference in new issue
Block a user