Wave 3: SSoT flip tooling (forge-side)#3
Merged
brandonwie merged 27 commits intomainfrom Apr 24, 2026
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 50 minutes and 47 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds Wave 3 SSoT flip tooling: a migration/rollback script, a topology-focused drift checker, workflow split for Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Flip as flip-to-forge.sh
participant Forge as Forge Repo
participant Repo3B as 3B Repo
participant FS as Filesystem
participant State as scripts/.flip-state.json
User->>Flip: run --execute
Flip->>Forge: read plugins/3b/SOURCE-MANIFEST.yaml
Forge-->>Flip: manifest entries
Flip->>Repo3B: ensure clean git working tree
Repo3B-->>Flip: clean confirmed
Flip->>Repo3B: git rev-parse HEAD (baseline_sha)
Repo3B-->>Flip: baseline SHA
Flip->>State: write .flip-state.json (baseline + entries + relative targets)
State-->>Flip: state persisted
loop per manifest entry
Flip->>FS: remove source file
FS-->>Flip: removed
Flip->>FS: create relative symlink -> forge target
FS-->>Flip: symlink created
end
Flip-->>User: exit success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Introduces forge-side tooling and documentation for Wave 3’s Single Source of Truth (SSoT) ownership flip of the shared 3B content tracked in plugins/3b/SOURCE-MANIFEST.yaml, moving toward a symlink-based consumption model on the 3B side.
Changes:
- Adds
scripts/flip-to-forge.shto plan/execute/rollback the 3B-side conversion of manifest-tracked files into relative symlinks targeting forge. - Rewrites
scripts/check-3b-drift.shinto post-flip topology checks (symlink integrity/targets + advisory scans). - Updates plugin docs/policy and changelog entries to describe the Wave 3 model and tooling.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/flip-to-forge.sh | New flip/rollback script that computes relative symlinks and records rollback state. |
| scripts/check-3b-drift.sh | Replaces SHA-based drift counting with post-flip integrity checks + advisory scans. |
| plugins/3b/README.md | Updates plugin status and adds an SSoT topology diagram + tooling links. |
| plugins/3b/PUBLIC-PRIVATE-SPLIT.md | Switches tier classification to manifest-driven for shipped files; keeps grep rubric for candidates. |
| CHANGELOG.md | Documents Wave 3 tooling and plugin v0.0.4 release notes. |
Comments suppressed due to low confidence (1)
scripts/check-3b-drift.sh:49
--verboseis still parsed into VERBOSE but never used after the rewrite. This is dead code and can confuse users about available output modes; either reintroduce verbose behavior for checks/reports or remove the flag/variable.
# --- Flag parsing -----------------------------------------------------------
VERBOSE=0
for arg in "$@"; do
case "$arg" in
--verbose | -v) VERBOSE=1 ;;
-h | --help)
sed -n '2,33p' "$0"
exit 0
;;
*)
echo "Unknown flag: $arg" >&2
exit 2
;;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e52d171933
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
scripts/check-3b-drift.sh (1)
193-198: Quote$FORGE_3B_ROOTinside the prefix-removal to avoid glob interpretation (SC2295).If
FORGE_3B_ROOTever contains glob metacharacters (*,?,[), the unquoted form would treat them as a pattern rather than a literal prefix.♻️ Proposed tweak
- rel="${f#$FORGE_3B_ROOT/}" + rel="${f#"$FORGE_3B_ROOT"/}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check-3b-drift.sh` around lines 193 - 198, The parameter expansion removing the prefix uses an unquoted FORGE_3B_ROOT which can trigger globbing; change the prefix-removal to quote the variable (use the form that references FORGE_3B_ROOT within quotes when computing rel from f) so that rel="${f#$FORGE_3B_ROOT/}" becomes the quoted equivalent and treats the prefix literally (update the occurrence where rel is defined and any similar expansions in the script).scripts/flip-to-forge.sh (3)
89-110: Unify the Python-interpolation pattern; the execute path is safe, the rest isn't.The execute-phase state writer (lines 263–295) correctly uses env-var passing with a quoted
<<'PY'heredoc, so paths with shell/Python metacharacters can't break the embedded script. The four earlier Python invocations do the opposite — they interpolate$STATE_FILE,$MANIFEST,$forge_abs, and$source_absdirectly into Python source via an unquoted heredoc or-cargument:
- Line 89:
python3 -c "...open('$STATE_FILE')..."- Lines 104–110:
python3 -c "...open('$STATE_FILE')..."- Line 169:
with open("$MANIFEST") as f:inside<<PY(unquoted)- Line 209:
python3 -c "...os.path.relpath('$forge_abs', ...'$source_abs')..."Low real-world risk (paths come from
cd/pwdand the YAML manifest), but a single quote anywhere in a path or manifest string would silently miscompile the Python. Consider switching these to the sameVAR=... python3 - <<'PY' … os.environ[...]pattern you already use.♻️ Example: line 209 using env vars
- # Compute relative target: from dir-of-source to forge_abs - rel_target=$(python3 -c "import os; print(os.path.relpath('$forge_abs', os.path.dirname('$source_abs')))") + # Compute relative target: from dir-of-source to forge_abs + rel_target=$(FORGE_ABS="$forge_abs" SOURCE_ABS="$source_abs" python3 -c ' +import os +print(os.path.relpath(os.environ["FORGE_ABS"], os.path.dirname(os.environ["SOURCE_ABS"])))')Also applies to: 167-173, 208-212
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/flip-to-forge.sh` around lines 89 - 110, Several python3 invocations interpolate shell variables directly into Python source (e.g., the uses of python3 -c opening '$STATE_FILE' and the unquoted heredoc that reads "$MANIFEST" and the python call using '$forge_abs'/'$source_abs'), which breaks on paths containing quotes; change these to export the shell variable(s) and run python with a quoted heredoc (VAR="$STATE_FILE" python3 - <<'PY' ... import os; state_file = os.environ['VAR'] ...) or similar so the Python reads values from os.environ; update the occurrences that produce ENTRIES and the other three spots (the python -c lines and the unquoted <<PY block that opens MANIFEST and computes relpath from forge_abs/source_abs) to use this env-var + <<'PY' (single-quoted) pattern instead of interpolating into the Python source.
233-237: Quote prefix-removal to avoid glob semantics (SC2295).Same tweak as the one in
check-3b-drift.sh: if$FORGE_3B_ROOTever contains glob metacharacters, the unquoted prefix is treated as a pattern.♻️ Proposed tweak
while IFS=$'\t' read -r source_abs forge_abs rel_target; do [ -z "$source_abs" ] && continue - echo " ${source_abs#$FORGE_3B_ROOT/}" + echo " ${source_abs#"$FORGE_3B_ROOT"/}" echo " → $rel_target" done <<<"$PLAN"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/flip-to-forge.sh` around lines 233 - 237, The prefix removal uses an unquoted $FORGE_3B_ROOT in the parameter expansion which can be treated as a glob if it contains metacharacters; fix by assigning the prefix to a variable (e.g. prefix="$FORGE_3B_ROOT/") and then use the safe expansion ${source_abs#$prefix} (and keep the expansion quoted in the echo call), referencing the while loop variables source_abs, rel_target and the FORGE_3B_ROOT/ prefix.
273-295: Replacedatetime.datetime.utcnow()— deprecated in Python 3.12+.Emits a
DeprecationWarningand will eventually be removed. Use a timezone-aware alternative.♻️ Suggested fix
state = { "baseline_sha": os.environ["BASELINE_SHA"], - "timestamp": datetime.datetime.utcnow().isoformat() + "Z", + "timestamp": datetime.datetime.now(datetime.timezone.utc).isoformat() + "Z", "forge_home": os.environ["FORGE_HOME"],(Preserves microsecond precision; alternatively, use
.strftime("%Y-%m-%dT%H:%M:%SZ")if sub-second precision is unnecessary.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/flip-to-forge.sh` around lines 273 - 295, The use of datetime.datetime.utcnow() to build the "timestamp" field is deprecated; change it to a timezone-aware UTC timestamp (e.g., datetime.datetime.now(datetime.timezone.utc).isoformat()) when constructing state["timestamp"] in the block that builds the state dict (symbol: state, key: "timestamp"); ensure you preserve microsecond precision and keep the existing "Z" suffix behavior (or use the ISO offset from the aware datetime) and update imports if needed (datetime.timezone).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/3b/PUBLIC-PRIVATE-SPLIT.md`:
- Around line 99-102: The hit-range table in PUBLIC-PRIVATE-SPLIT.md leaves
files with exactly 11 hits unclassified; update the range lines so 11 is
covered—either change the Tier B line `3–10` to `3–11` or change the Tier C line
`≥ 12` to `≥ 11` (choose one) and ensure the three bullet lines (`0–2` hits →
Tier A..., `3–10` hits → Tier B..., `≥ 12` hits → Tier C...) are adjusted
accordingly so there is no gap for 11 hits.
In `@plugins/3b/README.md`:
- Around line 7-9: Update the stale graduation heading to match the new status:
replace the heading text "## Graduation criterion (v0.0.2 → v0.1.0)" with "##
Graduation criterion (v0.0.4 → v0.1.0)" so it aligns with the status line that
currently reads "v0.0.4".
In `@scripts/check-3b-drift.sh`:
- Around line 140-143: The error message uses macOS-only stat formatting;
replace the stat call so the type/detection is portable by invoking python3 to
inspect source_abs and emit a clear type string; update the critical_report
assembly (where critical_report is appended with "Expected symlink, got: $(stat
-f '%HT' "$source_abs" 2>/dev/null || echo 'missing')") to call a short python3
one-liner that uses os.path.lexists/os.path.islink/os.path.isdir to print
"symlink", "file", "directory" or "missing" (referencing source_abs and
source_path variables) so the message is correct on Linux and macOS.
In `@scripts/flip-to-forge.sh`:
- Around line 48-50: The help block printed for the -h | --help case uses sed
with a range that ends at line 32 and thus omits the Safety section; update the
sed range in the -h | --help branch (the line with sed -n '2,32p' in
scripts/flip-to-forge.sh) so it prints through the full header block (e.g., end
at line 37) to include the Safety: section and the
hard-allowlist/clean-tree/baseline-SHA contract.
- Around line 112-134: After a successful rollback (when restored/failed loop
completes and before the final exit 0), automatically archive the flip state
file ($STATE_FILE, e.g. ".flip-state.json") instead of leaving it in place: if
"$failed" == "0" and the file exists, move it to a timestamped backup name (for
example ".flip-state.json.rolled-back-<timestamp>" using date +%s or an ISO
timestamp) so downstream scripts like scripts/check-3b-drift.sh do not see the
original $STATE_FILE and misclassify entries; ensure the move succeeds (or log
an error) and preserve the original as the backup before exiting.
---
Nitpick comments:
In `@scripts/check-3b-drift.sh`:
- Around line 193-198: The parameter expansion removing the prefix uses an
unquoted FORGE_3B_ROOT which can trigger globbing; change the prefix-removal to
quote the variable (use the form that references FORGE_3B_ROOT within quotes
when computing rel from f) so that rel="${f#$FORGE_3B_ROOT/}" becomes the quoted
equivalent and treats the prefix literally (update the occurrence where rel is
defined and any similar expansions in the script).
In `@scripts/flip-to-forge.sh`:
- Around line 89-110: Several python3 invocations interpolate shell variables
directly into Python source (e.g., the uses of python3 -c opening '$STATE_FILE'
and the unquoted heredoc that reads "$MANIFEST" and the python call using
'$forge_abs'/'$source_abs'), which breaks on paths containing quotes; change
these to export the shell variable(s) and run python with a quoted heredoc
(VAR="$STATE_FILE" python3 - <<'PY' ... import os; state_file =
os.environ['VAR'] ...) or similar so the Python reads values from os.environ;
update the occurrences that produce ENTRIES and the other three spots (the
python -c lines and the unquoted <<PY block that opens MANIFEST and computes
relpath from forge_abs/source_abs) to use this env-var + <<'PY' (single-quoted)
pattern instead of interpolating into the Python source.
- Around line 233-237: The prefix removal uses an unquoted $FORGE_3B_ROOT in the
parameter expansion which can be treated as a glob if it contains
metacharacters; fix by assigning the prefix to a variable (e.g.
prefix="$FORGE_3B_ROOT/") and then use the safe expansion ${source_abs#$prefix}
(and keep the expansion quoted in the echo call), referencing the while loop
variables source_abs, rel_target and the FORGE_3B_ROOT/ prefix.
- Around line 273-295: The use of datetime.datetime.utcnow() to build the
"timestamp" field is deprecated; change it to a timezone-aware UTC timestamp
(e.g., datetime.datetime.now(datetime.timezone.utc).isoformat()) when
constructing state["timestamp"] in the block that builds the state dict (symbol:
state, key: "timestamp"); ensure you preserve microsecond precision and keep the
existing "Z" suffix behavior (or use the ISO offset from the aware datetime) and
update imports if needed (datetime.timezone).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c87f202f-f054-42f6-8d48-c9caa4d06d56
📒 Files selected for processing (5)
CHANGELOG.mdplugins/3b/PUBLIC-PRIVATE-SPLIT.mdplugins/3b/README.mdscripts/check-3b-drift.shscripts/flip-to-forge.sh
Adds scripts/flip-to-forge.sh with three modes: --dry-run plan the flip (default; read-only, tolerates dirty 3B tree) --execute perform the flip and emit scripts/.flip-state.json --rollback restore each manifest entry from recorded baseline SHA Flips the 18 Wave-2 manifest entries in 3B from real files to relative symlinks pointing into this forge repo. Relative targets computed via os.path.relpath so the link survives \$HOME changes. Reuses the PyYAML manifest loader pattern from check-3b-drift.sh. Safety: - hard allowlist: refuses any path outside the 18-entry manifest - --execute requires clean 3B tree (baseline SHA must be restorable) - per-entry preconditions: forge target exists, 3B path is a real file - .flip-state.json gates re-execution (move aside to intentionally re-flip) The 3B-side flip is executed via this script AFTER forge PR #3 merges; this commit ships only the tooling. Co-authored-by: claude <noreply@anthropic.com>
The pre-Wave-3 drift script counted commits between manifest SHA and 3B HEAD per source_path. After the Wave 3 flip, 3B files become symlinks into forge, making that comparison trivially equal and the drift signal meaningless. This rewrite replaces the count-based drift with five topology checks: A. Symlink integrity manifest entry in 3B is -L and target exists B. Wrong target readlink resolves to computed forge path C. Untracked candidates new Tier-A-looking files in 3B .claude/ D. Reintroduced refs forge Tier-A contains ~/dev/personal/3b/ E. Plugin-reinstall recorded symlink is now a regular file Checks A/B/E activate only when scripts/.flip-state.json exists (post-flip mode); C/D run in both modes. This preserves back-compat so the new script ships in forge PR #3 without needing the 3B-side flip merged first. Exit codes: 0 clean 1 critical (A/B/E) 2 advisory-only (C/D) or pre-flight failure Emergency recovery of pre-Wave-3 logic: `git show wave2-backup:scripts/check-3b-drift.sh`. Co-authored-by: claude <noreply@anthropic.com>
…fest-based Post-Wave-3, tier classification is an explicit property of manifest membership, not a content-grep score. For shipped files, forge either owns them (listed in SOURCE-MANIFEST.yaml → Tier A) or it does not (Tier B candidate / Tier C private). The grep rubric is retained for scoring new candidates before migration. Also documents the new drift-check semantics (five topology checks replacing the old `git log SHA..HEAD` count) and the post-flip consumer relationship between 3B and forge. Co-authored-by: claude <noreply@anthropic.com>
Adds a mermaid flowchart showing forge as SoT and 3B as consumer via relative symlinks. Points readers at the three supporting artifacts: flip-to-forge.sh, check-3b-drift.sh, and the updated tier rubric in PUBLIC-PRIVATE-SPLIT.md. Also bumps status header from v0.0.2 to v0.0.4 to reflect the Wave 3 landing. Co-authored-by: claude <noreply@anthropic.com>
CLAUDE.md gains a "SoT ownership (Wave 3)" section under § 3B Integration spelling out that forge owns the 18 manifest entries and 3B consumes via relative symlinks. Future edits to shared skills / rules / agents / hooks happen in forge. CHANGELOG [Unreleased] § Harness-level gains two new Added bullets (flip-to-forge.sh + drift rewrite) and three new Changed bullets covering the policy and doc updates from commits 3 and 4. Existing Wave 2 entries are preserved underneath for continuity. Co-authored-by: claude <noreply@anthropic.com>
Review of the initial flip script surfaced three compounded issues:
1. .flip-state.json was written AFTER the flip loop completed. A
mid-loop failure (disk full, signal, permission denied) would leave
a partially flipped 3B tree with no rollback record.
2. `rm <path>` followed by `ln -s <target> <path>` is not atomic. If
ln -s fails the original file is already gone, and with no state
file (per 1) there is no recovery path.
3. \$PLAN was interpolated inline into a python3 HEREDOC via triple
quotes. Paths containing quote or backslash characters would break
the literal. Low probability but structurally fragile.
Fix: emit .flip-state.json BEFORE the flip loop begins (the plan is
fully validated at that point, so the state file is always correct),
then perform rm/ln-s per entry. A mid-loop abort is now recoverable via
--rollback regardless of how many entries were flipped. The plan is
passed to Python via a tempfile (cleaned on exit trap) instead of
shell-into-python interpolation.
Post-flip operator guidance updated to mention rollback is safe both
before and after committing the symlinks, since git checkout restores
tree objects from the baseline SHA regardless of current index state.
Findings 1, 2, 4 from Wave 3 forge-side review.
Co-authored-by: claude <noreply@anthropic.com>
The original Check B used a shell `cd ... && cd ... && pwd` chain to
resolve the actual symlink target and compare against the computed
forge absolute path. This pattern breaks in two cases:
1. macOS (BSD) readlink lacks -f, so there is no native one-shot
absolute resolution. The cd-chain workaround depends on evaluating
`dirname $(readlink <path>)` in the outer shell, then cd-ing
relative. For relative targets containing multiple `..` components
(as produced by os.path.relpath in flip-to-forge.sh), the
intermediate cd can escape its expected base dir, returning the
wrong resolved path and flagging a spurious WRONG-TARGET.
2. Both sides of the comparison used different normalisation (expected
was raw absolute, actual was the cd-chain result), so a path that
contains `.` or `..` in either string would compare unequal even
when the filesystem resolves both to the same inode.
Fix: replace the cd-chain with `python3 -c "print(os.path.realpath(...))"`
for both sides. python3 is already a hard dependency (PyYAML loader),
so no new requirement. os.path.realpath is consistent across macOS and
Linux and normalises both sides honestly.
Finding 3 from Wave 3 forge-side review.
Co-authored-by: claude <noreply@anthropic.com>
README.md's status header was bumped to v0.0.4 in commit 0ee146e but the CHANGELOG plugin-level section still only listed v0.0.2 → v0.0.3 from Wave 2. This commit closes the gap by documenting the v0.0.4 plugin-level change explicitly: the Wave 3 ownership flip, the PUBLIC-PRIVATE-SPLIT tier-model rewrite, and the new README topology diagram. Finding 7 from Wave 3 forge-side review. Co-authored-by: claude <noreply@anthropic.com>
brandonwie
force-pushed
the
wave3/ssot-flip
branch
from
e52d171 to
568c5eb
Compare
Prevents double-fire when @claude review is used (handled by new claude-code-review.yml). Fails fast with clear message if secret absent.
Dedicated workflow for @claude review on PR comments. Custom prompt with 3b-forge context (macOS-only plugin harness), 7 review categories (Security, Portability, Plugin Correctness, Shell Quality, Docs, Change Discipline, SSoT Integrity), and conventions to avoid false positives (symlink docs/, verbatim history, Tier-B parameterization). Scoped tools: gh pr comment/diff/view + inline_comment MCP.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Document claude-code-review.yml (dedicated @claude review handler) and claude.yml exclusion-guard update under harness-level Unreleased. Notes the 3b-forge review-category adaptation and deliberate skip of per-actor environment gating for a solo personal repo. Co-authored-by: claude <noreply@anthropic.com>
Plan validation runs before any filesystem writes. Exiting 1 there violated the documented exit-code contract (1 = mid-run abort with possibly partial state; 2 = pre-flight failure, no changes made). Callers relying on the contract to distinguish safe-to-retry from state-may-be-dirty would misread a clean precondition failure. Co-authored-by: claude <noreply@anthropic.com>
After rollback, 18 entries are restored to regular files but the state file still exists. check-3b-drift.sh gates Checks A/B/E on the mere existence of .flip-state.json, so the next drift run flips into post-flip mode and reports all 18 entries as A. NOT-A-SYMLINK — 18 false-positive critical failures. Move the file to a timestamped archive so the drift checker reverts to pre-flip mode automatically. Re-flipping starts from a clean slate. Co-authored-by: claude <noreply@anthropic.com>
Three python3 invocations embedded shell-interpolated paths directly into the Python source (rollback ENTRIES parse, manifest parse, rel_target computation). Single quotes in any path would break the arguments, and the unquoted heredoc still expanded shell variables into Python code. Switch to env-var passing with quoted <<'PY' heredocs — the same pattern already used by the execute-path state writer. The Python source becomes opaque to the shell and paths containing special characters are safe. Co-authored-by: claude <noreply@anthropic.com>
forge_abs and source_abs are built via string concatenation. A manifest entry with `../` segments could cause `rm` and `ln -s` to operate on files outside $FORGE_HOME / $FORGE_3B_ROOT, violating the documented hard-allowlist safety contract. Compute realpath of both resolved paths and reject any entry whose resolved form does not stay under the canonical repo roots. Resolve the root prefixes once before the loop to avoid 18 redundant realpath calls. Co-authored-by: claude <noreply@anthropic.com>
The header block runs from line 2 through line 37, but sed -n '2,32p' stopped before the Safety: section. Users running --help missed the hard-allowlist / clean-tree / baseline-SHA contract. Extend the range. Co-authored-by: claude <noreply@anthropic.com>
stat -f '%HT' is BSD/macOS-only. On GNU/Linux the -f flag switches stat to filesystem-stat mode, %HT is undefined, and the command errors. The `|| echo 'missing'` fallback then fires — producing a misleading "Expected symlink, got: missing" for a file that exists as a regular file. Replace with python3 os.path lexists/islink/isdir/isfile, the same portability pattern already used for realpath resolution in Check B. Co-authored-by: claude <noreply@anthropic.com>
…bsent Post-flip detection relied solely on scripts/.flip-state.json. On fresh forge clones (CI jobs, collaborators), the state file is absent but the 3B entries may already be symlinks. The script silently ran in pre-flip mode and skipped Checks A/B/E — the critical symlink integrity checks. Add a topology signal: if the first manifest entry in $FORGE_3B_ROOT is a symlink, treat the run as post-flip and emit a WARNING so operators know why the state file is missing. Also switch the manifest parse to env-var passing for consistency with the flip script. Co-authored-by: claude <noreply@anthropic.com>
…flip Status text said the Wave 3 SSoT flip landed and that 3B already consumes via relative symlink. This PR only ships the forge-side tooling; the actual flip (--execute + 3B-side consumer symlinks) is a follow-on PR. Rewording prevents readers from assuming the migration is complete. Co-authored-by: claude <noreply@anthropic.com>
…ot forge The "adds SoT ownership section" CLAUDE.md edit was made in the 3B-side symlink target, not in this repo. git diff origin/main...HEAD shows no CLAUDE.md change in the PR, so the changelog line documents a change that did not happen in forge git. Co-authored-by: claude <noreply@anthropic.com>
…ift.sh Exit 2 was overloaded: both pre-flight failures (missing env var, bad flag, manifest not found) AND advisory-only findings (C/D) returned 2. Callers could not distinguish "script couldn't run" from "script ran and noticed something". Reserve exit 2 for pre-flight failures only. Advisory-only runs now exit 3. Callers treating >=2 as fatal stay correct; those wanting to branch on the difference can now do so. Co-authored-by: claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
plugins/3b/README.md (1)
158-158:⚠️ Potential issue | 🟡 MinorStale graduation heading persists after version bump.
The heading still references
v0.0.2 → v0.1.0but the status line (Line 7) was bumped tov0.0.4. This was previously flagged and remains unaddressed.📝 Proposed fix
-## Graduation criterion (v0.0.2 → v0.1.0) +## Graduation criterion (v0.0.4 → v0.1.0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/3b/README.md` at line 158, Update the stale graduation heading text "## Graduation criterion (v0.0.2 → v0.1.0)" to reflect the current status bump by changing it to "## Graduation criterion (v0.0.4 → v0.1.0)"; locate the heading that currently shows v0.0.2 and make the version on the left match the status line which was already updated to v0.0.4.
🧹 Nitpick comments (1)
scripts/flip-to-forge.sh (1)
89-89: Inconsistent shell→python interpolation; prefer the env-var pattern used elsewhere.The rest of the script (rollback
ENTRIESon Lines 106–115, manifest parse on Lines 167–186, per-entry realpath calls, and the plan-passing python block on Lines 298–330) consistently passes paths via environment variables with a quoted heredoc to avoid shell-into-python string-interpolation hazards. This line is the outlier and embeds$STATE_FILEdirectly into a double-quoted python source string.While
$STATE_FILEis deterministic (${SCRIPT_DIR}/.flip-state.json) and unlikely to contain special characters today, matching the established pattern keeps the script's safety story uniform.♻️ Proposed refactor for consistency
- BASELINE_SHA=$(python3 -c "import json; print(json.load(open('$STATE_FILE'))['baseline_sha'])") + BASELINE_SHA=$(STATE_FILE="$STATE_FILE" python3 -c 'import json, os; print(json.load(open(os.environ["STATE_FILE"]))["baseline_sha"])')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/flip-to-forge.sh` at line 89, The BASELINE_SHA assignment embeds $STATE_FILE into a Python command string; change it to pass STATE_FILE via the environment (matching the rest of the script) and read os.environ['STATE_FILE'] inside the Python snippet instead of interpolating the shell variable into the source; update the BASELINE_SHA=$(...) usage so the Python subprocess receives STATE_FILE as an env var (or via the existing quoted heredoc pattern used elsewhere) and uses json.load(open(os.environ['STATE_FILE'])) to extract 'baseline_sha'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/check-3b-drift.sh`:
- Line 225: The parameter-expansion strip rel="${f#$FORGE_3B_ROOT/}" treats
$FORGE_3B_ROOT as a glob pattern so paths with glob metacharacters can fail to
be stripped; change the expansion to quote the prefix pattern so the variable is
treated literally (e.g. use the quoted pattern form with rel, f and
FORGE_3B_ROOT) and keep the rest of the manifest-membership check (grep -qxF)
intact so relative paths are correctly produced for the check.
In `@scripts/flip-to-forge.sh`:
- Line 270: The prefix-strip parameter expansion uses an unquoted $FORGE_3B_ROOT
which can be interpreted as a glob; update the expansion so the right-hand side
is quoted by nesting the variable expansion (i.e., use
${source_abs#${FORGE_3B_ROOT}/} instead of ${source_abs#$FORGE_3B_ROOT/}) so
that source_abs is correctly stripped relative to FORGE_3B_ROOT; change the echo
that references source_abs and FORGE_3B_ROOT accordingly.
- Around line 304-330: Replace the deprecated call to datetime.datetime.utcnow()
used when building the state["timestamp"] with
datetime.datetime.now(datetime.timezone.utc) to produce a timezone-aware UTC
datetime; then call .isoformat() and convert the "+00:00" suffix to "Z" (e.g.
.isoformat().replace("+00:00", "Z")) to preserve the original format used in the
timestamp field of the state dict. Update the code that assigns
state["timestamp"] (near the state literal) accordingly; you may also optionally
strip microseconds via .replace(microsecond=0) if you want to remove sub-second
precision.
---
Duplicate comments:
In `@plugins/3b/README.md`:
- Line 158: Update the stale graduation heading text "## Graduation criterion
(v0.0.2 → v0.1.0)" to reflect the current status bump by changing it to "##
Graduation criterion (v0.0.4 → v0.1.0)"; locate the heading that currently shows
v0.0.2 and make the version on the left match the status line which was already
updated to v0.0.4.
---
Nitpick comments:
In `@scripts/flip-to-forge.sh`:
- Line 89: The BASELINE_SHA assignment embeds $STATE_FILE into a Python command
string; change it to pass STATE_FILE via the environment (matching the rest of
the script) and read os.environ['STATE_FILE'] inside the Python snippet instead
of interpolating the shell variable into the source; update the
BASELINE_SHA=$(...) usage so the Python subprocess receives STATE_FILE as an env
var (or via the existing quoted heredoc pattern used elsewhere) and uses
json.load(open(os.environ['STATE_FILE'])) to extract 'baseline_sha'.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2f3fa253-28c0-48bb-ae71-618b5dc94729
📒 Files selected for processing (7)
.github/workflows/claude-code-review.yml.github/workflows/claude.ymlCHANGELOG.mdplugins/3b/PUBLIC-PRIVATE-SPLIT.mdplugins/3b/README.mdscripts/check-3b-drift.shscripts/flip-to-forge.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/3b/PUBLIC-PRIVATE-SPLIT.md
… (SC2295)
${source_abs#$FORGE_3B_ROOT/} treats the pattern as a glob — if the
path ever contains glob metacharacters, the strip silently misbehaves.
Quoting the expansion ${source_abs#"$FORGE_3B_ROOT"/} forces literal
match. Same fix applied in both flip-to-forge.sh dry-run echo and
check-3b-drift.sh scan-result rel path.
Co-authored-by: claude <noreply@anthropic.com>
…e form datetime.utcnow() emits DeprecationWarning on Python 3.12+ and will be removed in a future release. Switch to datetime.now(datetime.timezone.utc), then strip the +00:00 suffix to keep the existing Z-suffixed ISO format stable for .flip-state.json consumers. Co-authored-by: claude <noreply@anthropic.com>
Heading said v0.0.2 → v0.1.0 but the status badge bumped to v0.0.4 when Wave 3 tooling landed. Align heading with current version so the release path reads consistently. Co-authored-by: claude <noreply@anthropic.com>
Rubric read 0-2 → Tier A, 3-10 → Tier B, ≥ 12 → Tier C — a file scoring exactly 11 hits was unclassified. Change threshold to ≥ 11 so every non-negative count maps to a tier. Co-authored-by: claude <noreply@anthropic.com>
The --verbose / -v flag set VERBOSE=1 but no code path read the value. Dead option misleads users into expecting per-check verbose output that does not exist. Strip the flag; add it back with real behavior if verbose reporting is ever implemented. Co-authored-by: claude <noreply@anthropic.com>
…e diff Shallow clone (fetch-depth: 1) breaks `git diff origin/main...HEAD` and any merge-base computation — the review action relies on both to scope changes. Bump to fetch-depth: 0 so the action sees the full history. Co-authored-by: claude <noreply@anthropic.com>
Owner
Author
Addressed AI review findings (Round 1)16 items across 4 reviewers — 16 fixed, 0 dismissed, 0 deferred. Fixed
Reviewers & convergence
Convergence was heavy: path traversal (R1-5) flagged by 3 reviewers; Commits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wave 3 inverts ownership of the 18 shared content files in
plugins/3b/SOURCE-MANIFEST.yaml. Forge becomes Single Source of Truth;3B will consume via relative symlinks (follow-on PR). This PR ships only
the forge-side tooling + policy/doc updates.
Scope
scripts/flip-to-forge.sh(new) —--dry-run/--execute/--rollbackmodes. Relative symlinks viaos.path.relpath. Writes.flip-state.jsonBEFORE the flip loop so rollback is guaranteed even on mid-loop abort. Tempfile-based plan passing avoids shell-into-python interpolation fragility.scripts/check-3b-drift.sh(rewrite) — five post-flip checks replace the oldgit log SHA..HEADcount. A symlink integrity / B wrong target / C untracked Tier-A candidates / D reintroduced hardcoded paths / E plugin-reinstall damage. Checks A/B/E gate on.flip-state.jsonpresence → back-compat with pre-flip regular-file state. Check B usespython3 os.path.realpathfor macOS compatibility.plugins/3b/PUBLIC-PRIVATE-SPLIT.md— tier classification switched from content-grep heuristic to manifest-membership SoT. Grep rubric retained for scoring new candidates.plugins/3b/README.md— mermaid SSoT topology diagram + status bump to v0.0.4.CHANGELOG.md— harness-level Unreleased +plugins/3b/v0.0.3 → v0.0.4 sections.CLAUDE.mdedit landed in 3B source (symlink target), not forge git. Intentional per 3B design; see commite6b3f85body.Rollback anchor
Tag
wave2-backupcreated onmainat commit430a8bapre-branch for emergency recovery of the old drift script viagit show wave2-backup:scripts/check-3b-drift.sh.Review iteration
Commits
2e1f59c,6c87ba1,e52d171resolve review findings (3 critical + 2 important + 1 advisory):Finding 6 (Check D grep noise on prose mentions) deferred — advisory-only, not a data risk.
Test plan
Follow-on PRs (not in this PR)
Plan: `~/.claude/plans/proceed-with-wave-three-vectorized-dragonfly.md`.
Branch scaffolding: `docs/actives/wave3-ssot-flip/{plan.md,todos.md}`.
Summary by CodeRabbit
New Features
Documentation