fix: refuse pharn add when clone layout mismatches recorded layout - #77
Conversation
Prevent orphan capability dirs by gating add on detectLayout vs configLayout before any write. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Next review available in: 52 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthrough
ChangesAdd layout gate
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AddCommand
participant LayoutGate
participant Installer
User->>AddCommand: run pharn add
AddCommand->>LayoutGate: compare fetched and configured layouts
LayoutGate-->>AddCommand: allow or refuse
AddCommand->>Installer: install capabilities when layouts match
Installer-->>User: report installation result
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.dev/features/add-layout-gate/GRILL.md:
- Line 3: Correct the future-dated GATE 1 approval record by replacing
2026-08-09 with the verified approval date in
.dev/features/add-layout-gate/GRILL.md lines 3-3 and use that same date in
.dev/features/add-layout-gate/SHIP.md lines 10-10.
In @.dev/features/add-layout-gate/PLAN.md:
- Line 181: Update the fenced pseudocode block in PLAN.md by labeling its
opening fence with the text language identifier, resolving the Markdownlint
MD040 violation.
- Around line 294-296: Update the recorded Gate 1 approval date in the “Open
questions — RESOLVED at GATE 1” section to the actual decision date, which must
not be later than August 8, 2026; leave the resolution status and surrounding
plan text unchanged.
In @.dev/features/add-layout-gate/REVIEW.md:
- Around line 95-96: Repair the inline Markdown in the review comments so the
`update` reference does not contain nested backticks; use plain quotation marks
or a double-backtick span. Then regenerate verify-report.json from the corrected
final file so its markdown lint result reflects the fix.
- Around line 117-120: Correct the advisory finding count so the verdict matches
the actual findings, either by adding the missing advisory entry or changing the
reported count. Update `.dev/features/add-layout-gate/REVIEW.md` lines 117-120
and synchronize `.dev/features/add-layout-gate/SHIP.md` lines 34-35 with the
same count.
In `@src/commands/add.ts`:
- Around line 106-110: Update the mismatch guidance returned by layoutGate to
direct users to run `pharn update --force` instead of plain `pharn update`, so
the recorded layout is migrated before retrying `pharn add`. Update the
corresponding message assertions and documentation examples to match the new
command.
🪄 Autofix
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 Plus
Run ID: 3149112e-68ec-4573-8984-919680673e0c
📒 Files selected for processing (15)
.dev/features/add-layout-gate/GRILL.md.dev/features/add-layout-gate/PLAN.md.dev/features/add-layout-gate/REGRESSION.md.dev/features/add-layout-gate/REVIEW.md.dev/features/add-layout-gate/SHIP.md.dev/features/add-layout-gate/VERIFY.md.dev/features/add-layout-gate/regression-report.json.dev/features/add-layout-gate/verify-report.json.pharn/writes-scope.jsonCHANGELOG.mdCLAUDE.mddocs/commands/add.mddocs/reference/pharn-config.mdsrc/commands/add.tstests/add.test.ts
| @@ -0,0 +1,183 @@ | |||
| # GRILL — add-layout-gate | |||
|
|
|||
| Plan under interrogation: `.dev/features/add-layout-gate/PLAN.md` (approved at GATE 1, 2026-08-09). | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Correct the future-dated approval records.
Both files state that GATE 1 was approved on August 9, 2026, while the current date is August 8, 2026. Correct both records to the verified approval date.
.dev/features/add-layout-gate/GRILL.md#L3-L3: replace2026-08-09with the verified approval date..dev/features/add-layout-gate/SHIP.md#L10-L10: use the same verified approval date.
📍 Affects 2 files
.dev/features/add-layout-gate/GRILL.md#L3-L3(this comment).dev/features/add-layout-gate/SHIP.md#L10-L10
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dev/features/add-layout-gate/GRILL.md at line 3, Correct the future-dated
GATE 1 approval record by replacing 2026-08-09 with the verified approval date
in .dev/features/add-layout-gate/GRILL.md lines 3-3 and use that same date in
.dev/features/add-layout-gate/SHIP.md lines 10-10.
| A single local helper in `add.ts`, mirroring `versionGate`'s shape exactly, called **once per | ||
| command**, inside each path's existing `try`: | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to this fenced block.
Line 181 opens an unlabeled code fence. Markdownlint reports MD040 for this fence. Use text for the pseudocode block.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 181-181: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dev/features/add-layout-gate/PLAN.md at line 181, Update the fenced
pseudocode block in PLAN.md by labeling its opening fence with the text language
identifier, resolving the Markdownlint MD040 violation.
Source: Linters/SAST tools
| ## Open questions — RESOLVED at GATE 1 (human, 2026-08-09) | ||
|
|
||
| Both were answered before any code was written; neither remains open. **Plan approved as written.** |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the recorded approval date.
Line 294 states that Gate 1 was resolved on August 9, 2026. That date is after the current review date of August 8, 2026. Use the actual decision date so the plan remains auditable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dev/features/add-layout-gate/PLAN.md around lines 294 - 296, Update the
recorded Gate 1 approval date in the “Open questions — RESOLVED at GATE 1”
section to the actual decision date, which must not be later than August 8,
2026; leave the resolution status and surrounding plan text unchanged.
| new comments are dense and directive in tone (`"add must NEVER record the clone's layout"`, | ||
| `"Only `update` can migrate a tree"`), and `CLAUDE.md` gained an imperative clause — but these are |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Repair the inline code span.
The backticks around update are nested inside the surrounding inline-code span. Static analysis reports markdownlint MD038 at Line 96, while verify-report.json records lint:md: 0. Use plain quotation marks or a double-backtick span, then regenerate the verification record from the final file.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 96-96: Spaces inside code span elements
(MD038, no-space-in-code)
[warning] 96-96: Spaces inside code span elements
(MD038, no-space-in-code)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dev/features/add-layout-gate/REVIEW.md around lines 95 - 96, Repair the
inline Markdown in the review comments so the `update` reference does not
contain nested backticks; use plain quotation marks or a double-backtick span.
Then regenerate verify-report.json from the corrected final file so its markdown
lint result reflects the fix.
Source: Linters/SAST tools
| **GREEN — 0 floor-gate findings, 4 advisory (2 important, 2 minor/other).** The increment is done by the | ||
| floor's measure: `validate` GREEN, `/pharn-dev-verify` PASS on all five gates, | ||
| `/pharn-dev-regress` `no-regressions`, 605 tests. The four advisory findings are for the human to weigh | ||
| at the post-review gate; none of them blocks, and none of them rests on anything the floor could have |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Correct the advisory finding count.
REVIEW.md lists three advisory findings, but both summaries report four. Correct the count in both files or add the missing finding.
.dev/features/add-layout-gate/REVIEW.md#L117-L120: change the verdict count or add the missing advisory entry..dev/features/add-layout-gate/SHIP.md#L34-L35: synchronize the ship summary withREVIEW.md.
📍 Affects 2 files
.dev/features/add-layout-gate/REVIEW.md#L117-L120(this comment).dev/features/add-layout-gate/SHIP.md#L34-L35
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dev/features/add-layout-gate/REVIEW.md around lines 117 - 120, Correct the
advisory finding count so the verdict matches the actual findings, either by
adding the missing advisory entry or changing the reported count. Update
`.dev/features/add-layout-gate/REVIEW.md` lines 117-120 and synchronize
`.dev/features/add-layout-gate/SHIP.md` lines 34-35 with the same count.
1 participant
What this changes
pharn addcopied capabilities at the clone's layout whileremove/status/listaddress the project viaconfigLayout. When those disagreed, adds landed where nothing looked for them — invisible to status/list, and a laterremovedropped only the config entry while reporting files already gone (orphaning the dir).This adds a layout gate (sibling of the version gate) that refuses when
detectLayout(clone) !== configLayout(config), names both layouts, points atpharn update, and writes nothing. Version still wins when both mismatch (??short-circuit).adddeliberately does not stamp the clone's layout the wayupdatedoes — that would re-address the rest of an install that still lives at the old paths.Type of change
feat— new stack option, wizard step, or command capabilityfix— bug fixdocs— docs-only changechore/refactor— tooling or internal restructure, no behavior changeArea(s) touched
commands/add|tests/add|docs/commands/add|docs/reference/pharn-config| CHANGELOG | CLAUDE.mdChecklist
.js-extension import convention.tests/*.test.tswhen wizard behavior changed (tests mirror step/lib files one-to-one).docs/page (see the "Documentation maintenance" table inCONTRIBUTING.md); unimplemented behavior is marked Coming soon or linked todocs/roadmap.md.src/lib/github.ts/src/lib/validate.ts(regex allowlists,..checks,redirect: 'error', timeout/size caps,schemaVersion === 1) if I touched remote-input handling.Quality gates
npm run checkpasses locally (format:check+lint+typecheck+test).npm run buildsucceeds.npm run test:coveragepasses (coverage thresholds met).Test plan
addrefuses when clone ispharnand config recordsflat(and the reverse); message names both layouts andpharn updategroupMultiselectrenderslayoutproceeds against a flat clone (configLayout default) and refuses against apharnclonepharn/pharnpath installs and recordspharn/-prefixed pathsNotes for the reviewer
Look at
layoutGate+ theversionGate(...) ?? layoutGate(...)call sites insrc/commands/add.tsfirst — especially why we refuse rather than recording the clone layout (unlikeupdate). Residual same-version layout drift still needspharn update --forcebecause plainupdateearly-returns on matchingskillsVersion.Made with Cursor
Summary by CodeRabbit
New Features
pharn addnow checks that the fetched repository layout matches the project’s configured layout.pharn update.Bug Fixes
Documentation