🐛 fix(standards): match rule removals and edits by content when the id is unknown

[987poiuytrewq]

Explanation

Follow-up to #515, which @quentinlebourles-packmind rightly declined: the fix belongs on the server, because a CLI fix only reaches whoever upgrades while every installed CLI keeps losing changes. This is that server-side fix. #515 can be closed once this lands.

A client that reads standards from Markdown has no rule ids to send — Markdown carries none. The CLI stamps deleteRule and updateRule proposals with createRuleId('unresolved') and identifies the rule by its text instead, which the payloads already carry:

• deleteRule → CollectionItemRemovePayload, so payload.item.content
• updateRule → payload.oldValue

StandardChangeProposalApplier resolved both by id alone:

const filteredRules = rules.filter(
  (rule) => rule.id !== changeProposal.payload.targetId,
);

The placeholder matches no rule, so the filter removed nothing and updateRule's map rewrote nothing — while the batch still reported the standard as updated and the CLI cleared its staged change. Nothing was left locally to retry.

Both branches now fall back to that text when the id names no rule. The id still wins whenever it resolves, so nothing changes for a client that sends a real one, and no protocol change is needed.

The two fall back differently, on purpose

• A removal takes every rule with that content. The client is saying the standard should no longer carry it, and copies hold no id to tell them apart.
• An edit takes the rule with that content only when exactly one has it. With several there is nothing to choose between them, and rewriting all would collapse them into duplicates.

When the fallback resolves to nothing usable — no rule carries the content, or several do and the change is an edit — it raises ChangeProposalConflictError rather than saving an unchanged version and reporting success. That is the signal applyDiff already uses, and clients already surface it. Only the fallback path raises: a proposal whose targetId resolves behaves exactly as before.

Type of Change

• Bug fix
• New feature
• Improvement/Enhancement
• Refactoring
• Documentation
• Breaking change

Affected Components

• Domain packages affected: packages/types (playbookChangeManagement/applier/StandardChangeProposalApplier.ts) — reached by packages/playbook-change-applier via StandardChangesApplier
• Frontend / Backend / Both: backend
• Breaking changes (if any): none. Behaviour is unchanged whenever targetId resolves; the fallback only runs where the change previously applied to nothing.

Testing

• Unit tests added/updated
• Integration tests added/updated
• Manual testing completed
• Test coverage maintained or improved

Test Details:

Eleven tests added to StandardChangeProposalApplier.spec.ts.

updateRule, when the target id names no rule:

• rewrites the rule carrying the replaced content
• preserves the id of the rule it matched
• throws ChangeProposalConflictError when no rule carries it
• throws when several rules share the content
• matches the rule named by the decision, not by the payload, when a reviewer adjusted it

updateRule, when the target id names a rule:

• ignores another rule sharing the replaced content

deleteRule, when the target id names no rule:

• removes the rule carrying the content the proposal names
• throws ChangeProposalConflictError when no rule carries that content
• removes all of them when several carry that content
• removes the rule named by the decision, not by the payload, when a reviewer adjusted it

deleteRule, when the target id names a rule:

• keeps another rule sharing its content

Eight of the eleven fail against main with only the spec applied. The other three are guards: they pass either way and exist to pin the id path, so the fallback cannot start firing where an id already resolves.

nx run-many -t test,typecheck,lint --projects=types,playbook-change-applier is green (681 + 97 tests). The pre-push hook's nx affected -t lint test build passed across 21 projects.

Verified by hand beforehand, against a live instance and the published @packmind/cli@0.35.1: removing a rule with --no-review reported 1 standard updated and left the rule in place. That is the behaviour this repairs, and it repairs it for that already-published CLI without anyone upgrading.

TODO List

• CHANGELOG Updated
• Documentation Updated

Reviewer Notes

One behaviour change worth your judgement. Re-applying an id-less proposal that already took effect now reports a conflict rather than passing silently, because after the first apply neither the id nor the old content matches anything. If there is a retry path that re-applies accepted proposals, that is where it would show. Small blast radius, since only the CLI produces id-less proposals, but real.

targetId is still matched against the raw payload. That predates this PR, so I have left it: if a decision can carry a targetId differing from the proposal's, the id would be resolved from one and the diff from the other. Happy to fold it in, but it changes how ids resolve, which felt like more than a fallback fix should decide.

Two smaller notes:

• The CLI pairs a delete and an add into an updateRule via matchUpdatedRules when the contents are similar, so a merely reworded rule takes the edit path and was affected in the same way.
• Content is the only identifier available here, so the ambiguity around duplicate rules is not resolvable server-side. A client that can send real ids still gets exact targeting — which is the argument for eventually doing both, with this as the floor that catches every client.

🤖 Generated with Claude Code

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Changes how rule edits and removals match their targets.

The PR appears safe to merge; no outstanding findings remain.

Summary

This PR lets the server apply rule edits and removals from clients that lack rule IDs by matching rule content. It preserves exact-ID targeting and reports a conflict when an id-less proposal cannot be resolved. Both previous Greptile findings are resolved.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Rule edit or removal] --> B{Target ID resolves?}
  B -- Yes --> C[Apply by ID]
  B -- No --> D{Content match}
  D -- Edit: exactly one --> E[Apply edit]
  D -- Removal: one or more --> F[Remove matching rules]
  D -- No usable match --> G[Report conflict]

Loading

_{Reviews (3) · Last reviewed commit: "Merge branch 'main' into fix/resolve-rul..."}

[gitguardian]

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.

🔎 Detected hardcoded secret in your pull request

GitGuardian id	GitGuardian status	Secret	Commit	Filename
37726415	Triggered	Basic Auth String	589edc8	apps/api/src/app/shared/middleware/GitRemoteCredentialsMiddleware.spec.ts	View secret

🛠 Guidelines to remediate hardcoded secrets

1. Understand the implications of revoking this secret by investigating where it is used in your code.
2. Replace and store your secret safely. Learn here the best practices.
3. Revoke and rotate this secret.
4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider

• following these best practices for managing and storing secrets including API keys and other credentials
• install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.

---

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.}

[stevdrouet]

Hi @987poiuytrewq,

Thanks a lot for the PR, it's merged!

Don't hesitate to contribute again.