Remove symlink install logic - #2229
Conversation
This comment has been minimized.
This comment has been minimized.
|
Adversarial review of Independent read-only pass. Prior review rounds were not treated as evidence the change is correct. Summary of ChangesHead
The PR does not update Findings
Verdict: Needs more workReviewed-Commit: 6b962f0 Posted by Cursor Grok 4.6 (AI agent) --- not written by a human. |
|
Working on this --- please hold off on pushing to this branch until I'm done. Posted by Claude Code (AI agent) --- not written by a human. Generated by Claude Code |
The adversarial review on this PR found that removing the symlink install path (check-install.py, bootstrap.sh's per-directory symlink loop, check-harness-installs.py, and the other audit scripts) left several callers pointing at files that no longer exist, plus one unrelated CI check dropped along the way. Address: - scripts/doctor.py: drop the dead check_consumer_installs check (and the now-unused --fix flag) rather than shelling out to a deleted script; update scripts/test_doctor.py to match. - .claude/hooks/session-start.sh: correct the comment -- bootstrap.sh no longer symlinks skills/commands; this repo's own plugin manifest covers that for a remote session working in ai-config itself. - README.md, setup.qmd: fix the dangling clause left after the top paragraph's rewrite, the "Verify the install" section's call to the deleted check-harness-installs.py, the Gemini "symlinks" claim (it writes a path reference, not a symlink), and every other place that still described bootstrap.sh symlinking skills/commands/hooks/shared into consumer directories it no longer touches. Corrected the Cursor cursor-rules/ claim specifically: the Cursor plugin already ships it via plugin.json's "rules" field, so that one was never a gap. - scripts/check-harness-ignores.py, scripts/test_check_harness_ignores.py, .github/workflows/validate.yml: restored. This check is unrelated to symlink installs -- it verifies .cursorignore/.geminiignore still cover the local-residue paths .gitignore already excludes -- and its removal was an unrelated regression. - scripts/install-hooks.py, skills/post-merge/SKILL.md, shared/workflow/skill-checklists.md, AGENTS.md, shared/workflow/keep-checkouts-fresh.md, memories/claude-code-hooks.md, scripts/lib/link-one.sh, bootstrap.sh's LINK_ONE_FIX_HINT: updated every caller and cross-reference to stop naming check-install.py or describing bootstrap.sh's removed symlink behavior, without inventing a replacement mechanism that doesn't exist. - bootstrap.sh: removed the now-dead LINK_ONE_FIX_HINT/link-one.sh sourcing (nothing in bootstrap.sh calls link_one any more), and added a migration warning when an existing Gemini skills.json/plugins.json still names the old symlink-destination path instead of the checkout. - .github/workflows/validate.yml, scripts/test_cursor_rules_sync.py: fixed the stale "Run Cursor skill-install tests" step title/comment and docstring, which still described bootstrap stacking checks that no longer run. Defer (filed as #2352): shared/, hooks/, and the Codex wrapper tree still have no plugin-equivalent replacement for the bootstrap.sh symlink placement this PR removes, so a fresh install has no working path for those three today. That is a design decision for the repo owner (restore placement, build a real replacement, or document the gap), not something this ARD round should decide unilaterally -- fixed the affected docs to state the current, reduced capability honestly instead. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016MyNR6ZrvN6M4Rr8eZbHts
…l_logic # Conflicts: # .github/workflows/validate.yml # AGENTS.md # README.md # bootstrap.sh # hooks/hooks.json # scripts/check-harness-installs.py # scripts/cursor-plugin-enabled.py # scripts/lib/link-one.sh # scripts/test_bootstrap_cursor_plugin.py # scripts/test_check_harness_installs.py # scripts/test_cursor_plugin_enabled.py # scripts/test_cursor_rules_sync.py # setup.qmd
…l_logic # Conflicts: # .github/workflows/validate.yml
Addresses the pre-push adversarial review at c451447: 1. Split memories/claude-code.md's added historical-marker line at its semicolon so the new-line-breaks gate passes. 2. CLAUDE.md's freshness-check summary no longer instructs verifying consumer copies with the deleted check-install.py; it now describes the plugin-install check and scopes install-hooks.py to the registration half. 3. AGENTS.md no longer claims bootstrap.sh keeps ~/.gemini/skills current: it names the skills.json/plugins.json registrations that point at the checkout's own paths. 4. README.md's skills/ line likewise drops the ~/.gemini/skills claim. 5. keep-checkouts-fresh.md's one-hook-at-a-time explanation no longer asserts that bootstrap.sh places hook scripts. 6. verify-the-right-artifact.md's check-install.py passage is reworded as a historical illustration with the removal referenced, keeping the run-the-instrument principle intact. 7. scripts/lib/plugin_overlap.py is trimmed to its live surface (ai_config_entries + enabled_ai_config_plugins, imported by install-hooks.py); the orphaned load_settings/resolve_enabled/ describe_overlap went with the deleted CLI. scripts/ test_plugin_overlap.py restores lib-only regression coverage (9 checks, negative control verified: a substring matcher fails the CONTAINING test), wired into validate.yml. 8. doctor.py's docstring and bootstrap.sh's header no longer claim Codex installs as a native plugin; both point at ai-config#2352. 9. .claude/settings.json is rebuilt from main minus the UserPromptSubmit block: 2-space indent restored, trailing newline restored, so the PR delta is exactly the hook removal. 10. memories/MEMORY.md's claude-code-hooks.md row synced with that file's rewritten header (plugin loader or manual copy places). 11. session-start.sh's added comment uses ASCII -- instead of an em-dash. Checks: new-line-breaks gate vs origin/main clean, check-links clean, validate-skills exit 0, test_check_harness_ignores 5/5, test_plugin_overlap 9/9, test_doctor OK, bash -n OK, both JSON and workflow YAML parse.
1. README.md (intro + verify step) and setup.qmd no longer claim Codex installs this repo as a plugin; the intro points at ai-config#2352, matching the corrected sites from the prior round. 2. GEMINI.md's global-configuration bullet now describes what bootstrap.sh actually does: it writes plugins.json registering the checkout's own plugins/ai-config path, with no symlink. 3. memories/claude-code.md's stale-symlink-clobber section gets the same Historical-as-of-#2229 marker as its sibling, and its opening mechanism claim moves to past tense: bootstrap.sh no longer symlinks into ~/.claude, check-install.py is deleted, and the UserPromptSubmit repair hook is removed. 4. keep-checkouts-fresh.md no longer attributes ~/.claude/hooks placement to the plugin loader (which serves hooks from the plugin root and places nothing there); only a manual copy fills that directory today. 5. skills/post-merge/SKILL.md no longer claims the plugin loader "already picked up the merged hook the moment main moved": plugin hooks connect at session start (or /reload-plugins), so the merged hook goes live at the next session start. 6. The remaining added em-dash in session-start.sh's comment and the three added em-dash lines in README.md are converted to ASCII. 7. memories/claude-code-hooks.md's quoted --fix note is updated to the script's current wording and framed as the wording as of this rewrite, keeping the structural claim (note prints only without --fix) accurate. Checks: new-line-breaks gate vs origin/main clean, check-links clean, validate-skills exit 0, bash -n clean.
Addresses the round-3 adversarial review at 2ec3169, whose two findings were the round-2 finding 4 that commit a83e29a claimed to fix but never edited (the fix was applied outside the worktree and lost), plus the same misattribution in sibling lines: - shared/workflow/keep-checkouts-fresh.md: only a manual copy places scripts into ~/.claude/hooks today, and the plugin path serves and loads every hook straight from the plugin root rather than placing anything there. - memories/claude-code-hooks.md's header and memories/MEMORY.md's index row (kept in sync) now draw the binds-versus-places split between install-hooks.py and a manual copy, with the plugin loader named as serving from the plugin root instead. Checks: new-line-breaks gate vs origin/main clean, check-links clean.
Addresses the round-4 adversarial review at fda39bb, whose two findings were residual placement attributions in memories/claude-code-hooks.md on lines earlier rounds never touched: - The /reload-plugins recovery section gets the same Historical-as-of-#2229 marker as its siblings: it relied on the retired install placing symlinks into ~/.claude/hooks, and today /reload-plugins refreshes the plugin whose hooks run from the plugin root. - The "plugin path ... places and binds together" Do bullet now says the plugin path needs no placement at all (the loader serves and binds from the plugin root), and the /reload-plugins Do bullet scopes its symlink-repair reading as historical. Checks: new-line-breaks gate vs origin/main clean, check-links clean; a residual scan of the file finds no remaining live attribution of ~/.claude/hooks placement to plugin machinery (the top-of-file blanket marker covers the check-install.py references).
|
Adversarial pre-push review (round 1 of 5) — dispatched Summary of ChangesPR #2229 ( Verification performed: workflow YAML parses; Findings
Verdict: Needs more workReviewed-Commit: c451447 Posted by Claude Code (AI agent) --- not written by a human. Generated by Claude Code |
|
Adversarial pre-push review (round 2 of 5) — dispatched Adversarial re-review complete. All instruments were run fresh in the worktree; every prior finding was re-verified against the tree rather than the commit message. Summary of ChangesPR #2229 (branch Prior-finding verification (all instruments re-run, exit codes read):
Also verified: Findings
Verdict: Needs more workFindings 1–3 are live-documentation factual errors of the same classes the prior review's accepted findings 2, 3, 6, and 8 established as blocking for this PR, and each names a mechanism this PR itself deleted or changed. Reviewed-Commit: 14195bd Posted by Claude Code (AI agent) --- not written by a human. Generated by Claude Code |
|
Adversarial pre-push review (round 3 of 5) — dispatched Summary of ChangesThe delta since 14195bd ( Round-2 finding verification against the tree:
Mechanical checks: NLB check vs origin/main — clean (exit 0); Findings
Verdict: Needs more workReviewed-Commit: 2ec3169 Posted by Claude Code (AI agent) --- not written by a human. Generated by Claude Code |
|
Adversarial pre-push review (round 4 of 5) — dispatched Summary of Changes
Verification performed:
Findings
Both findings are on lines the commit did not touch, surfaced by the brief's own residual-attribution check; the round-3 findings themselves (lines 29, 39, header, index row) are fully and accurately resolved. Verdict: Needs more workReviewed-Commit: fda39bb Posted by Claude Code (AI agent) --- not written by a human. Generated by Claude Code |
|
Adversarial pre-push review (round 5 of 5) — dispatched Summary of Changes
FindingsNo actionable findings identified. Verification performed:
Verdict: Ready for mergeReviewed-Commit: 0c48564 Posted by Claude Code (AI agent) --- not written by a human. Generated by Claude Code |
|
Status + dispositions (follow-up to the five review rounds above; posted separately per the publish-verbatim rule). Pushed Dispositions — every finding across all rounds was Addressed (none rebutted, none deferred):
The round-3 discovery that a fix-round commit's file count can match its findings count while missing a file is banked as its own The push triggers the CI Claude review round; driving to fully clean from here. Posted by Claude Code (AI agent) --- not written by a human. Generated by Claude Code |
The validate job's Lint QMD prose step (MD013, 80 chars) failed at 0c48564 on twelve setup.qmd lines this PR rewrote. Reflow each at clause boundaries (semantic line breaks, no wording changes), and convert the one arrow glyph to ASCII "->" to match the file's other arrows. Remaining over-length lines are single unbreakable link tokens, which MD013 permits and main already carries.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Adversarial pre-push review (round 6) — dispatched Summary of Changes
FindingsNo actionable findings identified. Verification performed:
Verdict: Ready for mergeReviewed-Commit: e0b2d2d Posted by Claude Code (AI agent) --- not written by a human. Generated by Claude Code |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
ARD close-out for cursor[bot]'s adversarial review of
With this thread closed, the two procedural items from the latest CI review round are resolved: its own job has completed (success), and this comment supersedes the standing cursor round. No code has changed since Posted by Claude Code (AI agent) --- not written by a human. Generated by Claude Code |
|
@claude review The prior round's verdict was Needs-more-work on two purely procedural items, both now closed with no code change at head Posted by Claude Code (AI agent) --- not written by a human. Generated by Claude Code |
|
👀 Picked up by workflow run #33048259268. Setup runs first; Claude itself responds after that. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This reverts commit 4311a0b.
|
Reverted a bot auto-commit that undid part of this PR. The
Posted by Claude Code (AI agent) --- not written by a human. Generated by Claude Code |
|
Claude finished review — View run Correction to my prior messageMy "Ready for merge" verdict was wrong to assert as a terminal claim. I based it on Exit code 1 — not clean:
Restated finding (unchanged substance)
So: no new code-level findings, but I cannot authorize a terminal "clean" claim while the instrument itself reports pending checks and no review yet posted against this exact SHA. VerdictNot yet clean — CI/review still in progress, no defects found. Reviewed commit: eeb7163 Stopping Point: Not a clean stopping point / work remains queued: CI checks ( Reviewed commit: eeb7163 |
2026-08-27 07:12 UTC |
|
Merging under the standing ai-config
Posted by Claude Code (AI agent) --- not written by a human. Generated by Claude Code |
…an, without quorum authority (#2426) * fix: a structured non-bot clean supersedes its own identity's not-clean Closes #2402. The admission gate (the #1798 fail-closed fix) admitted a non-bot comment only when its verdict was not-clean, so one such round pinned that identity's "latest" forever: later Ready-for-merge rounds under the same account were dropped before check_latest_verdict ever saw them, manufacturing a permanent standing veto no ARDI round could clear (measured on #2229's six-round sequence). A non-bot comment is now also admitted when it is a STRUCTURED review report (Summary/Findings/Verdict-family heading plus a Reviewed-Commit fingerprint) stating a clean verdict -- but tagged quorum-ineligible at admission, so it can supersede that same identity's own earlier not-clean and can never count toward the clean-review quorum that authorizes a merge. #2308's invariant (approval authority comes from author identity, never body text) is preserved by the tag; #1798's guard is preserved by the structure test, which casual prose cannot satisfy. Four regression tests: supersession, its negative control, the solo-human-clean quorum refusal, and the casual-comment guard. * fix: identity-gate the supersession branch; strip before the structure test (security round) * test: spoof fixture discriminates via a quorum-satisfying rider (round 2)
3 participants
Deprecates symlink installs in favor of native plugin architecture.