mirror of https://github.com/garrytan/gstack.git
fix(brain-allowlist): sync project-root eng-review-test-plan artifacts (#1452)
Cherry-picked from #1465 by genisis0x and extended with the v1.40.0.0 upgrade migration that codex review #5 surfaced. #1465 alone only patches bin/gstack-artifacts-init, which means fresh installs and re-inits pick up the new pattern. But existing users who already ran v1.38.1.0 have a `.migrations/v1.38.1.0.done` marker — that migration won't re-run no matter what we change. So their installed `.brain-allowlist`, `.brain-privacy-map.json`, and `.gitattributes` stay without the new pattern, and `/plan-eng-review` artifacts continue to silently drop out of their federation queue. This commit: - bin/gstack-artifacts-init: adds projects/*/*-eng-review-test-plan-*.md to the three managed blocks. v1.38.1.0 covered design + test-plan; this completes the set for /plan-eng-review. - gstack-upgrade/migrations/v1.40.0.0.sh: targeted in-place repair for existing installs. Same idempotent jq-based shape as v1.38.1.0. Adds the new pattern to .brain-allowlist (before the USER ADDITIONS marker), .brain-privacy-map.json (as class=artifact), and .gitattributes (as merge=union). NEVER commits + pushes — the user controls when the patches ship to their federated artifacts repo. - test/artifacts-init-migration.test.ts: 5 new tests covering the v1.40.0.0 migration applied on top of a post-v1.38.1.0 state, jq patching, gitattributes append, idempotent re-run, and done-marker write when files are missing entirely. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
genisis0x
Garry Tan
parent
62ff90cfc9
commit
d0463e59ed
|
|
@ -227,8 +227,18 @@ projects/*/ceo-plans/*.md
|
|||
projects/*/ceo-plans/*/*.md
|
||||
projects/*/designs/*.md
|
||||
projects/*/designs/*/*.md
|
||||
# Project-root design / test-plan artifacts written by /office-hours,
|
||||
# /plan-eng-review, and /autoplan. The skills emit
|
||||
# `{user}-{branch}-design-{datetime}.md`,
|
||||
# `{user}-{branch}-test-plan-{datetime}.md`, and
|
||||
# `{user}-{branch}-eng-review-test-plan-{datetime}.md` at the project
|
||||
# root (not under designs/), so the existing `designs/*.md` patterns
|
||||
# miss them. Without these the cross-machine pull on machine B gets
|
||||
# the referencing CEO plan but not the underlying design / test plan
|
||||
# (#1452).
|
||||
projects/*/*-design-*.md
|
||||
projects/*/*-test-plan-*.md
|
||||
projects/*/*-eng-review-test-plan-*.md
|
||||
projects/*/timeline.jsonl
|
||||
retros/*.md
|
||||
developer-profile.json
|
||||
|
|
@ -256,6 +266,7 @@ cat > "$GSTACK_HOME/.brain-privacy-map.json" <<'EOF'
|
|||
{"pattern": "projects/*/designs/*/*.md", "class": "artifact"},
|
||||
{"pattern": "projects/*/*-design-*.md", "class": "artifact"},
|
||||
{"pattern": "projects/*/*-test-plan-*.md", "class": "artifact"},
|
||||
{"pattern": "projects/*/*-eng-review-test-plan-*.md", "class": "artifact"},
|
||||
{"pattern": "retros/*.md", "class": "artifact"},
|
||||
{"pattern": "builder-journey.md", "class": "artifact"},
|
||||
{"pattern": "projects/*/timeline.jsonl", "class": "behavioral"},
|
||||
|
|
|
|||
|
|
@ -0,0 +1,97 @@
|
|||
#!/usr/bin/env bash
|
||||
# Migration: v1.40.0.0 — add eng-review-test-plan project-root pattern to
|
||||
# .brain-allowlist, .brain-privacy-map.json, and .gitattributes (#1452 follow-on).
|
||||
#
|
||||
# Why a second migration: v1.38.1.0 shipped two of three filenames for #1452
|
||||
# (`*-design-*.md` and `*-test-plan-*.md`) but missed `/plan-eng-review`'s
|
||||
# actual filename: `*-eng-review-test-plan-*.md`. The v1.38.1.0 migration has
|
||||
# a done-marker, so a "fix v1.38.1.0 and re-run" approach silently no-ops on
|
||||
# existing users. v1.40.0.0 needs its own migration to patch installs that
|
||||
# already ran v1.38.1.0.
|
||||
#
|
||||
# Per-file independent — if one file is missing we still repair the others.
|
||||
#
|
||||
# Idempotent: each insertion is gated on `not already present` so re-running
|
||||
# the migration is a no-op.
|
||||
|
||||
set -u
|
||||
|
||||
GSTACK_HOME="${HOME}/.gstack"
|
||||
ALLOWLIST="${GSTACK_HOME}/.brain-allowlist"
|
||||
PRIVACY="${GSTACK_HOME}/.brain-privacy-map.json"
|
||||
GITATTRS="${GSTACK_HOME}/.gitattributes"
|
||||
|
||||
MIGRATION_DIR="${GSTACK_HOME}/.migrations"
|
||||
DONE="${MIGRATION_DIR}/v1.40.0.0.done"
|
||||
|
||||
mkdir -p "${MIGRATION_DIR}" 2>/dev/null || true
|
||||
if [ -f "${DONE}" ]; then
|
||||
exit 0
|
||||
fi
|
||||
|
||||
NEW_PATTERNS=(
|
||||
'projects/*/*-eng-review-test-plan-*.md'
|
||||
)
|
||||
|
||||
added_any=0
|
||||
|
||||
# ----- .brain-allowlist ---------------------------------------------------
|
||||
if [ -f "${ALLOWLIST}" ]; then
|
||||
for PATTERN in "${NEW_PATTERNS[@]}"; do
|
||||
if ! grep -Fq -- "${PATTERN}" "${ALLOWLIST}" 2>/dev/null; then
|
||||
if grep -q '^# ---- USER ADDITIONS BELOW' "${ALLOWLIST}" 2>/dev/null; then
|
||||
sed -i.bak "/^# ---- USER ADDITIONS BELOW/i\\
|
||||
${PATTERN}
|
||||
" "${ALLOWLIST}" && rm -f "${ALLOWLIST}.bak"
|
||||
added_any=1
|
||||
else
|
||||
printf '%s\n' "${PATTERN}" >> "${ALLOWLIST}"
|
||||
added_any=1
|
||||
fi
|
||||
fi
|
||||
done
|
||||
fi
|
||||
|
||||
# ----- .brain-privacy-map.json -------------------------------------------
|
||||
if [ -f "${PRIVACY}" ]; then
|
||||
if command -v jq >/dev/null 2>&1; then
|
||||
for PATTERN in "${NEW_PATTERNS[@]}"; do
|
||||
if ! jq -e --arg p "${PATTERN}" 'map(select(.pattern == $p)) | length > 0' "${PRIVACY}" >/dev/null 2>&1; then
|
||||
if jq --arg p "${PATTERN}" '. += [{"pattern": $p, "class": "artifact"}]' "${PRIVACY}" > "${PRIVACY}.tmp" 2>/dev/null; then
|
||||
mv "${PRIVACY}.tmp" "${PRIVACY}"
|
||||
added_any=1
|
||||
else
|
||||
rm -f "${PRIVACY}.tmp"
|
||||
echo " [v1.40.0.0] WARN: jq failed to patch ${PRIVACY}; skipping pattern ${PATTERN}." >&2
|
||||
fi
|
||||
fi
|
||||
done
|
||||
else
|
||||
echo " [v1.40.0.0] WARN: jq not found; skipping privacy-map repair. Install jq and re-run gstack-upgrade, or run gstack-artifacts-init manually." >&2
|
||||
fi
|
||||
fi
|
||||
|
||||
# ----- .gitattributes -----------------------------------------------------
|
||||
if [ -f "${GITATTRS}" ]; then
|
||||
for PATTERN in "${NEW_PATTERNS[@]}"; do
|
||||
RULE="${PATTERN} merge=union"
|
||||
if ! grep -Fq -- "${RULE}" "${GITATTRS}" 2>/dev/null; then
|
||||
printf '%s\n' "${RULE}" >> "${GITATTRS}"
|
||||
added_any=1
|
||||
fi
|
||||
done
|
||||
fi
|
||||
|
||||
# Mark done even if no patches needed — a fresh-init user's
|
||||
# bin/gstack-artifacts-init now writes the pattern directly, so re-runs
|
||||
# should no-op. The touchfile keeps the migration runner from looping.
|
||||
touch "${DONE}"
|
||||
|
||||
if [ "${added_any}" = "1" ]; then
|
||||
echo " [v1.40.0.0] allowlist/privacy-map/gitattributes patched for /plan-eng-review test plans (idempotent)" >&2
|
||||
fi
|
||||
|
||||
# NEVER `git commit + push` from this migration. The user controls when the
|
||||
# patches ship into their federated artifacts repo.
|
||||
|
||||
exit 0
|
||||
|
|
@ -201,3 +201,133 @@ describe('v1.38.1.0 migration', () => {
|
|||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ──────────────────────────────────────────────────────────────────────────
|
||||
// v1.40.0.0 — `projects/*/*-eng-review-test-plan-*.md` follow-on for #1452.
|
||||
// v1.38.1.0 shipped the design + test-plan patterns but missed
|
||||
// /plan-eng-review's filename. Codex review #5 flagged that
|
||||
// v1.38.1.0's done-marker prevents users who already upgraded from picking
|
||||
// up #1465's allowlist edit, so v1.40.0.0 needs its own migration.
|
||||
// ──────────────────────────────────────────────────────────────────────────
|
||||
const MIGRATION_V1_40 = join(REPO_ROOT, 'gstack-upgrade', 'migrations', 'v1.40.0.0.sh');
|
||||
|
||||
function runMigrationV140(fakeHome: string): { code: number; stdout: string; stderr: string } {
|
||||
const proc = Bun.spawnSync({
|
||||
cmd: ['bash', MIGRATION_V1_40],
|
||||
env: { ...process.env, HOME: fakeHome },
|
||||
stdout: 'pipe',
|
||||
stderr: 'pipe',
|
||||
});
|
||||
return {
|
||||
code: proc.exitCode ?? -1,
|
||||
stdout: new TextDecoder().decode(proc.stdout),
|
||||
stderr: new TextDecoder().decode(proc.stderr),
|
||||
};
|
||||
}
|
||||
|
||||
describe('v1.40.0.0 migration', () => {
|
||||
test('adds eng-review-test-plan pattern to allowlist on top of an installed v1.38.1.0 state', () => {
|
||||
const home = setupFakeHome();
|
||||
try {
|
||||
// Simulate post-v1.38.1.0 state: design + test-plan patterns present,
|
||||
// done-marker set so the v1.38.1.0 migration wouldn't re-run.
|
||||
mkdirSync(join(home, '.gstack', '.migrations'), { recursive: true });
|
||||
writeFileSync(join(home, '.gstack', '.migrations', 'v1.38.1.0.done'), '');
|
||||
writeFileSync(join(home, '.gstack', '.brain-allowlist'), [
|
||||
'projects/*/learnings.jsonl',
|
||||
'projects/*/designs/*.md',
|
||||
'projects/*/*-design-*.md',
|
||||
'projects/*/*-test-plan-*.md',
|
||||
'# ---- USER ADDITIONS BELOW ---- (survives re-init; above is managed)',
|
||||
'projects/*/my-custom.txt',
|
||||
].join('\n') + '\n');
|
||||
|
||||
const r = runMigrationV140(home);
|
||||
expect(r.code).toBe(0);
|
||||
|
||||
const content = readFileSync(join(home, '.gstack', '.brain-allowlist'), 'utf-8');
|
||||
expect(content).toContain('projects/*/*-eng-review-test-plan-*.md');
|
||||
// New pattern above the user marker.
|
||||
const engRevIdx = content.indexOf('projects/*/*-eng-review-test-plan-*.md');
|
||||
const markerIdx = content.indexOf('# ---- USER ADDITIONS BELOW');
|
||||
expect(engRevIdx).toBeLessThan(markerIdx);
|
||||
// User customizations below the marker preserved.
|
||||
expect(content).toContain('projects/*/my-custom.txt');
|
||||
// v1.40.0.0 done-marker created.
|
||||
expect(existsSync(join(home, '.gstack', '.migrations', 'v1.40.0.0.done'))).toBe(true);
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('adds eng-review-test-plan entry to privacy-map.json via jq', () => {
|
||||
const home = setupFakeHome();
|
||||
try {
|
||||
writeFileSync(join(home, '.gstack', '.brain-privacy-map.json'), JSON.stringify([
|
||||
{ pattern: 'projects/*/*-design-*.md', class: 'artifact' },
|
||||
{ pattern: 'projects/*/*-test-plan-*.md', class: 'artifact' },
|
||||
], null, 2));
|
||||
|
||||
const r = runMigrationV140(home);
|
||||
expect(r.code).toBe(0);
|
||||
|
||||
const parsed = JSON.parse(readFileSync(join(home, '.gstack', '.brain-privacy-map.json'), 'utf-8'));
|
||||
const patterns = parsed.map((e: any) => e.pattern);
|
||||
expect(patterns).toContain('projects/*/*-eng-review-test-plan-*.md');
|
||||
expect(parsed.find((e: any) => e.pattern === 'projects/*/*-eng-review-test-plan-*.md').class).toBe('artifact');
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('adds union-merge rule to gitattributes', () => {
|
||||
const home = setupFakeHome();
|
||||
try {
|
||||
writeFileSync(join(home, '.gstack', '.gitattributes'), [
|
||||
'projects/*/*-design-*.md merge=union',
|
||||
'projects/*/*-test-plan-*.md merge=union',
|
||||
].join('\n') + '\n');
|
||||
|
||||
const r = runMigrationV140(home);
|
||||
expect(r.code).toBe(0);
|
||||
|
||||
const content = readFileSync(join(home, '.gstack', '.gitattributes'), 'utf-8');
|
||||
expect(content).toContain('projects/*/*-eng-review-test-plan-*.md merge=union');
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('is idempotent: re-running is a no-op', () => {
|
||||
const home = setupFakeHome();
|
||||
try {
|
||||
writeFileSync(join(home, '.gstack', '.brain-allowlist'),
|
||||
'projects/*/*-eng-review-test-plan-*.md\n# ---- USER ADDITIONS BELOW ---- (survives re-init; above is managed)\n');
|
||||
|
||||
const r1 = runMigrationV140(home);
|
||||
expect(r1.code).toBe(0);
|
||||
|
||||
const r2 = runMigrationV140(home);
|
||||
expect(r2.code).toBe(0);
|
||||
|
||||
const content = readFileSync(join(home, '.gstack', '.brain-allowlist'), 'utf-8');
|
||||
const occurrences = content.match(/projects\/\*\/\*-eng-review-test-plan-\*\.md/g) || [];
|
||||
expect(occurrences.length).toBe(1);
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('writes done-marker even when files are missing', () => {
|
||||
const home = setupFakeHome();
|
||||
try {
|
||||
// No allowlist / privacy-map / gitattributes — fresh-init users with
|
||||
// no federated artifacts yet. Migration should still mark itself done.
|
||||
const r = runMigrationV140(home);
|
||||
expect(r.code).toBe(0);
|
||||
expect(existsSync(join(home, '.gstack', '.migrations', 'v1.40.0.0.done'))).toBe(true);
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue