fix(claude-ops): stop sync writing committed settings, restoring the invariant

[kyle-sexton]

Summary

SKILL.md states the rule the skill is built around:

converge is the one action that can touch a committed .claude/settings.json, and only after an
explicit per-plugin confirm.

sync.md Step 5 broke it. It issued claude plugin enable <id> -s project for every
currentProject: true completeness gap, and 0.31.8 verified on Claude Code 2.1.228 that this call
writes exactly that file. So the default, headless-capable action silently modified a team-shared
tracked file. 0.31.8 documented the exposure and asked the report to name it — a mitigation, not a
fix — and in the interim the invariant itself had been softened in SKILL.md and converge.md to
describe the violation rather than remove it.

The fix. Step 5 now enables automatically only where the write is not team-shared state:

• user — machine-scope ~/.claude/settings.json.
• local — the gitignored .claude/settings.local.json (verified in claude-ops plugins skill: enable -s project writes committed settings, and a subdirectory install is invisible to currentProject #2526).
• project — never enabled automatically. Reported as an "Action needed" row carrying the
  runnable command, in the cd-into-its-own-projectPath form converge.md mandates and with the
  fully-qualified id gotchas.md requires, so acting on it is a copy rather than a reconstruction.

Why not confirm instead of skip. converge can afford a confirm gate because it aborts in an
autonomous session — CLAUDE_CODE_REMOTE, /loop, /schedule. sync has no such abort by design;
it is the on-demand and headless maintenance action, and /claude-ops:lanes runs it from scheduled
background sessions. A path that writes team-shared state cannot be made safe by asking when there
may be nobody to answer. The write must not happen.

After this change no sync path writes a committed settings file, so SKILL.md and converge.md
state the invariant plainly again instead of carving out sync.

Test plan

• markdownlint-cli2 on all changed markdown — 0 issues.
• lychee --offline — 0 errors.
• check-skill.sh on plugins/claude-ops/skills/plugins — PASS, 0 errors, 0 warnings.
• scripts/affected-tests.sh --run — every changed file is a recorded no-suite class.
• plugin.json parses; version 0.31.9 → 0.31.10.
• Consistency sweep: grepped the skill tree for any surviving claim that sync writes committed
  settings — none remain. The 0.31.8 "Known exposure" paragraph is gone, and the two softened
  invariant statements are restored.

Related

Closes #2535

Completes the remediation deferred in #2526 / #2527, which documented this exposure but left it
unscheduled. Underlying CLI behavior is recorded upstream in anthropics/claude-code#81058.

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c41c73bccb

ℹ️ 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".

[claude]

Claude finished @kyle-sexton's task in 2m 4s —— View job

---

Security review

Scope: /review:security-review — logic, trust-boundary, and Actions security findings only (docs/config, no code diffs).

Result: No security issues found

This PR is doc-only (SKILL.md, converge.md, scope-semantics.md, sync.md, CHANGELOG.md, plugin.json version bump) and its entire purpose is to tighten a trust boundary: it stops sync — the on-demand/headless action with no autonomous-session abort — from ever calling claude plugin enable <id> -s project, the one call that writes a team-shared, git-tracked .claude/settings.json (confirmed by scope-semantics.md's empirically-verified call table at scope-semantics.md#L59-L84).

Checked specifically:

• Grepped every -s project occurrence across sync.md/converge.md/SKILL.md. The only -s project call sync still auto-executes is plugin update (sync.md#L92), which the same doc's own verified table confirms does not write committed settings. The enable -s project command at sync.md#L194 is emitted as inert report text for a human to run deliberately, never executed by the skill itself — so the write-without-confirmation path is genuinely closed.
• converge's autonomous-session abort gate (converge.md#L8-L15) — the only remaining path that can touch committed settings — is unmodified by this PR and still fails closed when session-type is ambiguous.
• No shell-injection surface: the only unexecuted placeholder command ((cd "<projectPath>" && claude plugin enable <id>@<marketplace> -s project)) is presented as documentation text, not built into anything this skill runs, and projectPath/<id> originate from the local installed_plugins.json record (same-machine install state), not attacker-influenced input.
• No workflow/permissions changes, no secrets handling, no dependency/action pins touched.

Note (out of scope for this lane): Codex's review already flagged a real logic bug at sync.md#L182-L199 — for an id installed at both user and project scope with no raw entry in either, Step 5 auto-enables user scope first, which (per scope-semantics.md#L82-L84) makes the merged effective value true, so the reported enable -s project remediation command will then fail with "already enabled at project scope" instead of writing the project entry. I confirmed this against the doc's own stated CLI semantics and it's accurate — but it fails safe (no unauthorized write occurs; the committed file is simply never updated), so it's a correctness/usability issue for /review:code-review, not a security finding for this lane.

[github-actions]

Last security-reviewed head: 78a2cf77cc21cb83748286973afa1b8e5162b1f3. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[claude]

Claude finished @kyle-sexton's task in 2m 24s —— View job

---

Code review complete

• Gather context (PR diff, changed files, custom instructions)
• Run /review:code-review criteria against the diff
• Investigate Codex's P2 finding on sync.md:194 (dual-scope remediation command)
• Post findings (inline + summary)

Scope: reviewed the 6 changed files (plugin.json, CHANGELOG.md, SKILL.md, converge.md, scope-semantics.md, sync.md) against origin/main per the code-review lane's criteria (correctness + maintainability, security out of scope for this lane).

Finding posted inline

One correctness issue, posted as an inline comment on sync.md:182-194:

Independently confirmed Codex's P2 flag on the same lines. For a plugin id installed at both user and project scope with no raw enabledPlugins entry in either, Step 5 auto-enables the user row first, then reports the project row's enable -s project command. Per this PR's own scope-semantics.md:82-84, enable -s project gates on the merged effective value and fails once the id is already true at user scope — so the reported "runnable" remediation command is guaranteed to fail exactly in this dual-scope case, contradicting the PR's stated goal that the row is "a copy rather than a reconstruction."

Everything else checked out

• The core fix is sound: Step 5's user/local auto-enable vs. project report-only split correctly stops sync from writing team-shared state, matching the empirical CLI facts already recorded in scope-semantics.md (enable -s project writes the committed file; -s user/-s local do not).
• converge.md and SKILL.md's invariant language is restored to state plainly that converge is the only action touching committed settings — verified no other file in the skill tree still claims sync can write one (grepped for "writes committed"/"write committed"/"Known exposure").
• The project-scope report row uses the fully-qualified <id>@<marketplace> form and the cd-into-projectPath form, consistent with gotchas.md's fully-qualified-id requirement and converge.md's -s project-has-no-path-flag rationale — good, except for the dual-scope ordering bug above.
• plugin.json version bump (0.31.10 → 0.31.11) matches the new CHANGELOG.md entry.
• Note: the PR body's test-plan items (markdownlint, lychee, check-skill.sh, affected-tests.sh) are author-claimed; I did not independently re-run them.

[github-actions]

Claude has reviewed this PR 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @kyle-sexton's task in 1m 19s —— View job

---

Security review

Scope: /review:security-review — logic, trust-boundary, and Actions security findings only (docs/config, no code diffs). Reviewed at HEAD 78a2cf7.

Result: No security issues found

This PR remains doc-only (SKILL.md, converge.md, scope-semantics.md, sync.md, CHANGELOG.md, plugin.json version bump), and its purpose continues to be tightening a trust boundary: sync never calls claude plugin enable <id> -s project — the one call that writes a team-shared, git-tracked .claude/settings.json.

The prior review of this PR (posted at head df05d59) found no security issues. Since then, one new commit was pushed — 78a2cf7 ("suppress the project-scope enable row the user branch would break") — which I reviewed specifically:

• It only touches sync.md and SKILL.md, adding prose guidance that suppresses the "Action needed" project-scope report row when the same run already auto-enabled the id at user/local scope (this is the fix for the dual-scope ordering bug both Codex and the prior code-review pass flagged).
• The change is purely descriptive/logic guidance consumed by the skill's LLM-driven steps — no new command is introduced, no existing command becomes auto-executed, and the -s project write path remains report-only, never run by the skill itself.
• No workflow, permissions, secrets, or dependency-pin changes.

Confirms the write-without-confirmation path stays closed, and this incremental commit doesn't introduce any new security-relevant surface.

Note (out of scope for this lane): the correctness fix itself (suppressing the project row) is a /review:code-review concern, already addressed by the commit — not re-litigated here since it fails safe either way (no unauthorized write in either case).

[claude]

Claude finished @kyle-sexton's task in 2m 8s —— View job

---

Code review complete

• Gather context (PR diff, changed files, custom instructions)
• Invoke /review:code-review criteria against the diff
• Verify the previously-flagged P2 (dual-scope remediation command) against the current HEAD
• Post findings

Scope: re-reviewed all 6 changed files at HEAD 78a2cf7 against origin/main's prior state, focused on correctness + maintainability (security already covered by a separate lane on this PR).

Result: no outstanding findings

This push resolves the one issue raised in earlier review rounds. Both Codex and my own prior pass flagged that sync.md could report a project-scope enable -s project remediation command that was guaranteed to fail — for an id installed at both user and project scope with no raw enabledPlugins entry anywhere, Step 5's user-scope auto-enable flips the merged effective value, so a subsequently reported -s project command would error with "already enabled at project scope."

The current diff adds exactly the fix that gap needed, at sync.md#L202-L213:

Order matters — suppress this row for any id the user/local branch just enabled. ... Emit the project row only for an id this step did not enable at user or local scope

I verified this against scope-semantics.md#L82-L84 (the merged-effective-gating fact both reviews cited) and it correctly covers both branches that can flip the merged value — user and local — not just user, so the local+project dual-install case is covered too.

Rest of the sweep:

• Grepped the whole skill tree for writes committed/write committed/Known exposure — no stale claim that sync can touch a committed file survives (grep output).
• SKILL.md's report format (SKILL.md#L108-L122) and sync.md Step 5/6 agree on when a project-scope row appears and its exact command shape.
• converge.md's restored "only action that can rewrite committed settings" language is consistent with the rest of the tree, and its own logic is untouched by this PR.
• gotchas.md references (fully-qualified id, CLAUDE_PROJECT_DIR resolution) check out against the actual file content.
• plugin.json version (0.31.11) matches the new CHANGELOG.md head entry.

No inline comments filed — nothing new to flag.