Uh oh!
There was an error while loading. Please reload this page.
silent verify script - #282
Conversation
Reviewer's GuideIntroduce a new silent verification runner subsystem under @cellix/local-dev, wire it into the root verify workflow, extend local-dev’s public API and documentation, and refine console noise from the serenity AgentFormatter, alongside several dependency override updates. Sequence diagram for the new silent verify workflowsequenceDiagram
actor Dev
participant verify_ts as verify_ts
participant verificationSequence as VerificationSequence
participant silentRunners as runSilentCommandSequence
participant childProcess as spawnSync
Dev->>verify_ts: runVerifyCommand()
verify_ts->>verificationSequence: addStep(pnpmScript('format:check'))
verify_ts->>verificationSequence: addStep(architectureTests())
verify_ts->>verificationSequence: run()
verificationSequence->>silentRunners: runSilentCommandSequence(options)
loop for each step
silentRunners->>childProcess: spawnSync(command, args, options)
childProcess-->>silentRunners: SpawnSyncReturns
alt status === 0
silentRunners-->>silentRunners: suppress stdout/stderr
else status !== 0
silentRunners-->>silentRunners: replayFailureOutput(result, streams)
silentRunners-->>verificationSequence: SilentCommandSequenceResult(failing step)
verificationSequence-->>verify_ts: result
verify_ts-->>Dev: process.exitCode = result.status
end
end
silentRunners-->>verificationSequence: SilentCommandSequenceResult(success)
verificationSequence-->>verify_ts: result
verify_ts-->>Dev: print "verify passed" and set process.exitCode
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
AgentFormatter.onTestRunFinishedyou now return early whenissueCount === 0, which removes the previous "All scenarios passed" messaging entirely; if that feedback is still desired, consider keeping a minimal success summary instead of returning silently. - The
verify.tsscript prints a custom failure line afterrunSilentCommandSequencehas already replayed the failing step's stdout/stderr; consider including the step exit status in that replay or adjusting the extra message to avoid redundant-but-slightly-different failure context.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments- In `AgentFormatter.onTestRunFinished` you now return early when `issueCount === 0`, which removes the previous "All scenarios passed" messaging entirely; if that feedback is still desired, consider keeping a minimal success summary instead of returning silently.
- The `verify.ts` script prints a custom failure line after `runSilentCommandSequence` has already replayed the failing step's stdout/stderr; consider including the step exit status in that replay or adjusting the extra message to avoid redundant-but-slightly-different failure context.
## Individual Comments### Comment 1
<locationpath="build-pipeline/scripts/verify.ts"line_range="29-38" />
<code_context>
+ .addStep(sonarPullRequestAnalysis())
+ .addStep(sonarQualityGate());
+
+const isRunningAsCommand = import.meta.main;
+
+function runVerifyCommand(): void {
+ const result = cellixVerify.run();
+ if (result.status !== 0) {
+process.stderr.write(`\nverify failed at step "${result.step.name}" (exit ${result.status})\n`);+ } else {
+process.stdout.write('verify passed\n');+ }
+ process.exitCode = result.status;
+}
+
+if (isRunningAsCommand) {
+ runVerifyCommand();
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `import.meta.main` will never trigger in Node.js and prevents the script from running when executed directly.
In Node.js, `import.meta.main` is always `undefined`, so `runVerifyCommand()` never runs, even when this file is invoked as the entry script (e.g. via `pnpm run verify`). Use a Node-compatible check instead, such as comparing `import.meta.url` to `pathToFileURL(process.argv[1]).href`, or invoke `runVerifyCommand()` from a dedicated bootstrap file that always calls it.
</issue_to_address>
### Comment 2
<locationpath="package.json"line_range="52" />
<code_context>
"sonar:pr-windows": "for /f %i in ('node build-pipeline/scripts/get-pr-number.cjs') do set PR_NUMBER=%i && sonar-scanner -Dsonar.pullrequest.key=%PR_NUMBER% -Dsonar.pullrequest.branch=%BRANCH_NAME% -Dsonar.pullrequest.base=main",
"check-sonar": "node build-pipeline/scripts/check-sonar-quality-gate.cjs",
-"verify": "pnpm run format:check && pnpm run test:arch && pnpm run test:coverage:merge && pnpm run test:e2e && pnpm run knip && pnpm run audit && pnpm run snyk && pnpm run sonar:pr && pnpm run check-sonar",+"verify": "node build-pipeline/scripts/verify.ts",
"knip": "knip",
"snyk": "pnpm run snyk:test && pnpm run snyk:code",
</code_context>
<issue_to_address>
**issue (bug_risk):** Running a `.ts` file directly with `node` will fail without a custom loader or precompiled JS.
This script now calls `node build-pipeline/scripts/verify.ts`, but Node can’t run TypeScript directly. Unless there’s an existing loader configured for this command (e.g. `tsx`, `ts-node`, or a custom `--loader`), it will fail at runtime. Consider either targeting the compiled `.js` file or running it via the configured TS runner instead of plain `node`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
jasonmorais
commented
Jun 22, 2026
jasonmorais
commented
Jun 22, 2026
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments### Comment 1
<locationpath="build-pipeline/scripts/verify.ts"line_range="29-38" />
<code_context>
+ .addStep(sonarPullRequestAnalysis())
+ .addStep(sonarQualityGate());
+
+const isRunningAsCommand = import.meta.main;
+
+function runVerifyCommand(): void {
+ const result = cellixVerify.run();
+ if (result.status !== 0) {
+process.stderr.write(`\nverify failed at step "${result.step.name}" (exit ${result.status})\n`);+ } else {
+process.stdout.write('verify passed\n');+ }
+ process.exitCode = result.status;
+}
+
+if (isRunningAsCommand) {
+ runVerifyCommand();
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `import.meta.main` is likely non-portable and may prevent the verify script from running when invoked directly.
In Node’s current ESM implementation `import.meta.main` is not available, so `isRunningAsCommand` will be `undefined` and `runVerifyCommand()` will never run, even when you call `node build-pipeline/scripts/verify.ts` directly. To ensure the script actually executes as a CLI, switch to the common pattern of comparing `import.meta.url` with `pathToFileURL(process.argv[1]).href` (or another supported entry-point check).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Uh oh!
There was an error while loading. Please reload this page.
jasonmorais
commented
Jun 22, 2026
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
verify.tsscript printsverify passedon success, which slightly conflicts with the goal of keeping successful verification runs silent; consider making this success message optional (e.g., behind a flag) or removing it to keep the workflow fully quiet when healthy. - The
runSilentCommandhelper hardcodes a 64MBmaxBuffer; if you expect some tools to produce larger output, consider exposingmaxBuffer(or a similar option) onSilentCommandOptionsso callers can tune this per command. - The construction of the
cellixVerifysequence inverify.tsis fairly specific and could be reused elsewhere; consider extracting this builder into a small shared helper in@cellix/local-dev(or a nearby module) so local/CI verification flows can share the same sequence definition without duplication.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments- The new `verify.ts` script prints `verify passed` on success, which slightly conflicts with the goal of keeping successful verification runs silent; consider making this success message optional (e.g., behind a flag) or removing it to keep the workflow fully quiet when healthy.
- The `runSilentCommand` helper hardcodes a 64MB `maxBuffer`; if you expect some tools to produce larger output, consider exposing `maxBuffer` (or a similar option) on `SilentCommandOptions` so callers can tune this per command.
- The construction of the `cellixVerify` sequence in `verify.ts` is fairly specific and could be reused elsewhere; consider extracting this builder into a small shared helper in `@cellix/local-dev` (or a nearby module) so local/CI verification flows can share the same sequence definition without duplication.
## Individual Comments### Comment 1
<locationpath="packages/cellix/local-dev/src/silent-runners/index.ts"line_range="177-186" />
<code_context>
+*@param options - Sequence steps and optional process context.
+*@returns The final successful step result or the first failing step result.
+*/
+export function runSilentCommandSequence(options: SilentCommandSequenceOptions): SilentCommandSequenceResult {
+ const { cwd, env, spawn, steps, streams } = options;
+ if (steps.length === 0) {
+throw new Error('runSilentCommandSequence requires at least one step');+ }
++ let lastResult: SilentCommandSequenceResult | undefined;
++ for (const step of steps) {
+const context: CommandSequenceContext = { cwd, env, spawn, streams };+const result = step.output === 'inherit' ? runInheritedCommand(buildCommandOptions(step, context)) : runSilentCommand(buildCommandOptions(step, context));+lastResult = {+ ...result,+ step,+};++if (result.status !== 0) {+ return lastResult;+}+ }
++ if (!lastResult) {
+throw new Error('runSilentCommandSequence requires at least one step');+ }
</code_context>
<issue_to_address>
**nitpick:** The `runSilentCommandSequence` empty-steps guard is duplicated and could be simplified.
The function throws `runSilentCommandSequence requires at least one step` both on the initial `steps.length === 0` check and again when `!lastResult` after the loop. Because of the initial check, the post-loop guard is unreachable and can be removed, or you can drop the initial check and rely solely on the `lastResult` check to cover the empty-steps case.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Uh oh!
There was an error while loading. Please reload this page.
# Conflicts: # pnpm-lock.yaml # pnpm-workspace.yaml
jasonmorais
commented
Jun 23, 2026
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new verify runner currently writes
verify passedon success, which slightly conflicts with the "silent on success" contract of the silent runners; consider omitting this message so local runs stay fully quiet when healthy. - In
replayFailureOutput, the stderr replay concatenates the failure banner and the tool stderr without ensuring a trailing newline from the tool output; consider always terminating the combined message with a newline to avoid malformed prompts when the tool's stderr is not newline-terminated. - The Snyk helper
snykIacScantakestargetsas the first argument whereas the other Snyk helpers accept only an options object; consider making its signature consistent (e.g.,snykIacScan({ targets, args, name })) to keep the public API uniform.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments- The new verify runner currently writes `verify passed` on success, which slightly conflicts with the "silent on success" contract of the silent runners; consider omitting this message so local runs stay fully quiet when healthy.
- In `replayFailureOutput`, the stderr replay concatenates the failure banner and the tool stderr without ensuring a trailing newline from the tool output; consider always terminating the combined message with a newline to avoid malformed prompts when the tool's stderr is not newline-terminated.
- The Snyk helper `snykIacScan` takes `targets` as the first argument whereas the other Snyk helpers accept only an options object; consider making its signature consistent (e.g., `snykIacScan({ targets, args, name })`) to keep the public API uniform.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
# Conflicts: # package.json # packages/cellix/serenity-framework/src/formatters/agent-formatter.ts
# Conflicts: # package.json
jasonmorais
commented
Jul 2, 2026
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
Fixed security issues:
@opentelemetry/exporter-prometheus (link)
@vitest/browser (link)
form-data (link)
protobufjs (link)
vite (link)
ws (link)
Some of the step-definition and helper changes collapse previously multi-line logical conditions and
actor.attemptsTocalls into single very long lines, which hurts readability; consider keeping these broken across lines to preserve clarity and easier diffs.For
runSilentCommandSequencefailures you currently only show the raw command line inreplayFailureOutput; consider also including theCommandSequenceStep.nameso failures are easier to correlate with the higher-level verify steps (e.g.snyk:code,audit:prod).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments- Some of the step-definition and helper changes collapse previously multi-line logical conditions and `actor.attemptsTo` calls into single very long lines, which hurts readability; consider keeping these broken across lines to preserve clarity and easier diffs.
- For `runSilentCommandSequence` failures you currently only show the raw command line in `replayFailureOutput`; consider also including the `CommandSequenceStep.name` so failures are easier to correlate with the higher-level verify steps (e.g. `snyk:code`, `audit:prod`).
## Individual Comments### Comment 1
<locationpath="packages/cellix/local-dev/src/silent-runners/index.ts"line_range="283-293" />
<code_context>
+...(step.args ? { args: step.args } : {}),+...(options.cwd ? { cwd: options.cwd } : {}),+...(options.env ? { env: options.env } : {}),+...(step.maxBuffer !== undefined ? { maxBuffer: step.maxBuffer } : options.maxBuffer !== undefined ? { maxBuffer: options.maxBuffer } : {}),+...(options.spawn ? { spawn: options.spawn } : {}),+...(options.streams ? { streams: options.streams } : {}),
</code_context>
<issue_to_address>
**suggestion:** Nested ternary for resolving maxBuffer hurts readability and is easy to misinterpret.
Consider replacing this nested ternary with clearer logic, e.g. compute `const maxBuffer = step.maxBuffer ?? options.maxBuffer;` and then conditionally spread `{ maxBuffer }`, or use straightforward `if` branches. This makes the precedence explicit and reduces the chance of mistakes if more options are added later.
```suggestionfunction buildCommandOptions(step: CommandSequenceStep, options: CommandSequenceContext): SilentCommandOptions { const maxBuffer = step.maxBuffer ?? options.maxBuffer; return { ...(step.args ? { args: step.args } : {}), ...(options.cwd ? { cwd: options.cwd } : {}), ...(options.env ? { env: options.env } : {}), ...(maxBuffer !== undefined ? { maxBuffer } : {}), ...(options.spawn ? { spawn: options.spawn } : {}), ...(options.streams ? { streams: options.streams } : {}), command: step.command, };}```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Uh oh!
There was an error while loading. Please reload this page.
# Conflicts: # pnpm-lock.yaml # pnpm-workspace.yaml
# Conflicts: # packages/cellix/local-dev/README.md # packages/cellix/local-dev/manifest.md # packages/cellix/local-dev/package.json # packages/cellix/local-dev/src/index.test.ts # packages/cellix/local-dev/src/index.ts # packages/cellix/local-dev/tsconfig.vitest.json # packages/cellix/local-dev/vitest.config.ts # pnpm-lock.yaml # pnpm-workspace.yaml
jasonmorais
commented
Jul 10, 2026
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Fixed security issues:
- @opentelemetry/exporter-prometheus (link)
- @vitest/browser (link)
- form-data (link)
- protobufjs (link)
- vite (link)
- ws (link)
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
jasonmorais
commented
Jul 14, 2026
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue
Fixed security issues:
- @opentelemetry/exporter-prometheus (link)
- @vitest/browser (link)
- form-data (link)
- protobufjs (link)
- vite (link)
- ws (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments### Comment 1
<locationpath="packages/cellix/local-dev/src/silent-runners/index.ts"line_range="297-301" />
<code_context>
+ };
+}
+
+function toSilentCommandResult(result: SpawnSyncReturns<string>): SilentCommandResult {
+ return {
+...(result.error ? { error: result.error } : {}),+signal: result.signal,+status: result.status ?? 1,+stderr: result.stderr ?? '',+stdout: result.stdout ?? '',
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Clarify handling of signal-terminated processes so status and messaging stay consistent.
For signal-terminated children, `result.status` is `null` and `result.signal` is set. Forcing `status` to `1` here will cause `replayFailureOutput` to print `exit 1`, losing the signal context and giving a misleading reason for failure. Consider either mapping the signal to a conventional status while still surfacing the signal in the failure output, or leaving `status` null and special‑casing the message (e.g. `Command failed (signal SIGTERM)`).
Suggested implementation:
```typescriptfunction toSilentCommandResult(result:SpawnSyncReturns<string>):SilentCommandResult {
const status =result.status!==null&&result.status!==undefined?result.status:result.signal?null:1;
return {
...(result.error? { error: result.error } : {}),
signal: result.signal,
status,
stderr: result.stderr??'',
stdout: result.stdout??'',
};
}
```
To fully implement the suggestion:
1. Update the `SilentCommandResult` type (where it is defined) so that `status` can be `number | null` instead of just `number`, if it is currently restricted to `number`.
2. In the code that prints or logs failures (e.g. the `replayFailureOutput` helper referenced in your comment), special‑case the messaging:
- If `result.signal` is set and `result.status` is `null`, render something like: `Command failed (signal SIGTERM)` instead of `exit 1`.
- Otherwise, keep the existing `exit <status>` message for non‑signal failures.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Uh oh!
There was an error while loading. Please reload this page.
jasonmorais
commented
Jul 15, 2026
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
Fixed security issues:
@opentelemetry/exporter-prometheus (link)
@vitest/browser (link)
form-data (link)
protobufjs (link)
vite (link)
ws (link)
For
SilentRunnerStreams, consider typing thestdout/stderrfields as a more generalNodeJS.WritableStream(or a custom interface) rather thanPick<NodeJS.WriteStream, 'write'>so non-TTY writable streams can be used without type assertions.runSilentCommandSequenceskipsreplayFailureOutputfor steps withoutput: 'inherit', so failures on those steps don’t get a standardized header or error replay; consider still emitting the failure header (and any spawn error) for inherited-output steps to keep failure reporting consistent across the sequence.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments- For `SilentRunnerStreams`, consider typing the `stdout`/`stderr` fields as a more general `NodeJS.WritableStream` (or a custom interface) rather than `Pick<NodeJS.WriteStream, 'write'>` so non-TTY writable streams can be used without type assertions.
-`runSilentCommandSequence` skips `replayFailureOutput` for steps with `output: 'inherit'`, so failures on those steps don’t get a standardized header or error replay; consider still emitting the failure header (and any spawn error) for inherited-output steps to keep failure reporting consistent across the sequence.
## Individual Comments### Comment 1
<locationpath="packages/cellix/local-dev/src/silent-runners/index.ts"line_range="329-338" />
<code_context>
+ });
+}
+
+function replayFailureOutput(result: SilentCommandResult, streams: SilentRunnerStreams, command: string, args: string[]): void {
+ const commandLine = [command, ...args].join(' ');
+ const reason = result.signal ? `signal ${result.signal}` : `exit ${result.status}`;
+ streams.stderr?.write(`\nCommand failed (${reason}): ${commandLine}\n\n`);
+ if (result.stdout) {
+streams.stdout?.write(result.stdout);+ }
+ if (result.stderr) {
+streams.stderr?.write(result.stderr);+ }
+ if (result.error) {
+streams.stderr?.write(`${result.error.message}\n`);+ }
+}
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Consider avoiding logging the full command line on failure to prevent leaking potentially sensitive arguments.
The failure banner currently logs the full reconstructed command line:
```tsconst commandLine = [command, ...args].join('');
streams.stderr?.write(`\nCommand failed (${reason}): ${commandLine}\n\n`);
```
If any step passes tokens or credentials via `args`, they will be exposed in logs. Consider letting callers provide a safe, redacted display string (e.g. `displayName`/`redactedCommand`) or a flag to omit arguments and only show the executable name, so CI consumers can control how much detail is logged.
Suggested implementation:
```typescriptfunction replayFailureOutput(
result:SilentCommandResult,
streams:SilentRunnerStreams,
command:string,
args:string[],
displayCommand?:string,
logArgumentsInFailureBanner:boolean=false,
):void {
const reason =result.signal?`signal ${result.signal}`:`exit ${result.status}`;
// Prefer an explicit, redacted display string when provided.// Otherwise, optionally include arguments in the banner based on the flag,// defaulting to only the executable name to avoid leaking sensitive data.const bannerCommand =displayCommand??
(logArgumentsInFailureBanner&&args.length>0? [command, ...args].join('')
:command);
streams.stderr?.write(`\nCommand failed (${reason}): ${bannerCommand}\n\n`);
if (result.stdout) {
streams.stdout?.write(result.stdout);
}
if (result.stderr) {
streams.stderr?.write(result.stderr);
}
if (result.error) {
streams.stderr?.write(`${result.error.message}\n`);
}
}
``````typescriptexportinterfaceSilentCommandOptions {
/** Executable name. */
command:string;
/** Command-line arguments passed without shell interpolation. */
args?:string[];
/** * Human-readable, redacted command label to use in logs instead of the raw command/args. * If omitted, logging falls back to either the full command line or just the executable name, * depending on `logArgumentsInFailureBanner`.*/
displayCommand?:string;
/** * Controls whether arguments are included in the failure banner when `displayCommand` is not provided. * Defaults to `false` to avoid leaking sensitive data via arguments.*/
logArgumentsInFailureBanner?:boolean;
/** Working directory for the command. Defaults to the current process cwd. */```To fully implement this behavior, you’ll also need to:1. Update all call sites of `replayFailureOutput` to pass `options.displayCommand` and `options.logArgumentsInFailureBanner` (or leave them undefined to keep the new default of not logging arguments). For example: - Previously: `replayFailureOutput(result, streams, options.command, options.args ?? []);` - Now: `replayFailureOutput(result, streams, options.command, options.args ?? [], options.displayCommand, options.logArgumentsInFailureBanner ?? false);`2. Ensure that code constructing `SilentCommandOptions` in this module (and any consumers) sets `displayCommand` to a redacted/summary form when arguments may contain secrets, or relies on the default behavior (only executable name) by leaving both `displayCommand` and `logArgumentsInFailureBanner` unset.</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Uh oh!
There was an error while loading. Please reload this page.
jasonmorais
commented
Jul 15, 2026
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Fixed security issues:
- @opentelemetry/exporter-prometheus (link)
- @vitest/browser (link)
- form-data (link)
- protobufjs (link)
- vite (link)
- ws (link)
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Summary by Sourcery
Introduce silent verification command runners and wire them into the repo-wide verify workflow while expanding local-dev utilities and docs.
New Features:
Enhancements:
Build:
Tests:
Chores: