Skip to content

Commit f1a4ac0

Browse files
authored
Audit cleanup: concurrency, error handling, DRY pass (#407)
* Audit cleanup: concurrency, error handling, DRY pass Cross-package code-review pass addressing high- and medium-severity findings across the SDK, CLI, MCP server, VS extension, and mrt-utilities. Concurrency / correctness: - SDK: atomic write for the stateful auth-session file (temp + rename). - SDK: single-flight refresh in OAuthStrategy.getAccessToken so concurrent callers coalesce instead of stampeding the AM token endpoint. - SDK: clean up debug-session keepalive/poll timers when connect() fails after starting them. - SDK: try/finally around progress timers in code download/deploy so aborted requests can no longer leak intervals. - CLI: long-running commands (code:watch, logs:tail, mrt:tail-logs) deregister their SIGINT/SIGTERM handlers when finished. - VS ext: guard webview postMessage against panel disposal in the Swagger API Browser; drain pending uploads before tearing down code- sync watchers; prevent sandbox-tree polling stop-checks from stacking when the polling interval is shorter than the stabilization window. - mrt-utilities: pipeToDestination now destroys the destination on pipeline error so consumers fail fast instead of hanging. - MCP: registerToolsets() throws clearly if invoked twice for the same server; telemetry send failures are logged at debug. Error handling: - Replaced silent catch {} blocks in stateful-store unlinkSync, job:run afterOperation hooks, logs:tail signal handler, and MCP telemetry with debug-level logging. - CIP commands now stream output through ux.stdout instead of process.stdout, restoring --json and test capture; CIP tests use runSilent. DRY / dead code: - New @salesforce/b2c-tooling-sdk/ux export hosts the canonical confirm() prompt; CLI's prompts.ts is now a thin re-export. - New auth/jwt-utils consolidates JWT exp/scope decoding previously duplicated across three auth strategies. - AM list commands share an amPageSizeFlag definition. - formatApiError SLAS/SCAPI variants now alias the SDK's getApiErrorMessage; ECDN gets a single requireScapiCoordinates helper. - Removed deprecated LocalSourceResult re-export. Lint, typecheck, and the full test suite (4,075 tests) pass. * Test-quality cleanup: tighten assertions, add expectError helper Companion to the audit cleanup. Addresses every High and the highest- leverage Medium items from TEST_AUDIT_PLAN.md so the test suite catches real regressions instead of just confirming functions ran. CLI: - expectError(fn, match?) helper added to test/helpers/test-setup.ts; replaces the verbose try { await x; expect.fail(...) } catch {} pattern that could swallow the wrong error type. - code/deploy: tighten .calledOnce checks to verify args (instance, cartridges, code version) and afterOperation hook payload. - code/activate: assert PATCH path/body and reload toggle order (active → alternate → active) instead of just call counts. - auth/token: assert returned JSON shape and that ux.stdout was called with exactly the access token, not just .calledOnce. - mrt/env/var/push: positively assert that listEnvVars ran when asserting setBatchStub was *not* called. - cip/query: drop echo tautologies (result.sql === input); assert that the resolved SQL was passed to mockClient.query. - Promote sinon.stub(odsClient) and makeCommandThrowOnError out of 12 duplicated copies in sandbox tests into the canonical helpers. - Switch .to.equal(true|false) → .to.be.true|false where present. SDK: - logger.test.ts: drop 18 redundant `expect(logger).to.exist` lines (the followup `.to.be.a('function')` was the real check). MCP: - registry: new smoke test invokes a registered handler (sfnext_get_guidelines) end-to-end; previously only tool names were asserted. - theming-store: 25 weak `expect(guidance).to.exist` calls upgraded to `expect(guidance, '...').to.not.be.undefined` so failures point at the missing key. - figma generate-component: replace 7 `array.some(d => …).to.equal(true)` chains with `array.find(...)` + concrete property assertions so failures show what actually went wrong. mrt-utilities: - create-lambda-adapter-compression.test.ts: replace 39 fixed `setTimeout(50ms)` waits with the existing event-driven `stream.waitForEnd()`. Suite runtime drops from ~3s to ~1s. All 4,076 tests pass (+1 from the new MCP smoke test). Lint and typecheck clean.
1 parent b947888 commit f1a4ac0

76 files changed

Lines changed: 844 additions & 652 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.changeset/audit-cleanup-cli.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@salesforce/b2c-cli': patch
3+
---
4+
5+
CLI cleanup and correctness fixes:
6+
7+
- `b2c cip query`, `cip describe`, `cip tables`, and `cip report *` now stream output through oclif's `ux.stdout` instead of writing directly to `process.stdout`. This restores the `--json` flag and makes output capturable by tests and CI.
8+
- Long-running commands (`code:watch`, `logs:tail`, `mrt:tail-logs`) now deregister their SIGINT/SIGTERM handlers when finished, so re-invocations no longer stack handlers on the same process.
9+
- Hook and signal-handler errors that were previously swallowed (`job:run` afterOperation hooks, `logs:tail` stop, `setup:ide:prophet` console fallbacks) now log at debug instead of disappearing.
10+
- AM list commands (`am clients|roles|users list`) share a single `amPageSizeFlag` definition.
11+
- Removed deprecated `LocalSourceResult` re-export.

.changeset/audit-cleanup-mcp.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@salesforce/b2c-dx-mcp': patch
3+
---
4+
5+
- Telemetry send failures are no longer silently swallowed; they now log at debug level so deployment-monitoring drift is visible behind the `--debug` flag.
6+
- `registerToolsets()` throws a clear error if invoked more than once for the same server instance (instead of producing a cryptic duplicate-tool error from the SDK).

.changeset/audit-cleanup-mrt.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@salesforce/mrt-utilities': patch
3+
---
4+
5+
- The Lambda response adapter's `pipeToDestination` now destroys the destination stream when the underlying pipeline rejects, so consumers fail fast instead of hanging.
6+
- `pipedDestinations` cleanup is unified between the success and error paths.

.changeset/audit-cleanup-sdk.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@salesforce/b2c-tooling-sdk': patch
3+
---
4+
5+
Hardened auth and long-running operation paths:
6+
7+
- Token store now writes atomically (temp file + rename) so concurrent CLI invocations cannot corrupt `auth-session.json`.
8+
- `OAuthStrategy.getAccessToken()` coalesces concurrent refreshes onto a single in-flight request, preventing token-endpoint stampedes.
9+
- Debug session cleans up its keepalive/poll timers if `connect()` fails after starting them.
10+
- `downloadCartridges` and `deployCartridges` use try/finally around progress timers so an aborted or failing request can no longer leak intervals.
11+
- New `@salesforce/b2c-tooling-sdk/ux` export surfaces the canonical `confirm()` prompt; CLI re-exports from here.
12+
- New `auth/jwt-utils` consolidates JWT `exp`/`scope` decoding previously duplicated across three auth strategies.
13+
- Better error message when the implicit-OAuth port is already in use (suggests `SFCC_OAUTH_LOCAL_PORT`).

.changeset/audit-cleanup-vsx.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'b2c-vs-extension': patch
3+
---
4+
5+
VS Code extension reliability fixes:
6+
7+
- Swagger API Browser webview no longer attempts `postMessage` after the panel has been disposed (previously could throw on token refresh or proxy responses arriving after close).
8+
- Sandbox tree polling no longer stacks "stop-check" timers when the configured polling interval is shorter than the 3-second stabilization window.
9+
- Code Sync now drains pending uploads/deletes before tearing down its file watchers, so saves immediately preceding a stop are no longer dropped.

packages/b2c-cli/src/commands/am/clients/list.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {Flags, Errors} from '@oclif/core';
77
import {AmCommand, TableRenderer, type ColumnDef} from '@salesforce/b2c-tooling-sdk/cli';
88
import type {AccountManagerApiClient, APIClientCollection} from '@salesforce/b2c-tooling-sdk';
99
import {t} from '../../../i18n/index.js';
10+
import {amPageSizeFlag} from '../../../utils/am/flags.js';
1011

1112
/** Format date as MM/DD/YYYY HH:MM:SS with zero-padding for equal column width. */
1213
function formatDateEqualLength(value: Date | number | string): string {
@@ -75,10 +76,7 @@ export default class ClientList extends AmCommand<typeof ClientList> {
7576
];
7677

7778
static flags = {
78-
size: Flags.integer({
79-
char: 's',
80-
description: 'Page size (default: 20, min: 1, max: 4000)',
81-
}),
79+
size: amPageSizeFlag,
8280
page: Flags.integer({
8381
description: 'Page number (zero-based index, default: 0, min: 0)',
8482
}),

packages/b2c-cli/src/commands/am/roles/list.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {Flags, Errors} from '@oclif/core';
77
import {AmCommand, TableRenderer, type ColumnDef} from '@salesforce/b2c-tooling-sdk/cli';
88
import type {AccountManagerRole, RoleCollection} from '@salesforce/b2c-tooling-sdk';
99
import {t} from '../../../i18n/index.js';
10+
import {amPageSizeFlag} from '../../../utils/am/flags.js';
1011

1112
const COLUMNS: Record<string, ColumnDef<AccountManagerRole>> = {
1213
id: {
@@ -64,10 +65,7 @@ export default class RoleList extends AmCommand<typeof RoleList> {
6465
];
6566

6667
static flags = {
67-
size: Flags.integer({
68-
char: 's',
69-
description: 'Page size (default: 20, min: 1, max: 4000)',
70-
}),
68+
size: amPageSizeFlag,
7169
page: Flags.integer({
7270
description: 'Page number (zero-based index, default: 0, min: 0)',
7371
}),

packages/b2c-cli/src/commands/am/users/list.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {Flags, Errors} from '@oclif/core';
77
import {AmCommand, TableRenderer, type ColumnDef} from '@salesforce/b2c-tooling-sdk/cli';
88
import type {AccountManagerUser, UserCollection} from '@salesforce/b2c-tooling-sdk';
99
import {t} from '../../../i18n/index.js';
10+
import {amPageSizeFlag} from '../../../utils/am/flags.js';
1011

1112
const COLUMNS: Record<string, ColumnDef<AccountManagerUser>> = {
1213
mail: {
@@ -89,10 +90,7 @@ export default class UserList extends AmCommand<typeof UserList> {
8990
];
9091

9192
static flags = {
92-
size: Flags.integer({
93-
char: 's',
94-
description: 'Page size (default: 20, min: 1, max: 4000)',
95-
}),
93+
size: amPageSizeFlag,
9694
page: Flags.integer({
9795
description: 'Page number (zero-based index, default: 0, min: 0)',
9896
}),

packages/b2c-cli/src/commands/cip/describe.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {Args, Flags} from '@oclif/core';
88
import {describeCipTable} from '@salesforce/b2c-tooling-sdk';
99
import {withDocs} from '../../i18n/index.js';
1010
import {CipCommand} from '../../utils/cip/command.js';
11-
import {renderTable, toCsv, type CipOutputFormat} from '../../utils/cip/format.js';
11+
import {renderTable, writeCsv, writeJson, type CipOutputFormat} from '../../utils/cip/format.js';
1212

1313
const {from: _unusedFrom, to: _unusedTo, ...cipMetadataFlags} = CipCommand.baseFlags;
1414

@@ -77,7 +77,7 @@ export default class CipDescribe extends CipCommand<typeof CipDescribe> {
7777
}
7878

7979
if (this.flags.format === 'json') {
80-
process.stdout.write(`${JSON.stringify(output, null, 2)}\n`);
80+
writeJson(output);
8181
return output;
8282
}
8383

@@ -88,7 +88,7 @@ export default class CipDescribe extends CipCommand<typeof CipDescribe> {
8888
private renderRows(rows: CipDescribeCommandResult['columns'], format: CipOutputFormat): void {
8989
const columns = ['column', 'dataType', 'isNullable'];
9090
if (format === 'csv') {
91-
process.stdout.write(`${toCsv(columns, rows)}\n`);
91+
writeCsv(columns, rows);
9292
return;
9393
}
9494

packages/b2c-cli/src/commands/cip/query.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import fs from 'node:fs';
88
import {Args, Flags} from '@oclif/core';
99
import {withDocs} from '../../i18n/index.js';
1010
import {CipCommand} from '../../utils/cip/command.js';
11-
import {renderTable, toCsv, type CipOutputFormat} from '../../utils/cip/format.js';
11+
import {renderTable, writeCsv, writeJson, type CipOutputFormat} from '../../utils/cip/format.js';
1212

1313
interface CipQueryCommandResult {
1414
columns: string[];
@@ -56,7 +56,7 @@ export default class CipQuery extends CipCommand<typeof CipQuery> {
5656
}
5757

5858
if (this.flags.format === 'json') {
59-
process.stdout.write(`${JSON.stringify(output, null, 2)}\n`);
59+
writeJson(output);
6060
return output;
6161
}
6262

@@ -66,7 +66,7 @@ export default class CipQuery extends CipCommand<typeof CipQuery> {
6666

6767
private renderRows(columns: string[], rows: Array<Record<string, unknown>>, format: CipOutputFormat): void {
6868
if (format === 'csv') {
69-
process.stdout.write(`${toCsv(columns, rows)}\n`);
69+
writeCsv(columns, rows);
7070
return;
7171
}
7272

0 commit comments

Comments
 (0)