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

CHANGELOG.MD

@@ -8,6 +8,8 @@
 
 ## Fixed
 
+- `playbook submit` now resolves a standard's rule ids before submitting, so removing or editing a rule is applied rather than silently dropped when submitted with `--no-review`
+
 ## Removed
 
 # [1.18.0] - 2026-09-22

apps/cli/src/application/utils/artifactComparison.spec.ts

@@ -7,6 +7,10 @@ import {
   compareStandardFields,
 } from './artifactComparison';
 
+jest.mock('../../infra/utils/consoleLogger', () => ({
+  logWarningConsole: jest.fn(),
+}));
+
 const PACKMIND_PATH = '.packmind/standards/my-standard.md';
 
 function buildStandard(opts: {
@@ -178,6 +182,92 @@ describe('compareStandardFields', () => {
     );
   });
 
+  describe('when rule ids are supplied', () => {
+    const DELETED_RULE = 'Completely unique rule xyz';
+
+    function deletionChanges(ruleIds?: ReadonlyMap<string, string>) {
+      const local = buildStandard({ rules: ['Rule one'] });
+      const deployed = buildStandard({ rules: ['Rule one', DELETED_RULE] });
+      return compareStandardFields(local, deployed, PACKMIND_PATH, ruleIds);
+    }
+
+    it('targets a deleted rule by its server id', () => {
+      const changes = deletionChanges(
+        new Map([
+          ['Rule one', 'rule-1'],
+          [DELETED_RULE, 'rule-2'],
+        ]),
+      );
+      expect(changes).toContainEqual(
+        expect.objectContaining({
+          type: ChangeProposalType.deleteRule,
+          payload: expect.objectContaining({ targetId: 'rule-2' }),
+        }),
+      );
+    });
+
+    it('carries the server id on the deleted rule item', () => {
+      const changes = deletionChanges(new Map([[DELETED_RULE, 'rule-2']]));
+      expect(changes).toContainEqual(
+        expect.objectContaining({
+          type: ChangeProposalType.deleteRule,
+          payload: expect.objectContaining({
+            item: { id: 'rule-2', content: DELETED_RULE },
+          }),
+        }),
+      );
+    });
+
+    it('targets an updated rule by the id of the content it replaces', () => {
+      const local = buildStandard({
+        rules: ['Use camelCase for all variable names'],
+      });
+      const deployed = buildStandard({
+        rules: ['Use camelCase for variable names'],
+      });
+      const changes = compareStandardFields(
+        local,
+        deployed,
+        PACKMIND_PATH,
+        new Map([['Use camelCase for variable names', 'rule-9']]),
+      );
+      expect(changes).toContainEqual(
+        expect.objectContaining({
+          type: ChangeProposalType.updateRule,
+          payload: expect.objectContaining({ targetId: 'rule-9' }),
+        }),
+      );
+    });
+
+    describe('when the deleted rule is absent from the supplied ids', () => {
+      it('falls back to the unresolved placeholder', () => {
+        const changes = deletionChanges(new Map([['Rule one', 'rule-1']]));
+        expect(changes).toContainEqual(
+          expect.objectContaining({
+            type: ChangeProposalType.deleteRule,
+            payload: expect.objectContaining({ targetId: 'unresolved' }),
+          }),
+        );
+      });
+    });
+  });
+
+  describe('when no rule ids are supplied', () => {
+    it('falls back to the unresolved placeholder', () => {
+      const local = buildStandard({ rules: ['Rule one'] });
+      const deployed = buildStandard({
+        rules: ['Rule one', 'Completely unique rule xyz'],
+      });
+      const changes = compareStandardFields(local, deployed, PACKMIND_PATH);
+      expect(changes).toContainEqual(
+        expect.objectContaining({
+          type: ChangeProposalType.deleteRule,
+          payload: expect.objectContaining({ targetId: 'unresolved' }),
+        }),
+      );
+    });
+  });
+
   describe('when parsing fails', () => {
     it('returns empty array', () => {
       const result = compareStandardFields(

apps/cli/src/application/utils/artifactComparison.ts

@@ -3,11 +3,21 @@ import {
   ChangeProposalType,
   canonicalJsonStringify,
   createRuleId,
+  RuleId,
 } from '@packmind/types';
 
 import { parseCommandFile } from './parseCommandFile';
 import { parseStandardMd } from './parseStandardMd';
 import { matchUpdatedRules } from './ruleSimilarity';
+import { logWarningConsole } from '../../infra/utils/consoleLogger';
+
+const RULE_LABEL_MAX_LENGTH = 60;
+
+function truncateRule(content: string): string {
+  return content.length > RULE_LABEL_MAX_LENGTH
+    ? `${content.slice(0, RULE_LABEL_MAX_LENGTH)}...`
+    : content;
+}
 
 export type FieldChange = {
   type: ChangeProposalType;
@@ -25,10 +35,28 @@ export type SkillDefinitionInput = {
   additionalProperties?: Record<string, unknown>;
 };
 
+/**
+ * Maps a deployed rule's content to its server-side id.
+ *
+ * `deleteRule` and `updateRule` proposals are applied by matching
+ * `payload.targetId` against the rule's real id, so a proposal built without
+ * one silently applies to nothing. The caller fetches the ids; passing the map
+ * is optional so that callers which only display a diff stay synchronous.
+ */
+export type RuleIdsByContent = ReadonlyMap<string, string>;
+
+/**
+ * Stands in for a rule id that could not be resolved. It matches no rule, so a
+ * proposal carrying it is applied to nothing; callers that asked for ids treat
+ * its presence as a failure rather than shipping it.
+ */
+export const UNRESOLVED_RULE_ID = 'unresolved';
+
 export function compareStandardFields(
   localContent: string,
   deployedContent: string,
   filePath: string,
+  ruleIdsByContent?: RuleIdsByContent,
 ): FieldChange[] {
   const localParsed = parseStandardMd(localContent, filePath);
   const serverParsed = parseStandardMd(deployedContent, filePath);
@@ -105,8 +133,24 @@ export function compareStandardFields(
     addedRules,
   );
 
+  // The placeholder kept for callers that supply no ids matches no rule, so
+  // the change it names is applied to nothing. A caller that did supply ids
+  // and still missed one is warned, rather than left to read a success line
+  // for a change that was dropped.
+  const resolveRuleId = (content: string, changeLabel: string): RuleId => {
+    const resolved = ruleIdsByContent?.get(content);
+    if (resolved) return createRuleId(resolved);
+
+    if (ruleIdsByContent) {
+      logWarningConsole(
+        `Could not resolve the rule "${truncateRule(content)}" in ${filePath} to a known rule; its ${changeLabel} may not be applied.`,
+      );
+    }
+    return createRuleId(UNRESOLVED_RULE_ID);
+  };
+
   for (const update of updates) {
-    const ruleId = createRuleId('unresolved');
+    const ruleId = resolveRuleId(update.oldValue, 'update');
     changes.push({
       type: ChangeProposalType.updateRule,
       payload: {
@@ -118,7 +162,7 @@ export function compareStandardFields(
   }
 
   for (const rule of remainingDeleted) {
-    const ruleId = createRuleId('unresolved');
+    const ruleId = resolveRuleId(rule, 'removal');
     changes.push({
       type: ChangeProposalType.deleteRule,
       payload: {