refactor(cli): migrate off Bun runtime to Node#44
Conversation
|
Warning Review limit reached
More reviews will be available in 51 minutes and 18 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (68)
WalkthroughThis PR migrates the workspace and CLI to pnpm, adds a live browser rendering path and doctor command, and updates shared UI primitives, tables, diagrams, and examples to support new sizing, layout, and styling behavior. ChangesTooling and CLI runtime
Component and example rendering
Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/components/Badge.tsx (1)
432-434:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNullish check regression drops valid
label={0}in non-label variants.Line 433 still uses a truthy guard (
!label) even thoughlabelnow supportsReact.ReactNode. This hides valid values like0(and can misalign label/value rendering). Use a nullish check instead.Proposed fix
- if (!label && !Icon) return null; + if (label == null && !Icon) return null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Badge.tsx` around lines 432 - 434, The renderLabel function currently uses a truthy guard (!label) which hides valid values like 0; update the conditional in renderLabel to use a nullish check (label == null) so that falsy-but-valid nodes (e.g., 0 or empty string) are rendered, keeping the Icon behavior unchanged when both are absent.src/styles.css (1)
415-421:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid forcing left alignment for all
.table-headercolumns.
Line [420](text-align: left) overrides per-column alignment utilities due to higher specificity, so headers likesrc/components/LogoGrid.tsxLines [121-124]won't stay centered. Move alignment control to component classes (or make this rule non-opinionated).Proposed fix
.table-header th { `@apply` font-semibold text-zinc-800; font-size: calc(1em + 1pt); padding: 2mm; - text-align: left; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/styles.css` around lines 415 - 421, The CSS rule ".table-header th" is forcing left alignment via "text-align: left", which overrides per-column alignment utilities (e.g., header cells in LogoGrid.tsx); remove or neutralize that property so alignment is controlled by component-level classes—either delete the "text-align: left" declaration or replace it with a non-opinionated value like "text-align: inherit" on the ".table-header th" selector so per-column utilities can take effect.cli/src/bundler/vite-builder.ts (1)
61-65:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftStop composing the loader command as raw shell text.
Line 142 builds a raw
/bin/sh -ccommand fromsandboxsettings andtsxpath tokens. This is vulnerable to shell metacharacter injection and also breaks valid paths containing spaces. Build argv arrays and execute without a shell.As per coding guidelines, injection risks should be fixed at the root cause (unsafe command construction).
🛠️ Safer argv-based direction
-async function runLoaderOnce(args: LoaderArgs): Promise<{ stderr?: Buffer | string; }> { - const prefix = srtPrefix(args.sandbox); - const tsx = resolveTsxBin(args.facetRoot).join(' '); - return await $`${{ raw: prefix }}${{ raw: tsx }} ${args.loaderPath} --facet-root=${args.facetRoot} --data-file=${args.dataFilePath} --output-file=${args.resultFilePath}`.quiet(); -} +async function runLoaderOnce(args: LoaderArgs): Promise<{ stderr?: Buffer | string; }> { + const tsxArgv = resolveTsxBin(args.facetRoot); + const baseArgv = [ + ...tsxArgv, + args.loaderPath, + `--facet-root=${args.facetRoot}`, + `--data-file=${args.dataFilePath}`, + `--output-file=${args.resultFilePath}`, + ]; + const argv = args.sandbox + ? ['srt', '--settings', typeof args.sandbox === 'string' ? args.sandbox : '/etc/facet/srt-settings.json', ...baseArgv] + : baseArgv; + // Execute argv without shell expansion (spawn/execFile wrapper). + return await runCommandArgv(argv).quiet(); +}Also applies to: 140-143
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/src/bundler/vite-builder.ts` around lines 61 - 65, The code currently composes a shell command string (using srtPrefix and then joining with tsx path tokens) which is vulnerable to shell injection and breaks on paths with spaces; instead construct an argv array and run the binary without a shell: change srtPrefix to return the settings token (or undefined) rather than a shell fragment and in the caller (the code that builds and runs the loader command around the tsx path tokens) assemble an argv like ['srt', '--settings', settings, ...tsxArgs] or ['tsx', tsxPath, ...] as appropriate, then invoke the process using a shell-less API (child_process.spawn or execFile / execa with shell: false) so each argument is passed as a distinct argv element and not interpolated into /bin/sh -c. Ensure you preserve handling of sandbox being boolean or string and include settings only when provided.cli/src/generators/html.ts (1)
65-72:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
clearCachebehavior no longer matches its documented intent.Line 65-72 now clears only
.facet/, so the “node_modules cache” part of the--clear-cachedescription is stale unless that cleanup happens elsewhere.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/src/generators/html.ts` around lines 65 - 72, The clearCache branch currently only removes the .facet directory, which no longer matches the flag’s documented intent to also clear the node_modules cache; update the options.clearCache handling (the block that computes root using consumerRoot || resolve(dirname(options.template))) to also detect and remove the node_modules cache under the same root (e.g., compute nodeModulesDir = join(root, 'node_modules') and if existsSync(nodeModulesDir) then rmSync(nodeModulesDir, { recursive: true, force: true }) and log the removal), or if you prefer to change behavior instead, update the CLI docs/help to remove the “node_modules cache” claim — adjust the code around facetDir and the logger calls accordingly.Taskfile.yml (1)
69-74:⚠️ Potential issue | 🟠 MajorFix
test:unitto use Vitest instead of Bun
Taskfile.yml’stest:unittask still runsbun test(and the description says “with bun”), butcli/package.jsondefines the unit test command asvitest run("test": "vitest run"). Replace thecmds: - bun testentry with the appropriate Vitest invocation (e.g.,pnpm testfromdir: cli) and update thedescaccordingly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Taskfile.yml` around lines 69 - 74, The Taskfile.yml task currently describes and runs Bun but the CLI package uses Vitest; update the task's desc from "Run cli unit tests with bun" to "Run cli unit tests with Vitest" and replace the cmds entry that currently runs "bun test" with the proper Vitest invocation—e.g., run "pnpm test" (keeping the existing dir: cli and deps: gen-version) so the task executes the CLI package.json "test": "vitest run" command.cli/vite-ssr-loader.ts (1)
109-116:⚠️ Potential issue | 🟠 MajorUse
path.delimiterforNODE_PATHincli/vite-ssr-loader.ts.
Line 112 hardcodes:when buildingprocess.env.NODE_PATH; on WindowsNODE_PATHentries are separated with;(and:can corrupt drive-letter paths), so module resolution can fail. Replace the concatenation to usepath.delimiterinstead (e.g.,${facetNodeModules}${delimiter}${originalNodePath}).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/vite-ssr-loader.ts` around lines 109 - 116, The NODE_PATH concatenation in vite-ssr-loader uses a hardcoded ':' which breaks on Windows; update the logic that builds process.env.NODE_PATH (using facetNodeModules and originalNodePath) to join entries with path.delimiter instead of ':' and ensure path is imported/available, then call require('module').Module._initPaths() as before to reload resolution; adjust the ternary that sets process.env.NODE_PATH to use `${facetNodeModules}${path.delimiter}${originalNodePath}` when originalNodePath is non-empty.
🧹 Nitpick comments (2)
.github/workflows/test.yml (1)
36-40: ⚡ Quick winVerify whether Bun is still needed in CI after the Node migration.
Line 39 still provisions Bun. If no remaining unit-test/build path calls Bun, remove it to reduce CI setup time and prevent hidden runtime coupling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test.yml around lines 36 - 40, The workflow still provisions Bun in the "Setup tools" step (uses: flanksource/deps) by listing bun@v1.3.10 in the tools input; check all CI/workflow steps and repo scripts (package.json scripts, task files, any workflow jobs) for references to bun or bun-specific commands, and if none remain after the Node migration remove bun@v1.3.10 from the tools list (leaving task@v3.39.2) in the Setup tools step to shorten setup time and eliminate hidden coupling, then run the CI locally or trigger the workflow to verify no job fails due to the removal.cli/test/shell.test.ts (1)
15-19: ⚡ Quick winAdd regression coverage for array interpolation and signal termination
The suite validates scalar interpolation, but it misses the two high-risk behaviors used in production: array interpolation and signal-terminated process handling. Adding those tests will lock in the expected contract of
$.Also applies to: 41-51
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/test/shell.test.ts` around lines 15 - 19, Add two regression tests alongside the existing "shell-escapes interpolated values" test in cli/test/shell.test.ts: one that verifies array interpolation is escaped correctly by invoking the same $ template (e.g., using an array like ['a','b;echo pwned'] interpolated into $`printf '%s\n' ${arr}`) and asserting combined stdout matches expected values, and another that verifies signal-terminated child handling by spawning a process via the $ template, sending it a signal (SIGTERM/SIGINT) and asserting the returned result indicates termination by signal (or the promise rejects with the expected signal info); reuse the existing test harness/quiet() invocation and assertion style so behavior is locked in for both scalar/array interpolation and signal termination cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 66-69: The checkout step using actions/checkout@v4 currently sets
only ref: ${{ needs.version-bump.outputs.new_tag }}; update that checkout
configuration (the step with name "Checkout code" and uses: actions/checkout@v4)
to include persist-credentials: false so the publish job does not leave Git
credentials in local git config.
- Around line 27-30: The workflow uses mutable action tags (e.g. "uses:
pnpm/action-setup@v4", "uses: actions/checkout@v4", "uses:
actions/setup-node@v4"); replace each "`@v4`" tag with the corresponding pinned
commit SHA from the action's GitHub repository (use the commit SHA for the
release you want) so the workflow references an exact immutable version, and
update all occurrences of those three actions in the file.
In @.github/workflows/test.yml:
- Around line 22-29: The workflow uses mutable tags pnpm/action-setup@v4 and
actions/setup-node@v4 which should be pinned to immutable commit SHAs; update
the uses lines to reference the exact commit SHAs for pnpm/action-setup and
actions/setup-node (e.g., pnpm/action-setup@<commit-sha> and
actions/setup-node@<commit-sha>) by locating the repositories' latest stable
commit for the desired v4 behavior, replace the `@v4` tags with those SHAs, and
verify the workflow still sets the same version inputs (version: 8.13.1 for pnpm
and the Node.js version input) before committing.
In `@cli/README.md`:
- Around line 23-24: Update the global install commands in cli/README.md to use
the actual published package name; replace occurrences of "`@flanksource/facet`"
with "`@flanksource/facet-cli`" in the lines showing pnpm/npm install commands so
they match cli/package.json's "name": "`@flanksource/facet-cli`" and the release
workflow/notes that install the CLI package.
In `@cli/src/builders/facet-directory.ts`:
- Around line 833-835: The generated .npmrc is written with default permissions;
update the write operations (the writeFileSync calls that use npmrcPath and
npmrcContent) to write with restrictive permissions (mode 0o600) and also ensure
existing files that are overwritten have their mode set to 0o600 (e.g., via
fs.chmodSync) so inherited auth tokens are not world-readable; apply the same
change to the other writeFileSync block referenced around the 840-842 region and
reference the npmrcPath/writeFileSync calls when making the edit.
- Around line 508-509: The current code builds dataScript using
JSON.stringify(data) and writes it via writeFileSync to join(this.facetRoot,
'facet-data.js'), which allows script-breakout sequences like </script>;
instead, escape the serialized JSON before embedding: call JSON.stringify(data)
into a variable, then replace unsafe sequences (at minimum </script>, <!--, -->
and the line separator characters U+2028 and U+2029) with safe escaped
equivalents (e.g., replace '</script>' with '<\\/script>' and replace
U+2028/U+2029 with '\\u2028'/'\\u2029'), then build dataScript from the escaped
string and write it using the same writeFileSync to avoid raw injection in
facet-data.js.
- Around line 955-958: The synchronous pnpm invocation using spawnSync in
facet-directory.ts (the call that assigns to result) lacks a timeout and can
hang; update the spawnSync invocation to pass a timeout option (e.g., timeout:
<ms>) and handle the case where result.error or result.status is null/undefined
or where result.signal indicates a timeout by logging/throwing a clear error and
exiting the command. Ensure you reference the spawnSync call that produces the
const result and add logic after that call to detect result.error or a timeout
and fail fast with a helpful message.
In `@cli/src/commands/doctor.ts`:
- Around line 521-532: The checkTsx function currently uses
probeOptionalBin('tsx', ...) which only checks PATH and yields false warnings;
update checkTsx to probe the same resolver the runtime uses instead (e.g., use
the project's runtime resolver or bundled tsx lookup used elsewhere—replace
probeOptionalBin call with the resolver function used by the runtime, such as
resolve/bundled tsx path or getBundledTsxPath), and if that resolver returns a
path, report status 'pass' with its version output; if not found, downgrade the
result to non-failing (change status to 'info' or remove the warn) and keep the
hint only if truly required. Ensure you modify references inside checkTsx and
stop relying solely on probeOptionalBin so the doctor reflects the migrated
bundled/resolved tsx behavior.
In `@cli/src/generators/html.ts`:
- Around line 88-103: The live mode branch (when options.live is true) currently
takes the server from startViteServer, snapshots HTML via snapshotHTML and
writes it out without applying the CSS scoping flow, so --css-scope is ignored;
either apply the same CSS scoping routine used in the SSR path to the captured
html before writing (invoke the existing scoping function used elsewhere) or
explicitly guard and error when options.cssScope is set in conjunction with
options.live (e.g., throw a user-facing error and avoid writing output). Locate
the live branch around options.live, startViteServer, snapshotHTML, outputPath
and writeFile and implement one of those two fixes so scoping behavior matches
the non-live path.
In `@cli/src/server/preview.ts`:
- Around line 156-158: The stream-forwarding loop in writeWebResponse currently
writes each chunk from response.body directly into the Node ServerResponse
without honoring backpressure; modify the loop that iterates over response.body
(the for await (const chunk of response.body ...) block) to check the boolean
result of res.write(chunk) and, if it returns false, await a Promise that
resolves on res.once('drain') before continuing, and ensure res.end() is still
called after the loop completes and any pending backpressure awaits are
resolved; this prevents unbounded buffering when clients are slow.
In `@cli/src/utils/shell.ts`:
- Around line 34-47: The buildCommand currently stringifies interpolated arrays
into one comma-separated token which breaks callers that expect multiple shell
arguments; update buildCommand (and reuse escape/isRaw) so when a value is an
array (Array.isArray(value)) you map each element to either element.raw if
isRaw(element) is true or escape(element) otherwise, then join those mapped
pieces with spaces and append into cmd (instead of treating the whole array as a
single value); leave non-array handling unchanged so single values still use
isRaw(value) ? value.raw : escape(value).
- Around line 90-104: The close handler in cli/src/utils/shell.ts currently
coerces a null code to 0 and treats signal-terminated children as success;
update the child.on('close', ...) callback to accept both (code, signal), treat
any non-null signal as a failure (rejecting with a ShellError that includes
stdout, stderr, signal and a null/undefined exitCode), and only treat numeric
code === 0 as success; modify the logic around the existing Error/ShellError
construction (referencing this.command and ShellError) to include signal details
when rejecting.
In `@examples/kitchen-sink/HeaderSolid.tsx`:
- Around line 54-55: Update the example page title strings that incorrectly
state "10mm" to reflect the configured default header height of "14mm"; find the
JSX elements in HeaderSolid.tsx where the title prop is set to "Page 2 — 10mm
compact chrome" (and the similar title strings on the other occurrences) and
change those strings to "Page 2 — 14mm compact chrome" (and the corresponding
changes for the other two title props so all titles match the 14mm default
header).
In `@README.md`:
- Around line 21-22: Update the global install examples to use the new CLI
package name by replacing occurrences of the string "`@flanksource/facet`" with
"`@flanksource/facet-cli`" in the README examples (the two global install lines
using "pnpm add -g" and "npm install -g"); ensure both commands show the new
package name so users install the CLI package rather than the old library
package.
In `@src/components/diagram/Arrow.tsx`:
- Around line 15-18: The bidirectional arrow preset fails to render tails
because showTail is not included in VariantProps and showTail: true is not set
on the preset; update the VariantProps type to include 'showTail' alongside the
other picked props (so TypeScript will accept it) and add showTail: true to the
bidirectional preset object (alongside its tailShape: 'circle') in Arrow.tsx to
enable tail endpoint rendering.
In `@src/components/diagram/content.tsx`:
- Around line 27-29: The JSX in items.map uses key={item} which can collide for
duplicate labels; update the mapping in the component that renders NodePill so
each element uses a stable unique key (for example combine the item value with
its index or an explicit id) by changing the key expression in the items.map
callback (the NodePill render) to something like `${item}-${index}` or use an
item.id property if available to ensure unique React keys and stable
reconciliation.
In `@src/components/ListTable.tsx`:
- Around line 127-138: splitColorClasses currently drops any non-color tokens so
tagMapping consumers lose layout/typography/shape classes; update
splitColorClasses (and how its result is used in the badge rendering around the
tagMapping code at lines that build badges) to preserve passthrough classes by
collecting all tokens that are not text-*/bg-*/border-* into a passthrough field
(e.g. passthrough?: string[] or passthrough?: string) and return it alongside
textColor/color/borderColor, then update the badge rendering logic (the
tagMapping usage) to append those passthrough classes back into the badge
className so existing layout/shape/typography classes are not dropped.
In `@src/styles.css`:
- Line 4: Replace the `@import` url(...) statement in styles.css with the string
import notation to satisfy stylelint; locate the `@import` statement (the Google
Fonts import at the top of styles.css) and change its syntax from
url("https://fonts.googleapis.com/...") to use a quoted string import (i.e.,
`@import` "https://fonts.googleapis.com/..." ) so the linter no longer flags an
import-notation violation.
---
Outside diff comments:
In `@cli/src/bundler/vite-builder.ts`:
- Around line 61-65: The code currently composes a shell command string (using
srtPrefix and then joining with tsx path tokens) which is vulnerable to shell
injection and breaks on paths with spaces; instead construct an argv array and
run the binary without a shell: change srtPrefix to return the settings token
(or undefined) rather than a shell fragment and in the caller (the code that
builds and runs the loader command around the tsx path tokens) assemble an argv
like ['srt', '--settings', settings, ...tsxArgs] or ['tsx', tsxPath, ...] as
appropriate, then invoke the process using a shell-less API (child_process.spawn
or execFile / execa with shell: false) so each argument is passed as a distinct
argv element and not interpolated into /bin/sh -c. Ensure you preserve handling
of sandbox being boolean or string and include settings only when provided.
In `@cli/src/generators/html.ts`:
- Around line 65-72: The clearCache branch currently only removes the .facet
directory, which no longer matches the flag’s documented intent to also clear
the node_modules cache; update the options.clearCache handling (the block that
computes root using consumerRoot || resolve(dirname(options.template))) to also
detect and remove the node_modules cache under the same root (e.g., compute
nodeModulesDir = join(root, 'node_modules') and if existsSync(nodeModulesDir)
then rmSync(nodeModulesDir, { recursive: true, force: true }) and log the
removal), or if you prefer to change behavior instead, update the CLI docs/help
to remove the “node_modules cache” claim — adjust the code around facetDir and
the logger calls accordingly.
In `@cli/vite-ssr-loader.ts`:
- Around line 109-116: The NODE_PATH concatenation in vite-ssr-loader uses a
hardcoded ':' which breaks on Windows; update the logic that builds
process.env.NODE_PATH (using facetNodeModules and originalNodePath) to join
entries with path.delimiter instead of ':' and ensure path is
imported/available, then call require('module').Module._initPaths() as before to
reload resolution; adjust the ternary that sets process.env.NODE_PATH to use
`${facetNodeModules}${path.delimiter}${originalNodePath}` when originalNodePath
is non-empty.
In `@src/components/Badge.tsx`:
- Around line 432-434: The renderLabel function currently uses a truthy guard
(!label) which hides valid values like 0; update the conditional in renderLabel
to use a nullish check (label == null) so that falsy-but-valid nodes (e.g., 0 or
empty string) are rendered, keeping the Icon behavior unchanged when both are
absent.
In `@src/styles.css`:
- Around line 415-421: The CSS rule ".table-header th" is forcing left alignment
via "text-align: left", which overrides per-column alignment utilities (e.g.,
header cells in LogoGrid.tsx); remove or neutralize that property so alignment
is controlled by component-level classes—either delete the "text-align: left"
declaration or replace it with a non-opinionated value like "text-align:
inherit" on the ".table-header th" selector so per-column utilities can take
effect.
In `@Taskfile.yml`:
- Around line 69-74: The Taskfile.yml task currently describes and runs Bun but
the CLI package uses Vitest; update the task's desc from "Run cli unit tests
with bun" to "Run cli unit tests with Vitest" and replace the cmds entry that
currently runs "bun test" with the proper Vitest invocation—e.g., run "pnpm
test" (keeping the existing dir: cli and deps: gen-version) so the task executes
the CLI package.json "test": "vitest run" command.
---
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 36-40: The workflow still provisions Bun in the "Setup tools" step
(uses: flanksource/deps) by listing bun@v1.3.10 in the tools input; check all
CI/workflow steps and repo scripts (package.json scripts, task files, any
workflow jobs) for references to bun or bun-specific commands, and if none
remain after the Node migration remove bun@v1.3.10 from the tools list (leaving
task@v3.39.2) in the Setup tools step to shorten setup time and eliminate hidden
coupling, then run the CI locally or trigger the workflow to verify no job fails
due to the removal.
In `@cli/test/shell.test.ts`:
- Around line 15-19: Add two regression tests alongside the existing
"shell-escapes interpolated values" test in cli/test/shell.test.ts: one that
verifies array interpolation is escaped correctly by invoking the same $
template (e.g., using an array like ['a','b;echo pwned'] interpolated into
$`printf '%s\n' ${arr}`) and asserting combined stdout matches expected values,
and another that verifies signal-terminated child handling by spawning a process
via the $ template, sending it a signal (SIGTERM/SIGINT) and asserting the
returned result indicates termination by signal (or the promise rejects with the
expected signal info); reuse the existing test harness/quiet() invocation and
assertion style so behavior is locked in for both scalar/array interpolation and
signal termination cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9906502f-d338-4183-8467-04f3bd9550cf
⛔ Files ignored due to path filters (4)
cli/package-lock.jsonis excluded by!**/package-lock.jsoncli/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpackage-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (96)
.gavel.yaml.github/workflows/release.yml.github/workflows/test.yml.gitignore.npmrcDockerfileREADME.mdTaskfile.ymlchart/test/.ginkgo/ginkgo-report-.-.-1777281827467563000.jsoncli/.gitignorecli/.npmrccli/ARCHITECTURE.mdcli/README.mdcli/STATUS.mdcli/package.jsoncli/scripts/copy-assets.cjscli/scripts/gen-version.cjscli/src/builders/facet-directory.test.tscli/src/builders/facet-directory.tscli/src/bundler/live-snapshot.tscli/src/bundler/vite-builder.test.tscli/src/bundler/vite-builder.tscli/src/bundler/vite-server.tscli/src/cli.tscli/src/commands/doctor.test.tscli/src/commands/doctor.tscli/src/generators/html.tscli/src/server/archive.tscli/src/server/facet-cache.tscli/src/server/facet-types.tscli/src/server/preview.tscli/src/server/routes.tscli/src/types.tscli/src/utils/assets.tscli/src/utils/data-loader.tscli/src/utils/package-manager.test.tscli/src/utils/package-manager.tscli/src/utils/pdf-generator.tscli/src/utils/remote-resolver.test.tscli/src/utils/remote-resolver.tscli/src/utils/shell.tscli/src/utils/tailwind.test.tscli/src/utils/tailwind.tscli/test/jsx-parser.test.tscli/test/lint-rules.test.tscli/test/pdf-bleed.test.tscli/test/pdf-security.test.tscli/test/render-api.test.tscli/test/resolve-output.test.tscli/test/shell.test.tscli/tsconfig.jsoncli/tsconfig.test.jsoncli/tsdown.config.tscli/vite-dev-loader.tscli/vite-ssr-loader.tscli/vitest.config.tsexamples/kitchen-sink/BadgeExamples.tsxexamples/kitchen-sink/BleedTest.tsxexamples/kitchen-sink/DataFlowDiagram.tsxexamples/kitchen-sink/FindingsReport.tsxexamples/kitchen-sink/FlanksourceHeader.tsxexamples/kitchen-sink/HeaderSolid.tsxexamples/kitchen-sink/PageSizeTest.tsxexamples/kitchen-sink/package.jsonexamples/quickstart/README.mdpackage.jsonsrc/components/Avatar.tsxsrc/components/AvatarGroup.tsxsrc/components/Badge.tsxsrc/components/BulletList.tsxsrc/components/CalloutBox.tsxsrc/components/CompactTable.tsxsrc/components/Document.tsxsrc/components/Finding.tsxsrc/components/Header.tsxsrc/components/ListTable.tsxsrc/components/LogoGrid.tsxsrc/components/MatrixTable.tsxsrc/components/ProgressBar.tsxsrc/components/ScoreGauge.tsxsrc/components/SeverityStatCard.tsxsrc/components/Shield/Shield.tsxsrc/components/StatCard.tsxsrc/components/diagram/Arrow.test.tsxsrc/components/diagram/Arrow.tsxsrc/components/diagram/BoxNode.test.tsxsrc/components/diagram/BoxNode.tsxsrc/components/diagram/Diagram.tsxsrc/components/diagram/colors.tssrc/components/diagram/content.tsxsrc/components/diagram/index.tssrc/components/index.tsxsrc/components/utils/resolveSizeVariant.tssrc/stories/components/Badge.stories.tsxsrc/styles.csstailwind.config.js
💤 Files with no reviewable changes (6)
- cli/test/resolve-output.test.ts
- cli/ARCHITECTURE.md
- cli/STATUS.md
- cli/test/jsx-parser.test.ts
- cli/test/lint-rules.test.ts
- cli/src/server/facet-cache.ts
| - name: Setup pnpm | ||
| uses: pnpm/action-setup@v4 | ||
| with: | ||
| version: 8.13.1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expectation after fix: no matches in this workflow.
rg -n '^\s*uses:\s*[^#\s]+@v?[0-9]+(\.[0-9]+)?(\.[0-9]+)?\s*$' .github/workflows/release.ymlRepository: flanksource/facet
Length of output: 289
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba .github/workflows/release.yml | sed -n '15,45p'
nl -ba .github/workflows/release.yml | sed -n '55,95p'Repository: flanksource/facet
Length of output: 104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba .github/workflows/release.yml | sed -n '15,45p'
nl -ba .github/workflows/release.yml | sed -n '55,95p'Repository: flanksource/facet
Length of output: 104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n .github/workflows/release.yml | sed -n '15,45p'
cat -n .github/workflows/release.yml | sed -n '55,95p'Repository: flanksource/facet
Length of output: 2476
Pin uses: actions to commit SHAs (not @v4)
release.yml uses mutable major version tags (@v4) for GitHub Actions, which can be retargeted upstream; pin to exact commit SHAs instead.
- name: Setup pnpm
uses: pnpm/action-setup@v4
with:
version: 8.13.1Also update other @v4 usages in this workflow: actions/checkout@v4 (22, 67), actions/setup-node@v4 (33, 77).
🧰 Tools
🪛 zizmor (1.25.2)
[error] 28-28: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release.yml around lines 27 - 30, The workflow uses
mutable action tags (e.g. "uses: pnpm/action-setup@v4", "uses:
actions/checkout@v4", "uses: actions/setup-node@v4"); replace each "`@v4`" tag
with the corresponding pinned commit SHA from the action's GitHub repository
(use the commit SHA for the release you want) so the workflow references an
exact immutable version, and update all occurrences of those three actions in
the file.
Source: Linters/SAST tools
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ needs.version-bump.outputs.new_tag }} |
There was a problem hiding this comment.
Disable credential persistence in publish-job checkout.
The publish job only reads the tag and publishes to npm; it doesn’t need a write-capable Git credential left in local git config. Add persist-credentials: false on Line 69 checkout config.
As per coding guidelines, least-privilege violations should be reduced in CI workflows.
🔐 Suggested workflow hardening
- name: Checkout code
uses: actions/checkout@v4
with:
ref: ${{ needs.version-bump.outputs.new_tag }}
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| with: | |
| ref: ${{ needs.version-bump.outputs.new_tag }} | |
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| with: | |
| ref: ${{ needs.version-bump.outputs.new_tag }} | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 66-69: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 67-67: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release.yml around lines 66 - 69, The checkout step using
actions/checkout@v4 currently sets only ref: ${{
needs.version-bump.outputs.new_tag }}; update that checkout configuration (the
step with name "Checkout code" and uses: actions/checkout@v4) to include
persist-credentials: false so the publish job does not leave Git credentials in
local git config.
Source: Linters/SAST tools
| - name: Setup pnpm | ||
| uses: pnpm/action-setup@v4 | ||
| with: | ||
| version: 8.13.1 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant section of the workflow
sed -n '1,120p' .github/workflows/test.yml | nl -ba | sed -n '1,120p'
# Extract just the "uses:" lines for quick confirmation
rg -n --fixed-string "uses:" .github/workflows/test.yml || trueRepository: flanksource/facet
Length of output: 104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/test.yml"
# Print the file with line numbers using awk (nl not available)
awk '{printf "%d:%s\n", NR, $0}' "$FILE" | sed -n '1,160p'
# Specifically show lines 15-60 to cover the reported 22-29 window
awk '{printf "%d:%s\n", NR, $0}' "$FILE" | sed -n '15,60p'
# Show all uses: lines (with context)
rg -n --context 2 --glob='*.yml' --glob='*.yaml' 'uses:' "$FILE" || trueRepository: flanksource/facet
Length of output: 3646
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --glob='!.git/*' \
"uses:\s+pnpm/action-setup@" .github/workflows . || true
rg -n --hidden --glob='!.git/*' \
"uses:\s+actions/setup-node@" .github/workflows . || true
# Look for any uses pinned to a 40+ char SHA (common pattern) as a proxy for existing convention
rg -n --hidden --glob='!.git/*' \
"uses:\s+[^@]+@[0-9a-f]{40,}" .github/workflows . || trueRepository: flanksource/facet
Length of output: 883
🌐 Web query:
pnpm/action-setup v4 commit SHA
💡 Result:
The pnpm/action-setup v4.0.0 release is tagged at commit SHA 0c17529a66d0e80a7199c06374f11467433f0249 [1]. This version was published on May 7, 2024 [2]. You can verify this in the repository's tag history or by viewing the specific release [2]. Note that the project has since evolved to v6 [3][4], and later v4 minor releases (e.g., v4.4.0) are associated with different SHAs [5].
Citations:
- 1: https://git.bjxgj.com/pnpm/action-setup/compare/v4.0.0..v4
- 2: https://github.com/pnpm/action-setup/releases/tag/v4.0.0
- 3: pnpm/action-setup@v4...v6
- 4: https://github.com/pnpm/action-setup/
- 5: kazupon/pnpmc@becb7d6
🌐 Web query:
actions/setup-node v4 commit SHA
💡 Result:
The actions/setup-node v4 major version is a tag that points to various releases, such as v4.0.0 through v4.0.4. Because v4 is a mutable tag, it does not have a single, static commit SHA [1][2][3]. For security and reproducibility, it is recommended to pin GitHub Actions to an immutable commit SHA rather than a version tag [4]. A commonly used commit SHA associated with the actions/setup-node v4 series is 49933ea5288caeca8642d1e84afbd3f7d6820020 [1][4]. You can verify or find the specific commit SHA for any version tag by navigating to the official repository's tags section [2][5]. To use a specific SHA in your workflow, update your step as follows [4]: - uses: actions/setup-node@49933ea # v4
Citations:
- 1: https://github.com/actions/setup-node/tree/49933ea5288caeca8642d1e84afbd3f7d6820020
- 2: https://github.com/actions/setup-node/tree/v4.0.0
- 3: https://github.com/actions/setup-node/releases/tag/v4.0.4
- 4: daytonaio/daytona@521bac5
- 5: https://github.com/actions/setup-node
🌐 Web query:
pnpm/action-setup tag v4 commit SHA (i.e., what does @v4 resolve to?)
💡 Result:
The tag v4 in the pnpm/action-setup repository is a floating tag, meaning it does not point to a single static commit SHA [1]. Instead, it is updated by the repository maintainers to point to the latest commit within the v4 release series [1]. Because tags in GitHub Actions are mutable, @v4 will resolve to the current head of the v4 branch or the latest v4-series release [1]. As of 2026-06-07, the project has progressed significantly beyond the v4 series, with releases currently in the v6.x.x range [2][3][4]. Users are generally encouraged to use more specific tags (e.g., v4.3.0) or to migrate to newer major versions (such as v6) to receive the latest features, bug fixes, and Node.js runtime compatibility updates [2][5].
Citations:
- 1: https://github.com/pnpm/action-setup/tree/v4
- 2: https://github.com/pnpm/action-setup/releases/tag/v6.0.8
- 3: https://github.com/pnpm/action-setup/releases/tag/v6.0.4
- 4: https://github.com/pnpm/action-setup/releases/tag/v6.0.1
- 5: pnpm/action-setup@v4...v6
🌐 Web query:
actions/setup-node tag v4 commit SHA (i.e., what does @v4 resolve to?)
💡 Result:
In GitHub Actions, the tag v4 for actions/setup-node is a mutable reference that points to the latest commit within the v4 release series. Because tags in Git can be updated or moved, the specific commit SHA that @v4 resolves to is not static; it changes whenever a new minor or patch version (e.g., v4.1.0, v4.2.0) is released and the v4 tag is updated to point to it [1]. While many users historically used 49933ea5288caeca8642d1e84afbd3f7d6820020 as a common stable reference for v4 [2][3], this commit does not represent the current state of the v4 tag. For security and stability, it is recommended to pin actions to their full immutable commit SHA (e.g., uses: actions/setup-node@sha256:...) rather than relying on mutable tags like @v4 [3]. You can verify the exact SHA currently associated with the v4 tag by checking the official repository's tags section on GitHub [4][5].
Citations:
- 1: actions/setup-node@v4.0.3...v4.1.0
- 2: https://github.com/actions/setup-node/tree/49933ea5288caeca8642d1e84afbd3f7d6820020
- 3: daytonaio/daytona@521bac5
- 4: https://github.com/actions/setup-node
- 5: https://github.com/actions/setup-node?tab=readme-ov-file
Pin GitHub Actions to immutable commit SHAs (avoid mutable @v4 tags).
In .github/workflows/test.yml, both pnpm/action-setup@v4 (line 23) and actions/setup-node@v4 (line 28) use mutable major tags, weakening workflow supply-chain integrity.
🔒 Suggested fix
- - name: Setup pnpm
- uses: pnpm/action-setup@v4
+ - name: Setup pnpm
+ uses: pnpm/action-setup@<full_commit_sha>
with:
version: 8.13.1
- name: Setup Node.js
- uses: actions/setup-node@v4
+ uses: actions/setup-node@<full_commit_sha>🧰 Tools
🪛 zizmor (1.25.2)
[error] 23-23: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 28-28: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/test.yml around lines 22 - 29, The workflow uses mutable
tags pnpm/action-setup@v4 and actions/setup-node@v4 which should be pinned to
immutable commit SHAs; update the uses lines to reference the exact commit SHAs
for pnpm/action-setup and actions/setup-node (e.g.,
pnpm/action-setup@<commit-sha> and actions/setup-node@<commit-sha>) by locating
the repositories' latest stable commit for the desired v4 behavior, replace the
`@v4` tags with those SHAs, and verify the workflow still sets the same version
inputs (version: 8.13.1 for pnpm and the Node.js version input) before
committing.
Source: Linters/SAST tools
| pnpm add -g @flanksource/facet | ||
| # or: npm install -g @flanksource/facet |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "CLI package name from cli/package.json:"
rg -n '"name"\s*:' cli/package.json -C1
echo
echo "Published package identifiers in release workflow:"
rg -n "`@flanksource/facet-cli`|`@flanksource/facet`" .github/workflows/release.yml -C2Repository: flanksource/facet
Length of output: 910
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba cli/README.md | sed -n '15,40p'
echo
echo "Search for global install instructions in cli/README.md:"
rg -n "install -g|pnpm add -g|npm install -g" cli/README.mdRepository: flanksource/facet
Length of output: 104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "cli/README.md lines 15-40 (with line numbers):"
cat -n cli/README.md | sed -n '15,40p'
echo
echo "Global install instructions in cli/README.md:"
rg -n "install -g|pnpm add -g|npm install -g" cli/README.md || true
echo
echo "Global install instructions elsewhere in repo:"
rg -n "install -g|pnpm add -g|npm install -g" . || trueRepository: flanksource/facet
Length of output: 1533
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "cli/package.json: name and bin fields"
rg -n '"name"\s*:' cli/package.json -C1
rg -n '"bin"\s*:' cli/package.json -C2 || true
cat -n cli/package.json | sed -n '1,80p'Repository: flanksource/facet
Length of output: 2969
Fix global install package name in cli/README.md
cli/README.md recommends pnpm add -g @flanksource/facet / `npm install -g `@flanksource/facet, but the CLI package being published is @flanksource/facet-cli (cli/package.json has "name": "@flanksource/facet-cli" and the release workflow/notes install @flanksource/facet-cli). Align the installation command in cli/README.md (lines 23-24) to use @flanksource/facet-cli.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/README.md` around lines 23 - 24, Update the global install commands in
cli/README.md to use the actual published package name; replace occurrences of
"`@flanksource/facet`" with "`@flanksource/facet-cli`" in the lines showing pnpm/npm
install commands so they match cli/package.json's "name":
"`@flanksource/facet-cli`" and the release workflow/notes that install the CLI
package.
| const dataScript = `window.__FACET_DATA__ = ${JSON.stringify(data)};\n`; | ||
| writeFileSync(join(this.facetRoot, 'facet-data.js'), dataScript, 'utf-8'); |
There was a problem hiding this comment.
Escape injected JSON before writing facet-data.js.
Embedding raw JSON.stringify(data) into a <script> payload allows script-breakout via sequences like </script>, which can execute arbitrary JS in the live preview page.
Proposed fix
- const dataScript = `window.__FACET_DATA__ = ${JSON.stringify(data)};\n`;
+ const serializedData = JSON.stringify(data)
+ .replace(/</g, '\\u003c')
+ .replace(/\u2028/g, '\\u2028')
+ .replace(/\u2029/g, '\\u2029');
+ const dataScript = `window.__FACET_DATA__ = ${serializedData};\n`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const dataScript = `window.__FACET_DATA__ = ${JSON.stringify(data)};\n`; | |
| writeFileSync(join(this.facetRoot, 'facet-data.js'), dataScript, 'utf-8'); | |
| const serializedData = JSON.stringify(data) | |
| .replace(/</g, '\\u003c') | |
| .replace(/\u2028/g, '\\u2028') | |
| .replace(/\u2029/g, '\\u2029'); | |
| const dataScript = `window.__FACET_DATA__ = ${serializedData};\n`; | |
| writeFileSync(join(this.facetRoot, 'facet-data.js'), dataScript, 'utf-8'); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/src/builders/facet-directory.ts` around lines 508 - 509, The current code
builds dataScript using JSON.stringify(data) and writes it via writeFileSync to
join(this.facetRoot, 'facet-data.js'), which allows script-breakout sequences
like </script>; instead, escape the serialized JSON before embedding: call
JSON.stringify(data) into a variable, then replace unsafe sequences (at minimum
</script>, <!--, --> and the line separator characters U+2028 and U+2029) with
safe escaped equivalents (e.g., replace '</script>' with '<\\/script>' and
replace U+2028/U+2029 with '\\u2028'/'\\u2029'), then build dataScript from the
escaped string and write it using the same writeFileSync to avoid raw injection
in facet-data.js.
| pnpm add -g @flanksource/facet | ||
| # or: npm install -g @flanksource/facet |
There was a problem hiding this comment.
Use the CLI package name in global install docs.
Line 21 and Line 22 still install @flanksource/facet, but this migration publishes the CLI as @flanksource/facet-cli. Users following this command can install the wrong package.
Proposed fix
-pnpm add -g `@flanksource/facet`
-# or: npm install -g `@flanksource/facet`
+pnpm add -g `@flanksource/facet-cli`
+# or: npm install -g `@flanksource/facet-cli`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pnpm add -g @flanksource/facet | |
| # or: npm install -g @flanksource/facet | |
| pnpm add -g `@flanksource/facet-cli` | |
| # or: npm install -g `@flanksource/facet-cli` |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 21 - 22, Update the global install examples to use
the new CLI package name by replacing occurrences of the string
"`@flanksource/facet`" with "`@flanksource/facet-cli`" in the README examples (the
two global install lines using "pnpm add -g" and "npm install -g"); ensure both
commands show the new package name so users install the CLI package rather than
the old library package.
| type VariantProps = Pick< | ||
| xarrowPropsType, | ||
| 'color' | 'strokeWidth' | 'headSize' | 'dashness' | 'curveness' | 'headShape' | 'tailShape' | 'showHead' | ||
| >; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For react-xarrows v2.0.2, what is the default value of showTail, and does setting tailShapewithoutshowTail render a tail marker?
💡 Result:
In react-xarrows version 2.0.2, the default value of the showTail prop is false [1][2][3]. Setting the tailShape prop without explicitly setting showTail to true does not render a tail marker [1][2]. The showTail prop acts as the toggle to determine whether the tail is displayed; the tailShape prop merely defines the appearance of that tail when it is enabled [2][3]. To render a tail marker, you must explicitly set showTail={true} [1][4].
Citations:
- 1: https://www.npmjs.com/package/react-xarrows
- 2: https://eliav2.github.io/react-xarrows/
- 3: https://github.com/Eliav2/react-xarrows
- 4: https://github.com/Eliav2/react-xarrows/blob/master/README.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la
fd -a "Arrow.tsx" -t f . || true
FILE="src/components/diagram/Arrow.tsx"
if [ -f "$FILE" ]; then
echo "=== $FILE (tail around 1-120) ==="
nl -ba "$FILE" | sed -n '1,140p'
else
echo "File not found: $FILE"
fiRepository: flanksource/facet
Length of output: 2693
🌐 Web query:
react-xarrows showTail default tailShape renders tail marker
💡 Result:
In react-xarrows, the tail marker is controlled by the showTail and tailShape properties [1][2]. When showTail is set to true (it defaults to false), the tail marker is rendered at the start of the arrow [1][2]. The tailShape property defines the appearance of this marker [1]. By default, tailShape is set to 'arrow1' [1][2]. You can customize the tail using these properties: - showTail: A boolean that toggles the visibility of the tail marker [1][2]. - tailShape: A string or object that determines the shape of the tail [1]. Predefined string options include 'arrow1', 'circle', and 'heart' [2][3]. - arrowTailProps: An object used to pass custom SVG properties to the tail marker if you wish to maintain the default shape while altering its attributes [1][2]. Note that advanced custom shapes can be implemented by passing an object to tailShape, which requires defining an svgElem (as a JSX element) and an offsetForward value [4].
Citations:
- 1: https://eliav2.github.io/react-xarrows/
- 2: https://github.com/Eliav2/react-xarrows/blob/master/README.md
- 3: https://www.npmjs.com/package/react-xarrows
- 4: https://github.com/Eliav2/react-xarrows/blob/master/CHANGELOG.md
🌐 Web query:
eliav2 react-xarrows showTail tailShape code
💡 Result:
In react-xarrows, the tail of an arrow can be enabled and customized using the showTail and tailShape props [1][2]. By default, the tail is hidden [1][3]. To show it, set the showTail prop to true [2][3]. For customization, you can use the tailShape prop to define the appearance of the arrow's tail [1][4]. This prop accepts either a string for predefined shapes (e.g., 'arrow1', 'circle', or 'heart') or an object for custom SVG definitions [1][2]. You can also use the arrowTailProps prop if you need to pass additional custom SVG properties to the tail element [2][4]. Example usage: <Xarrow start="startElement" end="endElement" showTail={true} tailShape="circle" tailColor="red" arrowTailProps={{ stroke: 'black', strokeWidth: 2 }} /> You can also import the arrowShapes object from the library if you wish to see or use the full list of predefined shapes [2][3].
Citations:
- 1: https://eliav2.github.io/react-xarrows/
- 2: https://github.com/Eliav2/react-xarrows/blob/master/README.md
- 3: https://www.npmjs.com/package/react-xarrows
- 4: https://github.com/Eliav2/react-xarrows/blob/master/src/types.ts
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la
fd -a "Arrow.tsx" -t f . || true
FILE="src/components/diagram/Arrow.tsx"
if [ -f "$FILE" ]; then
echo "=== $FILE (tail around 1-140) ==="
nl -ba "$FILE" | sed -n '1,140p'
else
echo "File not found: $FILE"
fiRepository: flanksource/facet
Length of output: 2693
🌐 Web query:
react-xarrows showTail default tailShape renders tail marker
💡 Result:
In the react-xarrows library, setting showTail to true enables the rendering of the arrow's tail [1][2]. The tailShape property defines the shape of this marker, and it defaults to 'arrow1' [1][3]. You can customize the tail marker by passing one of the predefined string values—'arrow1', 'circle', or 'heart'—to the tailShape prop [2][4]. If you need to apply custom SVG properties to the default shape, you can use the arrowTailProps object instead of replacing the shape entirely [1][4]. For more advanced use cases, it is also possible to pass a custom SVG element directly to tailShape [4]. To display the tail, ensure that both showTail={true} is set and that a valid shape (or the default) is defined [1][3].
Citations:
- 1: https://eliav2.github.io/react-xarrows/
- 2: https://github.com/Eliav2/react-xarrows/blob/master/README.md
- 3: https://www.npmjs.com/package/react-xarrows
- 4: https://github.com/Eliav2/react-xarrows
🌐 Web query:
eliav2 react-xarrows showTail tailShape code
💡 Result:
To use the tail functionality in react-xarrows, you must set the showTail prop to true [1][2]. You can customize the shape of the tail using the tailShape prop [1][3]. The showTail prop is a boolean that determines whether the arrow tail is rendered [1][3]. By default, it is set to false [1][2]. The tailShape prop accepts either a string for predefined shapes or an object for custom SVG shapes [2][4]. Predefined options include arrow1, circle, and heart [2][4]. Example usage: import Xarrow from 'react-xarrows'; // Basic usage with default tail shape // Using a predefined tail shape // Customizing tail properties <Xarrow start="start-element" end="end-element" showTail={true} tailShape="circle" arrowTailProps={{ fill: "transparent", stroke: "red", strokeWidth: 2 }} /> If you need to define a fully custom SVG shape, you can pass an object containing an svgElem and an optional offsetForward value to the tailShape prop [4]. Additionally, the arrowTailProps prop can be used to pass standard SVG attributes to the tail element [1][4].
Citations:
- 1: https://eliav2.github.io/react-xarrows/
- 2: https://github.com/Eliav2/react-xarrows/blob/master/README.md
- 3: https://www.npmjs.com/package/react-xarrows
- 4: https://registry.npmjs.org/react-xarrows
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/components/diagram/Arrow.tsx"
test -f "$FILE" || { echo "Missing $FILE"; exit 1; }
echo "=== grep key props ==="
rg -n --hidden --no-ignore -S "bidirectional|showTail|tailShape|tailColor|tailProps|xarrowPropsType" "$FILE" || true
echo "=== lines 1-120 ==="
awk '{printf "%4d\t%s\n", NR, $0}' "$FILE" | sed -n '1,120p'
echo "=== lines 35-90 ==="
awk '{printf "%4d\t%s\n", NR, $0}' "$FILE" | sed -n '35,90p'
echo "=== lines 110-200 ==="
awk '{printf "%4d\t%s\n", NR, $0}' "$FILE" | sed -n '110,200p'Repository: flanksource/facet
Length of output: 5102
Enable the tail endpoint for bidirectional (showTail)
src/components/diagram/Arrow.tsx’s bidirectional preset sets tailShape: 'circle' but never sets showTail, while showTail in react-xarrows (v2.0.2) defaults to false—so tailShape alone won’t render the tail marker. Add showTail: true to the bidirectional preset (and include showTail in VariantProps so it typechecks).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/diagram/Arrow.tsx` around lines 15 - 18, The bidirectional
arrow preset fails to render tails because showTail is not included in
VariantProps and showTail: true is not set on the preset; update the
VariantProps type to include 'showTail' alongside the other picked props (so
TypeScript will accept it) and add showTail: true to the bidirectional preset
object (alongside its tailShape: 'circle') in Arrow.tsx to enable tail endpoint
rendering.
| {items.map((item) => ( | ||
| <NodePill key={item}>{item}</NodePill> | ||
| ))} |
There was a problem hiding this comment.
Use unique keys for repeated section items.
Line 28 uses key={item}; duplicate labels will produce duplicate React keys and unstable reconciliation. Add an index suffix (or explicit ID).
Suggested fix
- {items.map((item) => (
- <NodePill key={item}>{item}</NodePill>
+ {items.map((item, index) => (
+ <NodePill key={`${title}-${item}-${index}`}>{item}</NodePill>
))}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {items.map((item) => ( | |
| <NodePill key={item}>{item}</NodePill> | |
| ))} | |
| {items.map((item, index) => ( | |
| <NodePill key={`${title}-${item}-${index}`}>{item}</NodePill> | |
| ))} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/diagram/content.tsx` around lines 27 - 29, The JSX in
items.map uses key={item} which can collide for duplicate labels; update the
mapping in the component that renders NodePill so each element uses a stable
unique key (for example combine the item value with its index or an explicit id)
by changing the key expression in the items.map callback (the NodePill render)
to something like `${item}-${index}` or use an item.id property if available to
ensure unique React keys and stable reconciliation.
| function splitColorClasses(classes: string): { textColor?: string; color?: string; borderColor?: string } { | ||
| return classes.split(/\s+/).reduce<{ textColor?: string; color?: string; borderColor?: string }>((acc, token) => { | ||
| if (token.startsWith('text-')) { | ||
| acc.textColor = token; | ||
| } else if (token.startsWith('bg-')) { | ||
| acc.color = token; | ||
| } else if (token.startsWith('border-')) { | ||
| acc.borderColor = token; | ||
| } | ||
| return acc; | ||
| }, {}); | ||
| } |
There was a problem hiding this comment.
tagMapping output contract was narrowed and now drops non-color classes.
Lines 127-138 only preserve text-*, bg-*, and border-* tokens; then Lines 240-250 render badges from those parsed parts only. Any existing tagMapping classes for layout/typography/shape are silently discarded, which changes consumer behavior without a type/API update.
Proposed fix (preserve passthrough classes)
-function splitColorClasses(classes: string): { textColor?: string; color?: string; borderColor?: string } {
- return classes.split(/\s+/).reduce<{ textColor?: string; color?: string; borderColor?: string }>((acc, token) => {
+function splitColorClasses(classes: string): { textColor?: string; color?: string; borderColor?: string; className?: string } {
+ const extra: string[] = [];
+ const parsed = classes.split(/\s+/).reduce<{ textColor?: string; color?: string; borderColor?: string }>((acc, token) => {
if (token.startsWith('text-')) {
acc.textColor = token;
} else if (token.startsWith('bg-')) {
acc.color = token;
} else if (token.startsWith('border-')) {
acc.borderColor = token;
+ } else if (token) {
+ extra.push(token);
}
return acc;
}, {});
+ return { ...parsed, className: extra.join(' ') || undefined };
}
@@
- const { color, textColor, borderColor } = splitColorClasses(resolveTagClasses(tagMapping, tagKey, value));
+ const { color, textColor, borderColor, className } = splitColorClasses(resolveTagClasses(tagMapping, tagKey, value));
@@
borderColor={borderColor}
+ className={className}
/>Also applies to: 240-250
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ListTable.tsx` around lines 127 - 138, splitColorClasses
currently drops any non-color tokens so tagMapping consumers lose
layout/typography/shape classes; update splitColorClasses (and how its result is
used in the badge rendering around the tagMapping code at lines that build
badges) to preserve passthrough classes by collecting all tokens that are not
text-*/bg-*/border-* into a passthrough field (e.g. passthrough?: string[] or
passthrough?: string) and return it alongside textColor/color/borderColor, then
update the badge rendering logic (the tagMapping usage) to append those
passthrough classes back into the badge className so existing
layout/shape/typography classes are not dropped.
| /* Load Fira Code so the `font-mono` Tailwind stack actually resolves to it | ||
| * in Puppeteer/headless Chromium, which doesn't ship Fira Code. | ||
| * Must come before @tailwind directives. */ | ||
| @import url("https://fonts.googleapis.com/css2?family=Fira+Code:wght@400;500;600;700&display=swap"); |
There was a problem hiding this comment.
Fix the stylelint import-notation violation.
Line [4] uses @import url(...), but static analysis is already flagging this as an error. Switch to string import notation to avoid lint failure.
Proposed fix
-@import url("https://fonts.googleapis.com/css2?family=Fira+Code:wght@400;500;600;700&display=swap");
+@import "https://fonts.googleapis.com/css2?family=Fira+Code:wght@400;500;600;700&display=swap";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @import url("https://fonts.googleapis.com/css2?family=Fira+Code:wght@400;500;600;700&display=swap"); | |
| `@import` "https://fonts.googleapis.com/css2?family=Fira+Code:wght@400;500;600;700&display=swap"; |
🧰 Tools
🪛 Stylelint (17.12.0)
[error] 4-4: Expected "url("https://fonts.googleapis.com/css2?family=Fira+Code:wght@400;500;600;700&display=swap")" to be ""https://fonts.googleapis.com/css2?family=Fira+Code:wght@400;500;600;700&display=swap"" (import-notation)
(import-notation)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/styles.css` at line 4, Replace the `@import` url(...) statement in
styles.css with the string import notation to satisfy stylelint; locate the
`@import` statement (the Google Fonts import at the top of styles.css) and change
its syntax from url("https://fonts.googleapis.com/...") to use a quoted string
import (i.e., `@import` "https://fonts.googleapis.com/..." ) so the linter no
longer flags an import-notation violation.
Source: Linters/SAST tools
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.github/workflows/release.yml (2)
111-115:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDisable persisted git credentials in
build-binariescheckout.This job only needs read access to build/upload artifacts; keep credentials out of local git config by setting
persist-credentials: false.Suggested hardening
- name: Checkout code uses: actions/checkout@v4 with: ref: ${{ needs.version-bump.outputs.new_tag }} + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 111 - 115, The checkout step using actions/checkout@v4 in the build-binaries job is persisting git credentials; update that step (the Checkout code step) to include persist-credentials: false so credentials are not stored in the local git config. Locate the step that uses actions/checkout@v4 and add the with key persist-credentials: false alongside the existing ref: ${{ needs.version-bump.outputs.new_tag }}.Source: Linters/SAST tools
112-112:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin all changed
uses:actions to immutable commit SHAs.These steps still use mutable tags (
@v4,@v1.0.24), which weakens workflow supply-chain integrity. Pin each action reference to a full commit SHA.#!/bin/bash set -euo pipefail # Expectation after fix: no output lines from this workflow. rg -n '^\s*uses:\s*[^#\s]+@v?[0-9]+(\.[0-9]+)?(\.[0-9]+)?\s*$' .github/workflows/release.ymlAlso applies to: 117-117, 122-122, 127-127, 149-149, 161-161
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml at line 112, Several workflow steps still reference mutable action tags (e.g. uses: actions/checkout@v4 and other uses: lines flagged), so replace each mutable tag with the corresponding action's immutable full commit SHA (40-char) — for example swap uses: actions/checkout@v4 to uses: actions/checkout@<full-commit-sha> and do the same for actions/setup-node, aws-actions/configure-aws-credentials, aws-actions/amazon-ecr-login, actions/upload-artifact (and any other uses: entries flagged). Retrieve the correct SHA from the action's GitHub repo (select the commit that matches the tag you want) and update the uses: values to the full SHA, then run the provided regex check to ensure there are no remaining numeric tags.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In @.github/workflows/release.yml:
- Around line 111-115: The checkout step using actions/checkout@v4 in the
build-binaries job is persisting git credentials; update that step (the Checkout
code step) to include persist-credentials: false so credentials are not stored
in the local git config. Locate the step that uses actions/checkout@v4 and add
the with key persist-credentials: false alongside the existing ref: ${{
needs.version-bump.outputs.new_tag }}.
- Line 112: Several workflow steps still reference mutable action tags (e.g.
uses: actions/checkout@v4 and other uses: lines flagged), so replace each
mutable tag with the corresponding action's immutable full commit SHA (40-char)
— for example swap uses: actions/checkout@v4 to uses:
actions/checkout@<full-commit-sha> and do the same for actions/setup-node,
aws-actions/configure-aws-credentials, aws-actions/amazon-ecr-login,
actions/upload-artifact (and any other uses: entries flagged). Retrieve the
correct SHA from the action's GitHub repo (select the commit that matches the
tag you want) and update the uses: values to the full SHA, then run the provided
regex check to ensure there are no remaining numeric tags.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 016c5db9-2603-41c9-ad57-597dcd938007
⛔ Files ignored due to path filters (1)
cli/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
.github/workflows/release.ymlcli/package.jsoncli/src/bundler/live-snapshot.tscli/src/cli.bun.tscli/src/server/worker-pool.tscli/src/utils/assets.bun.tscli/src/utils/assets.tscli/src/utils/debug-annotations.tscli/src/utils/pdf-generator.tscli/src/utils/pdf-multipass.tscli/tsconfig.jsoncli/tsdown.config.ts
✅ Files skipped from review due to trivial changes (3)
- cli/src/cli.bun.ts
- cli/src/utils/pdf-multipass.ts
- cli/src/utils/debug-annotations.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- cli/src/utils/pdf-generator.ts
- cli/tsdown.config.ts
- cli/tsconfig.json
- cli/src/bundler/live-snapshot.ts
moshloop
force-pushed
the
refactor/cli-bun-to-node
branch
from
ef279a8 to
bef4f06
Compare
…peteer-core
Rebased onto main (0.1.41), dropping commits main already absorbed
(pnpm migration, component work) and re-applying the bun->node migration
on top of main's newer CLI code.
CLI runtime migration:
- Node $ shell helper (utils/shell.ts) replaces Bun shell across the CLI,
including main's newer files (package-manager.ts, vite-builder.ts,
doctor.ts, routes.ts, generators/html.ts, remote-resolver.ts, etc.)
- node:http server with Web Request/Response adapters replaces Bun.serve
- node:child_process (spawn/execFileSync) replaces Bun.spawn*; tsx runs
the vite loaders via resolveTsxBin (no bun runtime)
- assets resolved via import.meta.url (utils/assets.ts) instead of Bun's
`with { type: 'file' }`; all tests run under vitest, not bun:test
Distribution:
- published as @flanksource/facet-cli (Node ESM bundle, dist/cli.mjs bin)
- standalone bun binary restored via build:bin -> src/cli.bun.ts, which
embeds assets (cli.bun.ts/assets.bun.ts use bun's file imports) so the
compiled binary resolves them from /$bunfs; release.yml builds 3 targets
and publishes both @flanksource/facet and @flanksource/facet-cli
puppeteer -> puppeteer-core:
- no Chromium auto-download; always use system Chrome via resolveChromePath()
Render cache:
- configurable cacheDir (--cache-dir / FACET_CACHE_DIR); render-api test
uses an isolated temp cache so the /render/stream test renders cold
Verified: node bundle and bun binary both render SimpleReport.tsx to a
valid PDF-1.7; vitest 203 passed; tsc 11 errors (all pre-existing on main).
moshloop
force-pushed
the
refactor/cli-bun-to-node
branch
from
bef4f06 to
c6353f2
Compare
2 participants
Summary
Ports the facet CLI off the Bun runtime so it can ship as a real, Node-installable npm package (`@flanksource/facet-cli`). Previously the CLI relied on Bun APIs (`Bun.serve`, Bun shell `$`, `Bun.spawn`, `import ... with { type: 'file' }`, `bun:test`) and was distributed as a bun-compiled standalone binary, which made it impossible to `npm install -g`.
Changes
Verification
`node dist/cli.mjs pdf SimpleReport.tsx --data simple-data.json` produces a valid 52KB PDF-1.7 with no Bun runtime present. `vitest run`: 183 passed; the 2 sandbox-only failures are macOS sandbox blocking Chrome (pass when Chrome can launch).
Summary by CodeRabbit
New Features
xxssizing for badges/avatars and denser layout controlsBug Fixes
Chores / Tooling