mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-10 15:09:00 +02:00
feat(review): content-addressed staleness via working-tree fingerprint
Review records now bind to the content they were made on. bin/gstack-review-log
stamps every appended record with commit_full, tree, dirty (informational) and
wtree — a working-tree fingerprint from the new bin/gstack-wtree (temp index
seeded from HEAD + git add -A + write-tree). The binding fields are computed
authoritatively; caller-supplied values for those keys are ignored, so a stale
rendered template or a forged field can't bind a record to content it wasn't
made on.
Why a working-tree fingerprint instead of HEAD^{tree}: committing identical
content doesn't change it (a record made on a dirty tree stays valid after the
same content is committed), untracked new source files DO change it (new code
can't hide from freshness), and gitignored scratch stays out. Rebase, amend
and squash with identical content grade CURRENT instead of stale.
Grading: the dashboard (scripts/resolvers/review.ts) and /land-and-deploy Step
3.5a apply a content-first rule to diff-scoped review rows — wtree match with
both sides clean is CURRENT, full stop. Plan-tier reviews grade a plan file,
not the repo tree, so they keep the 7-day logic (optional plan_sha256 caller
field noted). The rev-list fallback no longer errors when the stored commit
was rebased away: it grades UNKNOWN and treats it as stale.
bin/gstack-review-read emits ---WTREE---/---TREE---/---DIRTY--- so graders
consume one tool output. Old records without wtree fall back to the existing
heuristics; no migration.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
455c805125
commit
8e4a4f7c3d
+39
-4
@@ -1,21 +1,56 @@
|
||||
#!/usr/bin/env bash
|
||||
# gstack-review-log — atomically log a review result
|
||||
# Usage: gstack-review-log '{"skill":"...","timestamp":"...","status":"..."}'
|
||||
#
|
||||
# Binding fields (content-addressed staleness): every appended record is
|
||||
# stamped with commit_full, tree, dirty (informational) and wtree (the GATING
|
||||
# working-tree fingerprint from bin/gstack-wtree). These are computed
|
||||
# AUTHORITATIVELY here — caller-supplied values for the four keys are ignored,
|
||||
# so a stale rendered template (or a forged field) cannot bind a record to
|
||||
# content it wasn't made on. All other caller fields pass through untouched.
|
||||
# Outside a git repo the fields are simply omitted (legacy consumers fall back
|
||||
# to their heuristics).
|
||||
set -euo pipefail
|
||||
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
|
||||
eval "$("$SCRIPT_DIR/gstack-slug" 2>/dev/null)"
|
||||
GSTACK_HOME="${GSTACK_HOME:-$HOME/.gstack}"
|
||||
mkdir -p "$GSTACK_HOME/projects/$SLUG"
|
||||
|
||||
# Validate: input must be parseable JSON (reject malformed or injection attempts)
|
||||
INPUT="$1"
|
||||
if ! printf '%s' "$INPUT" | bun -e "JSON.parse(await Bun.stdin.text())" 2>/dev/null; then
|
||||
|
||||
# Compute binding fields (best-effort; empty outside a git repo).
|
||||
COMMIT_FULL=$(git rev-parse HEAD 2>/dev/null || true)
|
||||
TREE=""
|
||||
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)
|
||||
if [ -n "$(git status --porcelain -uno 2>/dev/null | head -1)" ]; then
|
||||
DIRTY="true"
|
||||
else
|
||||
DIRTY="false"
|
||||
fi
|
||||
fi
|
||||
|
||||
# Validate (reject malformed or injection attempts) AND stamp in one pass.
|
||||
# Caller values for the binding keys are dropped before stamping.
|
||||
STAMPED=$(printf '%s' "$INPUT" | GSTACK_STAMP_COMMIT_FULL="$COMMIT_FULL" GSTACK_STAMP_TREE="$TREE" GSTACK_STAMP_WTREE="$WTREE" GSTACK_STAMP_DIRTY="$DIRTY" bun -e "
|
||||
const rec = JSON.parse(await Bun.stdin.text());
|
||||
for (const k of ['commit_full', 'tree', 'wtree', 'dirty']) delete rec[k];
|
||||
const env = process.env;
|
||||
if (env.GSTACK_STAMP_COMMIT_FULL) rec.commit_full = env.GSTACK_STAMP_COMMIT_FULL;
|
||||
if (env.GSTACK_STAMP_TREE) rec.tree = env.GSTACK_STAMP_TREE;
|
||||
if (env.GSTACK_STAMP_WTREE) rec.wtree = env.GSTACK_STAMP_WTREE;
|
||||
if (env.GSTACK_STAMP_DIRTY) rec.dirty = env.GSTACK_STAMP_DIRTY === 'true';
|
||||
console.log(JSON.stringify(rec));
|
||||
" 2>/dev/null) || {
|
||||
# Not valid JSON — refuse to append
|
||||
echo "gstack-review-log: invalid JSON, skipping" >&2
|
||||
exit 1
|
||||
fi
|
||||
}
|
||||
|
||||
echo "$INPUT" >> "$GSTACK_HOME/projects/$SLUG/$BRANCH-reviews.jsonl"
|
||||
echo "$STAMPED" >> "$GSTACK_HOME/projects/$SLUG/$BRANCH-reviews.jsonl"
|
||||
|
||||
# gbrain-sync: enqueue for cross-machine sync (no-op if sync is off).
|
||||
"$SCRIPT_DIR/gstack-brain-enqueue" "projects/$SLUG/$BRANCH-reviews.jsonl" 2>/dev/null &
|
||||
|
||||
@@ -1,6 +1,13 @@
|
||||
#!/usr/bin/env bash
|
||||
# gstack-review-read — read review log and config for dashboard
|
||||
# Usage: gstack-review-read
|
||||
#
|
||||
# Emits, in order: the raw reviews JSONL, ---CONFIG--- (skip_eng_review),
|
||||
# ---HEAD--- (short sha), ---WTREE--- (current working-tree fingerprint from
|
||||
# bin/gstack-wtree, or "unknown"), ---TREE--- (HEAD tree, informational) and
|
||||
# ---DIRTY--- (tracked-file dirty flag). Consumers grade diff-scoped review
|
||||
# rows CURRENT when a record's `wtree` equals ---WTREE---; everything needed
|
||||
# for that rule ships in this one output so graders run no extra commands.
|
||||
set -euo pipefail
|
||||
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
|
||||
eval "$("$SCRIPT_DIR/gstack-slug" 2>/dev/null)"
|
||||
@@ -10,3 +17,9 @@ echo "---CONFIG---"
|
||||
"$SCRIPT_DIR/gstack-config" get skip_eng_review 2>/dev/null || echo "false"
|
||||
echo "---HEAD---"
|
||||
git rev-parse --short HEAD 2>/dev/null || echo "unknown"
|
||||
echo "---WTREE---"
|
||||
"$SCRIPT_DIR/gstack-wtree" 2>/dev/null || echo "unknown"
|
||||
echo "---TREE---"
|
||||
git rev-parse 'HEAD^{tree}' 2>/dev/null || echo "unknown"
|
||||
echo "---DIRTY---"
|
||||
if [ -n "$(git status --porcelain -uno 2>/dev/null | head -1)" ]; then echo "true"; else echo "false"; fi
|
||||
|
||||
Executable
+27
@@ -0,0 +1,27 @@
|
||||
#!/usr/bin/env bash
|
||||
# gstack-wtree — print a working-tree CONTENT fingerprint (a git tree hash).
|
||||
#
|
||||
# Builds a temp index seeded from HEAD, stages the full working tree into it
|
||||
# (`git add -A`, so .gitignore'd scratch stays out and UNTRACKED source is
|
||||
# included), and prints `git write-tree` of that index. Properties that make
|
||||
# this the right staleness fingerprint, vs `git rev-parse HEAD^{tree}`:
|
||||
#
|
||||
# - Committing identical content does NOT change the fingerprint, so a
|
||||
# record made on a dirty tree stays valid after the exact same content is
|
||||
# committed (the /ship Step 5 -> Step 16 case).
|
||||
# - Untracked new source files DO change the fingerprint, so "tests passed"
|
||||
# can't stay FRESH after a new file appears.
|
||||
# - Rebase/amend/squash that preserve content do not change it.
|
||||
#
|
||||
# The real repo index is never touched. Staged blobs land in the object store
|
||||
# as unreachable objects and get gc'd like stash churn. Exit 1 outside a git
|
||||
# repo or in a repo with no commits — callers treat that as "no fingerprint".
|
||||
set -euo pipefail
|
||||
|
||||
TOP=$(git rev-parse --show-toplevel 2>/dev/null) || exit 1
|
||||
TMPIDX=$(mktemp "${TMPDIR:-/tmp}/gstack-wtree-XXXXXX")
|
||||
trap 'rm -f "$TMPIDX"' EXIT
|
||||
export GIT_INDEX_FILE="$TMPIDX"
|
||||
git -C "$TOP" read-tree HEAD 2>/dev/null || exit 1
|
||||
git -C "$TOP" add -A 2>/dev/null || exit 1
|
||||
git -C "$TOP" write-tree 2>/dev/null
|
||||
@@ -1177,10 +1177,11 @@ Display:
|
||||
- If \`skip_eng_review\` config is \`true\`, Eng Review shows "SKIPPED (global)" and verdict is CLEARED
|
||||
|
||||
**Staleness detection:** After displaying the dashboard, check if any existing reviews may be stale:
|
||||
- Parse the \`---HEAD---\` section from the bash output to get the current HEAD commit hash
|
||||
- For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. Display: "Note: {skill} review from {date} may be stale — {N} commits since review"
|
||||
- **Content-first rule (diff-scoped rows only: \`review\`, \`adversarial-review\`, ship-stage entries).** Parse the \`---WTREE---\` and \`---DIRTY---\` sections from the bash output. If an entry has a \`wtree\` field AND it equals the current \`---WTREE---\` value AND the entry's \`dirty\` is false AND \`---DIRTY---\` is false, the review is CURRENT — identical content, regardless of commit count, rebase, or amend. Skip the commit-count heuristic for that entry and show no staleness note.
|
||||
- Plan-tier rows (plan-ceo-review, plan-eng-review, plan-design-review) grade a plan file, not the repo tree — never apply the wtree rule to them; they keep the 7-day freshness logic. If such an entry carries a \`plan_sha256\` field, you MAY compare it against the current plan file's sha256 and note "plan changed since review" on mismatch.
|
||||
- Fallback (no \`wtree\` on the entry, wtree mismatch, or either side dirty): parse the \`---HEAD---\` section to get the current HEAD commit hash. For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. If that command FAILS (the stored commit was rebased away), grade UNKNOWN and treat as stale — do not error. Display: "Note: {skill} review from {date} may be stale — {N} commits since review"
|
||||
- For entries without a \`commit\` field (legacy entries): display "Note: {skill} review from {date} has no commit tracking — consider re-running for accurate staleness detection"
|
||||
- If all reviews match the current HEAD, do not display any staleness notes
|
||||
- If all reviews grade CURRENT (wtree match or HEAD match), do not display any staleness notes
|
||||
|
||||
## Plan File Review Report
|
||||
|
||||
|
||||
@@ -1298,13 +1298,25 @@ plan-design-review, design-review-lite, codex-review, review, adversarial-review
|
||||
codex-plan-review):
|
||||
|
||||
1. Find the most recent entry within the last 7 days.
|
||||
2. Extract its `commit` field.
|
||||
3. Compare against current HEAD: `git rev-list --count STORED_COMMIT..HEAD`
|
||||
2. **Content-first rule (diff-scoped rows only: `review`, `adversarial-review`,
|
||||
`codex-review`, ship-stage entries).** If the entry has a `wtree` field AND it
|
||||
equals the `---WTREE---` section of the output AND the entry's `dirty` is false
|
||||
AND `---DIRTY---` is false → **CURRENT**, full stop. Identical working-tree
|
||||
content, regardless of commit count, rebase, or amend — skip steps 3-4 for
|
||||
this entry. Never apply the wtree rule to plan-tier rows (plan-eng-review,
|
||||
plan-ceo-review, plan-design-review): those grade a plan file, not the repo
|
||||
tree — they keep the 7-day logic and the commit heuristic below.
|
||||
3. Extract its `commit` field.
|
||||
4. Compare against current HEAD: `git rev-list --count STORED_COMMIT..HEAD`.
|
||||
**If this command fails** (the stored commit was rebased away and is
|
||||
unreachable) → grade **UNKNOWN** and treat as STALE. Do not error out of the
|
||||
readiness check.
|
||||
|
||||
**Staleness rules:**
|
||||
**Staleness rules (fallback path):**
|
||||
- 0 commits since review → CURRENT
|
||||
- 1-3 commits since review → RECENT (yellow if those commits touch code, not just docs)
|
||||
- 4+ commits since review → STALE (red — review may not reflect current code)
|
||||
- rev-list failed → UNKNOWN (treat as STALE)
|
||||
- No review found → NOT RUN
|
||||
|
||||
**Critical check:** Look at what changed AFTER the last review. Run:
|
||||
@@ -1314,6 +1326,8 @@ git log --oneline STORED_COMMIT..HEAD
|
||||
If any commits after the review contain words like "fix", "refactor", "rewrite",
|
||||
"overhaul", or touch more than 5 files — flag as **STALE (significant changes
|
||||
since review)**. The review was done on different code than what's about to merge.
|
||||
(Skip this check for entries already graded CURRENT by the content-first rule —
|
||||
same content is same content.)
|
||||
|
||||
**Also check for adversarial review (`codex-review`).** If codex-review has been run
|
||||
and is CURRENT, mention it in the readiness report as an extra confidence signal.
|
||||
|
||||
@@ -394,13 +394,25 @@ plan-design-review, design-review-lite, codex-review, review, adversarial-review
|
||||
codex-plan-review):
|
||||
|
||||
1. Find the most recent entry within the last 7 days.
|
||||
2. Extract its `commit` field.
|
||||
3. Compare against current HEAD: `git rev-list --count STORED_COMMIT..HEAD`
|
||||
2. **Content-first rule (diff-scoped rows only: `review`, `adversarial-review`,
|
||||
`codex-review`, ship-stage entries).** If the entry has a `wtree` field AND it
|
||||
equals the `---WTREE---` section of the output AND the entry's `dirty` is false
|
||||
AND `---DIRTY---` is false → **CURRENT**, full stop. Identical working-tree
|
||||
content, regardless of commit count, rebase, or amend — skip steps 3-4 for
|
||||
this entry. Never apply the wtree rule to plan-tier rows (plan-eng-review,
|
||||
plan-ceo-review, plan-design-review): those grade a plan file, not the repo
|
||||
tree — they keep the 7-day logic and the commit heuristic below.
|
||||
3. Extract its `commit` field.
|
||||
4. Compare against current HEAD: `git rev-list --count STORED_COMMIT..HEAD`.
|
||||
**If this command fails** (the stored commit was rebased away and is
|
||||
unreachable) → grade **UNKNOWN** and treat as STALE. Do not error out of the
|
||||
readiness check.
|
||||
|
||||
**Staleness rules:**
|
||||
**Staleness rules (fallback path):**
|
||||
- 0 commits since review → CURRENT
|
||||
- 1-3 commits since review → RECENT (yellow if those commits touch code, not just docs)
|
||||
- 4+ commits since review → STALE (red — review may not reflect current code)
|
||||
- rev-list failed → UNKNOWN (treat as STALE)
|
||||
- No review found → NOT RUN
|
||||
|
||||
**Critical check:** Look at what changed AFTER the last review. Run:
|
||||
@@ -410,6 +422,8 @@ git log --oneline STORED_COMMIT..HEAD
|
||||
If any commits after the review contain words like "fix", "refactor", "rewrite",
|
||||
"overhaul", or touch more than 5 files — flag as **STALE (significant changes
|
||||
since review)**. The review was done on different code than what's about to merge.
|
||||
(Skip this check for entries already graded CURRENT by the content-first rule —
|
||||
same content is same content.)
|
||||
|
||||
**Also check for adversarial review (`codex-review`).** If codex-review has been run
|
||||
and is CURRENT, mention it in the readiness report as an extra confidence signal.
|
||||
|
||||
@@ -669,10 +669,11 @@ Display:
|
||||
- If \`skip_eng_review\` config is \`true\`, Eng Review shows "SKIPPED (global)" and verdict is CLEARED
|
||||
|
||||
**Staleness detection:** After displaying the dashboard, check if any existing reviews may be stale:
|
||||
- Parse the \`---HEAD---\` section from the bash output to get the current HEAD commit hash
|
||||
- For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. Display: "Note: {skill} review from {date} may be stale — {N} commits since review"
|
||||
- **Content-first rule (diff-scoped rows only: \`review\`, \`adversarial-review\`, ship-stage entries).** Parse the \`---WTREE---\` and \`---DIRTY---\` sections from the bash output. If an entry has a \`wtree\` field AND it equals the current \`---WTREE---\` value AND the entry's \`dirty\` is false AND \`---DIRTY---\` is false, the review is CURRENT — identical content, regardless of commit count, rebase, or amend. Skip the commit-count heuristic for that entry and show no staleness note.
|
||||
- Plan-tier rows (plan-ceo-review, plan-eng-review, plan-design-review) grade a plan file, not the repo tree — never apply the wtree rule to them; they keep the 7-day freshness logic. If such an entry carries a \`plan_sha256\` field, you MAY compare it against the current plan file's sha256 and note "plan changed since review" on mismatch.
|
||||
- Fallback (no \`wtree\` on the entry, wtree mismatch, or either side dirty): parse the \`---HEAD---\` section to get the current HEAD commit hash. For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. If that command FAILS (the stored commit was rebased away), grade UNKNOWN and treat as stale — do not error. Display: "Note: {skill} review from {date} may be stale — {N} commits since review"
|
||||
- For entries without a \`commit\` field (legacy entries): display "Note: {skill} review from {date} has no commit tracking — consider re-running for accurate staleness detection"
|
||||
- If all reviews match the current HEAD, do not display any staleness notes
|
||||
- If all reviews grade CURRENT (wtree match or HEAD match), do not display any staleness notes
|
||||
|
||||
## Plan File Review Report
|
||||
|
||||
|
||||
@@ -405,10 +405,11 @@ Display:
|
||||
- If \`skip_eng_review\` config is \`true\`, Eng Review shows "SKIPPED (global)" and verdict is CLEARED
|
||||
|
||||
**Staleness detection:** After displaying the dashboard, check if any existing reviews may be stale:
|
||||
- Parse the \`---HEAD---\` section from the bash output to get the current HEAD commit hash
|
||||
- For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. Display: "Note: {skill} review from {date} may be stale — {N} commits since review"
|
||||
- **Content-first rule (diff-scoped rows only: \`review\`, \`adversarial-review\`, ship-stage entries).** Parse the \`---WTREE---\` and \`---DIRTY---\` sections from the bash output. If an entry has a \`wtree\` field AND it equals the current \`---WTREE---\` value AND the entry's \`dirty\` is false AND \`---DIRTY---\` is false, the review is CURRENT — identical content, regardless of commit count, rebase, or amend. Skip the commit-count heuristic for that entry and show no staleness note.
|
||||
- Plan-tier rows (plan-ceo-review, plan-eng-review, plan-design-review) grade a plan file, not the repo tree — never apply the wtree rule to them; they keep the 7-day freshness logic. If such an entry carries a \`plan_sha256\` field, you MAY compare it against the current plan file's sha256 and note "plan changed since review" on mismatch.
|
||||
- Fallback (no \`wtree\` on the entry, wtree mismatch, or either side dirty): parse the \`---HEAD---\` section to get the current HEAD commit hash. For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. If that command FAILS (the stored commit was rebased away), grade UNKNOWN and treat as stale — do not error. Display: "Note: {skill} review from {date} may be stale — {N} commits since review"
|
||||
- For entries without a \`commit\` field (legacy entries): display "Note: {skill} review from {date} has no commit tracking — consider re-running for accurate staleness detection"
|
||||
- If all reviews match the current HEAD, do not display any staleness notes
|
||||
- If all reviews grade CURRENT (wtree match or HEAD match), do not display any staleness notes
|
||||
|
||||
## Plan File Review Report
|
||||
|
||||
|
||||
@@ -643,10 +643,11 @@ Display:
|
||||
- If \`skip_eng_review\` config is \`true\`, Eng Review shows "SKIPPED (global)" and verdict is CLEARED
|
||||
|
||||
**Staleness detection:** After displaying the dashboard, check if any existing reviews may be stale:
|
||||
- Parse the \`---HEAD---\` section from the bash output to get the current HEAD commit hash
|
||||
- For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. Display: "Note: {skill} review from {date} may be stale — {N} commits since review"
|
||||
- **Content-first rule (diff-scoped rows only: \`review\`, \`adversarial-review\`, ship-stage entries).** Parse the \`---WTREE---\` and \`---DIRTY---\` sections from the bash output. If an entry has a \`wtree\` field AND it equals the current \`---WTREE---\` value AND the entry's \`dirty\` is false AND \`---DIRTY---\` is false, the review is CURRENT — identical content, regardless of commit count, rebase, or amend. Skip the commit-count heuristic for that entry and show no staleness note.
|
||||
- Plan-tier rows (plan-ceo-review, plan-eng-review, plan-design-review) grade a plan file, not the repo tree — never apply the wtree rule to them; they keep the 7-day freshness logic. If such an entry carries a \`plan_sha256\` field, you MAY compare it against the current plan file's sha256 and note "plan changed since review" on mismatch.
|
||||
- Fallback (no \`wtree\` on the entry, wtree mismatch, or either side dirty): parse the \`---HEAD---\` section to get the current HEAD commit hash. For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. If that command FAILS (the stored commit was rebased away), grade UNKNOWN and treat as stale — do not error. Display: "Note: {skill} review from {date} may be stale — {N} commits since review"
|
||||
- For entries without a \`commit\` field (legacy entries): display "Note: {skill} review from {date} has no commit tracking — consider re-running for accurate staleness detection"
|
||||
- If all reviews match the current HEAD, do not display any staleness notes
|
||||
- If all reviews grade CURRENT (wtree match or HEAD match), do not display any staleness notes
|
||||
|
||||
## Plan File Review Report
|
||||
|
||||
|
||||
@@ -728,10 +728,11 @@ Display:
|
||||
- If \`skip_eng_review\` config is \`true\`, Eng Review shows "SKIPPED (global)" and verdict is CLEARED
|
||||
|
||||
**Staleness detection:** After displaying the dashboard, check if any existing reviews may be stale:
|
||||
- Parse the \`---HEAD---\` section from the bash output to get the current HEAD commit hash
|
||||
- For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. Display: "Note: {skill} review from {date} may be stale — {N} commits since review"
|
||||
- **Content-first rule (diff-scoped rows only: \`review\`, \`adversarial-review\`, ship-stage entries).** Parse the \`---WTREE---\` and \`---DIRTY---\` sections from the bash output. If an entry has a \`wtree\` field AND it equals the current \`---WTREE---\` value AND the entry's \`dirty\` is false AND \`---DIRTY---\` is false, the review is CURRENT — identical content, regardless of commit count, rebase, or amend. Skip the commit-count heuristic for that entry and show no staleness note.
|
||||
- Plan-tier rows (plan-ceo-review, plan-eng-review, plan-design-review) grade a plan file, not the repo tree — never apply the wtree rule to them; they keep the 7-day freshness logic. If such an entry carries a \`plan_sha256\` field, you MAY compare it against the current plan file's sha256 and note "plan changed since review" on mismatch.
|
||||
- Fallback (no \`wtree\` on the entry, wtree mismatch, or either side dirty): parse the \`---HEAD---\` section to get the current HEAD commit hash. For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. If that command FAILS (the stored commit was rebased away), grade UNKNOWN and treat as stale — do not error. Display: "Note: {skill} review from {date} may be stale — {N} commits since review"
|
||||
- For entries without a \`commit\` field (legacy entries): display "Note: {skill} review from {date} has no commit tracking — consider re-running for accurate staleness detection"
|
||||
- If all reviews match the current HEAD, do not display any staleness notes
|
||||
- If all reviews grade CURRENT (wtree match or HEAD match), do not display any staleness notes
|
||||
|
||||
## Plan File Review Report
|
||||
|
||||
|
||||
@@ -67,10 +67,11 @@ Display:
|
||||
- If \\\`skip_eng_review\\\` config is \\\`true\\\`, Eng Review shows "SKIPPED (global)" and verdict is CLEARED
|
||||
|
||||
**Staleness detection:** After displaying the dashboard, check if any existing reviews may be stale:
|
||||
- Parse the \\\`---HEAD---\\\` section from the bash output to get the current HEAD commit hash
|
||||
- For each review entry that has a \\\`commit\\\` field: compare it against the current HEAD. If different, count elapsed commits: \\\`git rev-list --count STORED_COMMIT..HEAD\\\`. Display: "Note: {skill} review from {date} may be stale — {N} commits since review"
|
||||
- **Content-first rule (diff-scoped rows only: \\\`review\\\`, \\\`adversarial-review\\\`, ship-stage entries).** Parse the \\\`---WTREE---\\\` and \\\`---DIRTY---\\\` sections from the bash output. If an entry has a \\\`wtree\\\` field AND it equals the current \\\`---WTREE---\\\` value AND the entry's \\\`dirty\\\` is false AND \\\`---DIRTY---\\\` is false, the review is CURRENT — identical content, regardless of commit count, rebase, or amend. Skip the commit-count heuristic for that entry and show no staleness note.
|
||||
- Plan-tier rows (plan-ceo-review, plan-eng-review, plan-design-review) grade a plan file, not the repo tree — never apply the wtree rule to them; they keep the 7-day freshness logic. If such an entry carries a \\\`plan_sha256\\\` field, you MAY compare it against the current plan file's sha256 and note "plan changed since review" on mismatch.
|
||||
- Fallback (no \\\`wtree\\\` on the entry, wtree mismatch, or either side dirty): parse the \\\`---HEAD---\\\` section to get the current HEAD commit hash. For each review entry that has a \\\`commit\\\` field: compare it against the current HEAD. If different, count elapsed commits: \\\`git rev-list --count STORED_COMMIT..HEAD\\\`. If that command FAILS (the stored commit was rebased away), grade UNKNOWN and treat as stale — do not error. Display: "Note: {skill} review from {date} may be stale — {N} commits since review"
|
||||
- For entries without a \\\`commit\\\` field (legacy entries): display "Note: {skill} review from {date} has no commit tracking — consider re-running for accurate staleness detection"
|
||||
- If all reviews match the current HEAD, do not display any staleness notes`;
|
||||
- If all reviews grade CURRENT (wtree match or HEAD match), do not display any staleness notes`;
|
||||
}
|
||||
|
||||
export function generatePlanFileReviewReport(_ctx: TemplateContext): string {
|
||||
|
||||
+4
-3
@@ -993,10 +993,11 @@ Display:
|
||||
- If \`skip_eng_review\` config is \`true\`, Eng Review shows "SKIPPED (global)" and verdict is CLEARED
|
||||
|
||||
**Staleness detection:** After displaying the dashboard, check if any existing reviews may be stale:
|
||||
- Parse the \`---HEAD---\` section from the bash output to get the current HEAD commit hash
|
||||
- For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. Display: "Note: {skill} review from {date} may be stale — {N} commits since review"
|
||||
- **Content-first rule (diff-scoped rows only: \`review\`, \`adversarial-review\`, ship-stage entries).** Parse the \`---WTREE---\` and \`---DIRTY---\` sections from the bash output. If an entry has a \`wtree\` field AND it equals the current \`---WTREE---\` value AND the entry's \`dirty\` is false AND \`---DIRTY---\` is false, the review is CURRENT — identical content, regardless of commit count, rebase, or amend. Skip the commit-count heuristic for that entry and show no staleness note.
|
||||
- Plan-tier rows (plan-ceo-review, plan-eng-review, plan-design-review) grade a plan file, not the repo tree — never apply the wtree rule to them; they keep the 7-day freshness logic. If such an entry carries a \`plan_sha256\` field, you MAY compare it against the current plan file's sha256 and note "plan changed since review" on mismatch.
|
||||
- Fallback (no \`wtree\` on the entry, wtree mismatch, or either side dirty): parse the \`---HEAD---\` section to get the current HEAD commit hash. For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. If that command FAILS (the stored commit was rebased away), grade UNKNOWN and treat as stale — do not error. Display: "Note: {skill} review from {date} may be stale — {N} commits since review"
|
||||
- For entries without a \`commit\` field (legacy entries): display "Note: {skill} review from {date} has no commit tracking — consider re-running for accurate staleness detection"
|
||||
- If all reviews match the current HEAD, do not display any staleness notes
|
||||
- If all reviews grade CURRENT (wtree match or HEAD match), do not display any staleness notes
|
||||
|
||||
If the Eng Review is NOT "CLEAR":
|
||||
|
||||
|
||||
@@ -74,4 +74,139 @@ describe('gstack-review-log', () => {
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
function readNewestRecord(): any {
|
||||
const projectDirs = fs.readdirSync(slugDir);
|
||||
const projectDir = path.join(slugDir, projectDirs[0]);
|
||||
const jsonlFiles = fs.readdirSync(projectDir).filter((f) => f.endsWith('.jsonl'));
|
||||
const content = fs.readFileSync(path.join(projectDir, jsonlFiles[0]), 'utf-8').trim();
|
||||
const lines = content.split('\n');
|
||||
return JSON.parse(lines[lines.length - 1]);
|
||||
}
|
||||
|
||||
test('stamps authoritative binding fields (commit_full, tree, wtree, dirty) in a git repo', () => {
|
||||
const result = run('{"skill":"review","status":"clean"}');
|
||||
expect(result.exitCode).toBe(0);
|
||||
const rec = readNewestRecord();
|
||||
expect(rec.commit_full).toMatch(/^[0-9a-f]{40}$/);
|
||||
expect(rec.tree).toMatch(/^[0-9a-f]{40}$/);
|
||||
expect(rec.wtree).toMatch(/^[0-9a-f]{40}$/);
|
||||
expect(typeof rec.dirty).toBe('boolean');
|
||||
// Non-binding caller fields pass through untouched.
|
||||
expect(rec.skill).toBe('review');
|
||||
expect(rec.status).toBe('clean');
|
||||
});
|
||||
|
||||
test('caller-supplied binding fields are IGNORED, never trusted', () => {
|
||||
const forged = '{"skill":"review","status":"clean","wtree":"forged","tree":"forged","commit_full":"forged","dirty":"forged"}';
|
||||
const result = run(forged);
|
||||
expect(result.exitCode).toBe(0);
|
||||
const rec = readNewestRecord();
|
||||
expect(rec.wtree).not.toBe('forged');
|
||||
expect(rec.tree).not.toBe('forged');
|
||||
expect(rec.commit_full).not.toBe('forged');
|
||||
expect(rec.dirty).not.toBe('forged');
|
||||
expect(rec.wtree).toMatch(/^[0-9a-f]{40}$/);
|
||||
});
|
||||
|
||||
test('append still succeeds outside a git repo (binding fields omitted)', () => {
|
||||
const nonGit = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-nongit-'));
|
||||
try {
|
||||
const execOpts: ExecSyncOptionsWithStringEncoding = {
|
||||
cwd: nonGit,
|
||||
env: { ...process.env, GSTACK_HOME: tmpDir },
|
||||
encoding: 'utf-8',
|
||||
timeout: 10000,
|
||||
};
|
||||
execSync(`${BIN}/gstack-review-log '{"skill":"review","status":"clean"}'`, execOpts);
|
||||
// A record landed somewhere under projects/ without a wtree stamp.
|
||||
const found: string[] = [];
|
||||
const walk = (d: string) => {
|
||||
for (const e of fs.readdirSync(d, { withFileTypes: true })) {
|
||||
const p = path.join(d, e.name);
|
||||
if (e.isDirectory()) walk(p);
|
||||
else if (e.name.endsWith('-reviews.jsonl')) found.push(p);
|
||||
}
|
||||
};
|
||||
walk(slugDir);
|
||||
expect(found.length).toBeGreaterThan(0);
|
||||
const rec = JSON.parse(fs.readFileSync(found[0], 'utf-8').trim().split('\n').pop()!);
|
||||
expect(rec.skill).toBe('review');
|
||||
expect(rec.wtree).toBeUndefined();
|
||||
expect(rec.commit_full).toBeUndefined();
|
||||
} finally {
|
||||
fs.rmSync(nonGit, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('gstack-wtree', () => {
|
||||
function withScratchRepo(fn: (repoDir: string, wtree: () => string) => void) {
|
||||
const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-wtree-'));
|
||||
try {
|
||||
const git = (args: string) =>
|
||||
execSync(`git -c user.email=t@test -c user.name=t ${args}`, { cwd: repoDir, encoding: 'utf-8', timeout: 10000 });
|
||||
git('init -q -b main');
|
||||
fs.writeFileSync(path.join(repoDir, 'a.txt'), 'hello\n');
|
||||
fs.writeFileSync(path.join(repoDir, '.gitignore'), 'scratch.txt\n');
|
||||
git('add a.txt .gitignore');
|
||||
git('commit -q -m init');
|
||||
const wtree = () => execSync(`${BIN}/gstack-wtree`, { cwd: repoDir, encoding: 'utf-8', timeout: 10000 }).trim();
|
||||
fn(repoDir, wtree);
|
||||
} finally {
|
||||
fs.rmSync(repoDir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
test('an UNTRACKED source file changes the fingerprint; a gitignored file does not', () => {
|
||||
withScratchRepo((repoDir, wtree) => {
|
||||
const clean = wtree();
|
||||
expect(clean).toMatch(/^[0-9a-f]{40}$/);
|
||||
|
||||
// Gitignored scratch: invisible to the fingerprint (Conductor scratch stays out).
|
||||
fs.writeFileSync(path.join(repoDir, 'scratch.txt'), 'noise\n');
|
||||
expect(wtree()).toBe(clean);
|
||||
|
||||
// Untracked NEW source file: visible (new files can never be invisible to freshness).
|
||||
fs.writeFileSync(path.join(repoDir, 'new-source.ts'), 'export {}\n');
|
||||
expect(wtree()).not.toBe(clean);
|
||||
});
|
||||
});
|
||||
|
||||
test('committing identical content does NOT change the fingerprint', () => {
|
||||
withScratchRepo((repoDir, wtree) => {
|
||||
fs.writeFileSync(path.join(repoDir, 'a.txt'), 'edited\n');
|
||||
const dirtyFingerprint = wtree();
|
||||
execSync('git -c user.email=t@test -c user.name=t commit -q -am edit', { cwd: repoDir, timeout: 10000 });
|
||||
expect(wtree()).toBe(dirtyFingerprint);
|
||||
});
|
||||
});
|
||||
|
||||
test('exits non-zero outside a git repo', () => {
|
||||
const nonGit = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-wtree-nongit-'));
|
||||
try {
|
||||
expect(() => execSync(`${BIN}/gstack-wtree`, { cwd: nonGit, timeout: 10000, stdio: 'pipe' })).toThrow();
|
||||
} finally {
|
||||
fs.rmSync(nonGit, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('gstack-review-read', () => {
|
||||
test('emits ---WTREE---, ---TREE--- and ---DIRTY--- sections', () => {
|
||||
const out = execSync(`${BIN}/gstack-review-read`, {
|
||||
cwd: ROOT,
|
||||
env: { ...process.env, GSTACK_HOME: tmpDir },
|
||||
encoding: 'utf-8',
|
||||
timeout: 10000,
|
||||
});
|
||||
expect(out).toContain('---HEAD---');
|
||||
expect(out).toContain('---WTREE---');
|
||||
expect(out).toContain('---TREE---');
|
||||
expect(out).toContain('---DIRTY---');
|
||||
const wtreeLine = out.split('---WTREE---')[1].trim().split('\n')[0].trim();
|
||||
expect(wtreeLine).toMatch(/^([0-9a-f]{40}|unknown)$/);
|
||||
const dirtyLine = out.split('---DIRTY---')[1].trim().split('\n')[0].trim();
|
||||
expect(['true', 'false']).toContain(dirtyLine);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user