🐛 fix(cli): resolve rule ids before submitting a standard

[987poiuytrewq]

Explanation

Removing or editing a rule in a standard and submitting it with --no-review reports success and changes nothing on the server. The rule is still there afterwards, and nothing retries it.

compareStandardFields stamps every deleteRule and updateRule proposal with a placeholder id:

for (const rule of remainingDeleted) {
  const ruleId = createRuleId('unresolved');
  changes.push({
    type: ChangeProposalType.deleteRule,
    payload: { targetId: ruleId, item: { id: ruleId, content: rule } },
  });
}

createRuleId is brandedIdFactory(), which returns (id) => id, so targetId is the literal string 'unresolved'. The local Markdown carries no rule ids, so there was nothing else to put there.

StandardChangeProposalApplier resolves both change types by id:

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

The placeholder matches no rule, so the filter removes nothing and updateRule's map rewrites nothing. On the --no-review path, where playbookSubmitHandler sends proposals straight to batchApply, that is the whole story:

1. batchApply returns success, because the standard itself was touched.
2. The handler prints 1 standard updated from collectParts(response.updated).
3. It then clears the staged change, so the local playbook forgets the deletion.

The rule survives, the operator is told it did not, and there is nothing left locally to retry. Rule additions are unaffected, which is why this reads as "some edits don't stick" rather than as a broken command.

The fix

The CLI already had the missing piece. StandardsGateway.getRules returns { id, content }[] for a standard and was called from nowhere in the codebase. buildProposals now takes an optional fetcher, reads a standard's rules once, and hands compareStandardFields a content-to-id map. No server change and no new endpoint.

Two things left deliberately alone:

• StandardDiffStrategy keeps the placeholder. It feeds playbook diff, which only prints; no id is ever dereferenced there. Adding a network call to a display command seemed a poor trade.
• An unmatched rule still falls back to the placeholder. A lookup that fails or comes back incomplete then behaves exactly as it does today rather than blocking a submit — but it now warns, instead of leaving a success line to speak for a change that was dropped.

Type of Change

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

Affected Components

• Domain packages affected: apps/cli only (application/utils/artifactComparison.ts, infra/commands/playbook/submit/proposalBuilder.ts, infra/commands/playbook/submitHandler.ts)
• Frontend / Backend / Both: neither — CLI
• Breaking changes (if any): none. The new parameters are optional and every existing call site keeps its current behaviour.

Testing

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

Test Details:

Ten tests added across the two specs.

artifactComparison.spec.ts — compareStandardFields:

• targets a deleted rule by its server id
• carries the server id on the deleted rule item
• targets an updated rule by the id of the content it replaces
• falls back to the placeholder when the deleted rule is absent from the supplied ids
• falls back to the placeholder when no ids are supplied

proposalBuilder.spec.ts — buildProposals:

• targets a deleted rule by the id fetched from the standard
• fetches the rules with the space and artifact of the standard
• fetches a standard's rules once when several agents render it
• still builds the proposals when the rule lookup fails
• falls back to the placeholder when no fetcher is provided

The six that assert the new behaviour were confirmed to fail against main with only the specs applied, so they pin the behaviour rather than the implementation. The fallback tests pass either way, by design.

Verified against a live instance. Both CLIs were run against the same standard in a throwaway space, deleting the same rule with --no-review, pulling afterwards to read the server's own state back:

	reported	rule after pull
0.35.1 as published	1 standard updated	still there
this branch	1 standard updated	gone

Editing a rule was checked the same way and also lands on this branch, which matters because matchUpdatedRules turns a reworded rule into an updateRule carrying the same placeholder. The fixture was restored to its published state afterwards — by another --no-review deletion, which is its own small proof.

nx test packmind-cli — 126 suites, 3487 tests, all passing. nx typecheck packmind-cli clean. nx lint packmind-cli reports the same 13 pre-existing warnings as main, none in the changed files. prettier --check clean.

TODO List

• CHANGELOG Updated
• Documentation Updated

Reviewer Notes

This is not scoped to --no-review. buildProposals runs before the noReview branch, so the resolved ids now reach batchCreate as well as batchApply. That is deliberate, but it is the part I would most like a second opinion on, because I could not establish what the review path does with targetId today.

What the public code shows:

• StandardChangesApplier extends StandardChangeProposalApplier, so the review path ends in the same rules.filter((rule) => rule.id !== changeProposal.payload.targetId).
• That branch reads changeProposal.payload directly rather than through getEffectivePayload, so a reviewer's decision does not supply the id either.
• There is no content-based rule lookup anywhere in this repository, and StandardChangeProposalApplier.spec.ts supplies a real ruleId in its deleteRule fixtures.

Taken alone that suggests a CLI-originated rule deletion could never have applied on either path, which does not match what we observe — deletions do land through review. So batchCreate most likely resolves targetId when the proposal is created. That code is in the closed edition (the OSS PlaybookChangeManagementAdapter throws Method not implemented for every method), so this is inference, not something I could read.

Either way the change should be safe: if creation resolves the id, a correct id arriving pre-resolved is at worst redundant; if it does not, this repairs the review path too. One argument in favour of ids over content matching on this path specifically: a review proposal can sit pending for days, and saveNewVersion passes rule ids through to updateStandard, so an id survives someone rewording that rule in the UI where a content key would not.

The check I would ask of someone with access: submit a rule removal through review and look at the stored proposal's payload.targetId. If it is the literal string unresolved, the review path has the same defect and this fixes both.

Two smaller notes:

• matchUpdatedRules pairs a delete with an add into an updateRule, so a merely reworded rule takes the same path and was equally affected.
• The fetcher is keyed on spaceId/standardId and cached for the lifetime of one buildProposals call, so a standard rendered for several agents costs one request.

🤖 Generated with Claude Code

[987poiuytrewq]

Superseded by PackmindHub#515, which carries the same branch to upstream for review. Closing so there is a single place to review; the branch stays on this fork as the PR head.