fix(cli): correct hub push --fail-if-modified comparison and extend to control docs

Addresses review feedback on the strict-push comparison:

- Normalise the local document the same way Hub stores it (rewriting the
  $id to the latest version and applying default fields via
  updateDocumentMetadata) before comparing, so a version-only difference
  (e.g. another committer auto-bumped Hub) or an absent `description` no
  longer reports a false "has changed".
- Guard against an empty/unreadable latest-version body: surface a clear
  "could not read" error instead of silently reporting "has changed".
- Point users at --change-type in the modified-document error message.
- Extend --fail-if-modified to `control-requirement` and
  `control-configuration` push (same normalise-and-compare model).
- Add Commander argv wiring tests so the flag plumbing is covered.

Claude-Session: https://claude.ai/code/session_01QiHAHdi4d4mCoy4wku5s2v

Author: willosborne

cli/AGENTS.md

@@ -59,7 +59,7 @@ npm run copy-ai-tools         # Copy AI agent files from ../calm-ai/
 9. **hub** - Command **group** for interacting with CALM Hub. Subcommands:
    - `hub push <resource> <file>` / `hub pull <resource>` / `hub list <resources>` / `hub create <resource>`
    - Resources span architectures, patterns, standards, control-requirements, control-configurations, namespaces, and domains (which subcommands exist varies per verb).
-   - `hub push architecture|pattern|standard` auto-bumps by default (creates a new version off the latest). The `--fail-if-modified` flag switches to a strict, non-bumping mode: a brand-new mapping is still created at `1.0.0`, an unchanged document is skipped, and a document that differs from the latest published version fails the push. Content comparison uses `canonicalEqual` from `@finos/calm-shared` (key-order-insensitive).
+   - `hub push` auto-bumps by default (creates a new version off the latest). The `--fail-if-modified` flag (supported on `architecture`, `pattern`, `standard`, `control-requirement` and `control-configuration`) switches to a strict, non-bumping mode: a brand-new mapping is still created at `1.0.0`, an unchanged document is skipped, and a document that differs from the latest published version fails the push. The local document is normalised the same way Hub stores it (via `updateDocumentMetadata`/`updateControlDocumentMetadata`) before comparing with `canonicalEqual` from `@finos/calm-shared`, so a version-only or defaulted-field difference is not mistaken for a content change.
 
 ### Important Directories
 ```

cli/src/cli.spec.ts

@@ -784,6 +784,31 @@ describe('CLI Commands', () => {
             }));
         });
 
+        it('passes --fail-if-modified through to the architecture handler', async () => {
+            await program.parseAsync([
+                'node', 'cli.js', 'hub', 'push', 'architecture',
+                'arch.json',
+                '--calm-hub-url', 'http://hub',
+                '--fail-if-modified',
+            ]);
+
+            expect(hubCommandsModule.runPushArchitecture).toHaveBeenCalledWith(expect.objectContaining({
+                failIfModified: true,
+            }));
+        });
+
+        it('defaults --fail-if-modified to false when not provided', async () => {
+            await program.parseAsync([
+                'node', 'cli.js', 'hub', 'push', 'architecture',
+                'arch.json',
+                '--calm-hub-url', 'http://hub',
+            ]);
+
+            expect(hubCommandsModule.runPushArchitecture).toHaveBeenCalledWith(expect.objectContaining({
+                failIfModified: false,
+            }));
+        });
+
         it('passes --format pretty through to the handler', async () => {
             await program.parseAsync([
                 'node', 'cli.js', 'hub', 'push', 'architecture',
@@ -1087,6 +1112,18 @@ describe('CLI Commands', () => {
                 changeType: 'PATCH',
             }));
         });
+
+        it('passes --fail-if-modified through to the handler', async () => {
+            await program.parseAsync([
+                'node', 'cli.js', 'hub', 'push', 'control-requirement', 'req.json',
+                '--calm-hub-url', 'http://hub',
+                '--fail-if-modified',
+            ]);
+
+            expect(hubCommandsModule.runPushControlRequirement).toHaveBeenCalledWith(expect.objectContaining({
+                failIfModified: true,
+            }));
+        });
     });
 
     describe('pull control-requirement command', () => {

cli/src/cli.ts

@@ -543,12 +543,14 @@ Example:
         .option(CALMHUB_URL_OPTION, 'URL to CALMHub instance')
         .addOption(hubOutputOption)
         .addOption(hubVersionBumpOption)
+        .addOption(hubFailIfModifiedOption)
         .action(async (requirementFile, options) => {
             const pushOptions: PushControlOptions = {
                 calmHubOptions: { calmHubUrl: options.calmHubUrl },
                 file: requirementFile,
                 format: options.format,
-                changeType: options.changeType.toUpperCase() as ResourceChangeType
+                changeType: options.changeType.toUpperCase() as ResourceChangeType,
+                failIfModified: options.failIfModified
             };
             await runPushControlRequirement(pushOptions);
         });
@@ -559,12 +561,14 @@ Example:
         .option(CALMHUB_URL_OPTION, 'URL to CALMHub instance')
         .addOption(hubOutputOption)
         .addOption(hubVersionBumpOption)
+        .addOption(hubFailIfModifiedOption)
         .action(async (configFile, options) => {
             const pushOptions: PushControlOptions = {
                 calmHubOptions: { calmHubUrl: options.calmHubUrl },
                 file: configFile,
                 format: options.format,
-                changeType: options.changeType.toUpperCase() as ResourceChangeType
+                changeType: options.changeType.toUpperCase() as ResourceChangeType,
+                failIfModified: options.failIfModified
             };
             await runPushControlConfiguration(pushOptions);
         });

cli/src/command-helpers/hub-commands.spec.ts

@@ -342,8 +342,15 @@ describe('hub-commands', () => {
             it('skips (does not create a new version) when content is unchanged from the latest published version', async () => {
                 const { mockClient } = await getSharedMocks();
                 vi.mocked(mockClient.getMappedResourceVersions).mockResolvedValue(['1.0.0']);
-                // Latest published version is byte-identical to the local document (key order ignored).
-                vi.mocked(mockClient.getMappedResourceByVersion).mockResolvedValue(JSON.parse(pushDoc('my-arch')));
+                // The stored document is the local doc as Hub normalises it on the way in: $id at the
+                // latest version and a defaulted empty description. The local file (pushDoc) has no
+                // description and is only compared after the same normalisation, so this must match.
+                vi.mocked(mockClient.getMappedResourceByVersion).mockResolvedValue({
+                    $id: 'http://hub/calm/namespaces/finos/architectures/my-arch/versions/1.0.0',
+                    title: 'my-arch',
+                    description: '',
+                    nodes: []
+                });
 
                 await runPushArchitecture({
                     calmHubOptions: { calmHubUrl: 'http://hub' },
@@ -359,6 +366,48 @@ describe('hub-commands', () => {
                 );
             });
 
+            it('does not flag a version-only difference (auto-bumped elsewhere) as modified', async () => {
+                const { mockClient } = await getSharedMocks();
+                // Hub has moved on to 1.1.0; the local file still declares 1.0.0 but the architecture
+                // content is identical. Normalising to the latest version must make these compare equal.
+                vi.mocked(mockClient.getMappedResourceVersions).mockResolvedValue(['1.0.0', '1.1.0']);
+                vi.mocked(mockClient.getMappedResourceByVersion).mockResolvedValue({
+                    $id: 'http://hub/calm/namespaces/finos/architectures/my-arch/versions/1.1.0',
+                    title: 'my-arch',
+                    description: '',
+                    nodes: []
+                });
+
+                await runPushArchitecture({
+                    calmHubOptions: { calmHubUrl: 'http://hub' },
+                    file: 'arch.json',
+                    failIfModified: true
+                });
+
+                expect(mockClient.getMappedResourceByVersion).toHaveBeenCalledWith('finos', 'my-arch', '1.1.0', 'architectures');
+                expect(mockClient.createMappedResourceVersion).not.toHaveBeenCalled();
+                expect(hubOutput.printJsonSuccess).toHaveBeenCalledWith(
+                    expect.objectContaining({ status: 'skipped', version: '1.1.0' })
+                );
+            });
+
+            it('fails clearly when Hub returns an empty body for the latest version', async () => {
+                const { mockClient } = await getSharedMocks();
+                vi.mocked(mockClient.getMappedResourceVersions).mockResolvedValue(['1.0.0']);
+                vi.mocked(mockClient.getMappedResourceByVersion).mockResolvedValue(undefined as unknown as object);
+
+                await expect(runPushArchitecture({
+                    calmHubOptions: { calmHubUrl: 'http://hub' },
+                    file: 'arch.json',
+                    failIfModified: true
+                })).rejects.toThrow('process.exit');
+
+                expect(mockClient.createMappedResourceVersion).not.toHaveBeenCalled();
+                expect(hubOutput.printError).toHaveBeenCalledWith(
+                    0, expect.stringContaining('Could not read the latest published version'), expect.any(String), 'json'
+                );
+            });
+
             it('fails when the document has changed relative to the latest published version', async () => {
                 const { mockClient } = await getSharedMocks();
                 vi.mocked(mockClient.getMappedResourceVersions).mockResolvedValue(['1.0.0']);
@@ -1186,6 +1235,52 @@ describe('hub-commands', () => {
                 .rejects.toThrow('process.exit');
             expect(hubOutput.printError).toHaveBeenCalledWith(409, 'Version already exists', expect.any(String), 'json');
         });
+
+        describe('--fail-if-modified', () => {
+            it('creates 1.0.0 for a brand-new requirement even with the flag set', async () => {
+                const { mockClient } = await getSharedMocks();
+                vi.mocked(fs.readFile).mockResolvedValue(controlReqDoc() as unknown as Uint8Array);
+                vi.mocked(mockClient.getControlRequirementVersions).mockResolvedValue([]);
+                vi.mocked(mockClient.createControlRequirementVersion).mockResolvedValue(reqId('1.0.0'));
+
+                await runPushControlRequirement({ calmHubOptions: { calmHubUrl: 'http://hub' }, file: 'req.json', failIfModified: true });
+
+                expect(mockClient.getControlRequirementVersion).not.toHaveBeenCalled();
+                expect(mockClient.createControlRequirementVersion).toHaveBeenCalledWith('security', 'access-control', '1.0.0', expect.any(String));
+            });
+
+            it('skips when the requirement is unchanged from the latest published version', async () => {
+                const { mockClient } = await getSharedMocks();
+                vi.mocked(fs.readFile).mockResolvedValue(controlReqDoc() as unknown as Uint8Array);
+                vi.mocked(mockClient.getControlRequirementVersions).mockResolvedValue(['1.0.0']);
+                // Stored doc normalised to its latest version (only the $id version differs from disk).
+                vi.mocked(mockClient.getControlRequirementVersion).mockResolvedValue({ $id: reqId('1.0.0'), nodes: [] });
+
+                await runPushControlRequirement({ calmHubOptions: { calmHubUrl: 'http://hub' }, file: 'req.json', failIfModified: true });
+
+                expect(mockClient.getControlRequirementVersion).toHaveBeenCalledWith('security', 'access-control', '1.0.0');
+                expect(mockClient.createControlRequirementVersion).not.toHaveBeenCalled();
+                expect(fs.writeFile).not.toHaveBeenCalled();
+                expect(hubOutput.printJsonSuccess).toHaveBeenCalledWith(
+                    expect.objectContaining({ status: 'skipped', version: '1.0.0' })
+                );
+            });
+
+            it('fails when the requirement has changed relative to the latest published version', async () => {
+                const { mockClient } = await getSharedMocks();
+                vi.mocked(fs.readFile).mockResolvedValue(controlReqDoc() as unknown as Uint8Array);
+                vi.mocked(mockClient.getControlRequirementVersions).mockResolvedValue(['1.0.0']);
+                vi.mocked(mockClient.getControlRequirementVersion).mockResolvedValue({ $id: reqId('1.0.0'), nodes: [{ 'unique-id': 'changed' }] });
+
+                await expect(runPushControlRequirement({ calmHubOptions: { calmHubUrl: 'http://hub' }, file: 'req.json', failIfModified: true }))
+                    .rejects.toThrow('process.exit');
+
+                expect(mockClient.createControlRequirementVersion).not.toHaveBeenCalled();
+                expect(hubOutput.printError).toHaveBeenCalledWith(
+                    0, expect.stringContaining('has changed relative to the latest published version'), expect.any(String), 'json'
+                );
+            });
+        });
     });
 
     // ── runPushControlConfiguration ────────────────────────────────────────────
@@ -1240,6 +1335,38 @@ describe('hub-commands', () => {
                 0, expect.stringContaining('describes a control requirement, but a control configuration was expected'), expect.any(String), 'json'
             );
         });
+
+        describe('--fail-if-modified', () => {
+            it('skips when the configuration is unchanged from the latest published version', async () => {
+                const { mockClient } = await getSharedMocks();
+                vi.mocked(fs.readFile).mockResolvedValue(controlConfigDoc() as unknown as Uint8Array);
+                vi.mocked(mockClient.getControlConfigurationVersions).mockResolvedValue(['1.0.0']);
+                vi.mocked(mockClient.getControlConfigurationVersion).mockResolvedValue({ $id: cfgId('1.0.0'), nodes: [] });
+
+                await runPushControlConfiguration({ calmHubOptions: { calmHubUrl: 'http://hub' }, file: 'config.json', failIfModified: true });
+
+                expect(mockClient.getControlConfigurationVersion).toHaveBeenCalledWith('security', 'access-control', 'prod', '1.0.0');
+                expect(mockClient.createControlConfigurationVersion).not.toHaveBeenCalled();
+                expect(hubOutput.printJsonSuccess).toHaveBeenCalledWith(
+                    expect.objectContaining({ status: 'skipped', configName: 'prod', version: '1.0.0' })
+                );
+            });
+
+            it('fails when the configuration has changed relative to the latest published version', async () => {
+                const { mockClient } = await getSharedMocks();
+                vi.mocked(fs.readFile).mockResolvedValue(controlConfigDoc() as unknown as Uint8Array);
+                vi.mocked(mockClient.getControlConfigurationVersions).mockResolvedValue(['1.0.0']);
+                vi.mocked(mockClient.getControlConfigurationVersion).mockResolvedValue({ $id: cfgId('1.0.0'), nodes: [{ 'unique-id': 'changed' }] });
+
+                await expect(runPushControlConfiguration({ calmHubOptions: { calmHubUrl: 'http://hub' }, file: 'config.json', failIfModified: true }))
+                    .rejects.toThrow('process.exit');
+
+                expect(mockClient.createControlConfigurationVersion).not.toHaveBeenCalled();
+                expect(hubOutput.printError).toHaveBeenCalledWith(
+                    0, expect.stringContaining('has changed relative to the latest published version'), expect.any(String), 'json'
+                );
+            });
+        });
     });
 
     // ── runPullControlRequirement ──────────────────────────────────────────────