feat(dep-graph): add dependency graph generator package#289
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as dep-graph CLI
participant Discovery as App Discovery
participant FileSystem as File System
participant Graph as Graph Builder
participant Rules as Dependency Rules
participant Renderer as Markdown Renderer
participant Output as Output File
CLI->>Discovery: discoverApps(appsDir)
Discovery->>FileSystem: scan categories & services
FileSystem-->>Discovery: app metadata
Discovery-->>CLI: apps list
CLI->>Graph: buildGraph(apps, RULES)
Graph->>FileSystem: read .yml/.yaml files
FileSystem-->>Graph: concatenated content
Graph->>Rules: rule.detect(content)
Rules-->>Graph: boolean match
Graph-->>CLI: edges list
CLI->>Renderer: renderMarkdown(apps, edges)
Renderer-->>CLI: markdown string
CLI->>Output: writeFile(outputPath)
Output-->>CLI: file written confirmation
CLI-->>CLI: log success & metrics
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
hobroker
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/dep-graph/package.json`:
- Line 13: Update packages/dep-graph/package.json to add ESLint as a
devDependency and ensure the lint script can run: in the package.json object add
"eslint": "<version>" under "devDependencies" alongside existing devDeps
("@types/node", "tsx", "typescript") and run npm/yarn/pnpm install so the eslint
binary is available for the existing "lint": "eslint ." script; also add an
.eslintrc configuration file in packages/dep-graph (copy or adapt the config
from packages/catalog) so eslint has rules to use when the lint script runs.
In `@packages/dep-graph/src/cli.ts`:
- Line 1: The file has formatting issues reported by CI; run the project's
formatter (npm run format) to apply Prettier rules and commit the resulting
changes so imports/whitespace in packages/dep-graph/src/cli.ts (e.g., the
top-level import of Command) match the codebase style.
- Line 57: The call to program.parseAsync(process.argv) returns a Promise and
must be awaited or handled to avoid unhandled rejections; update the entrypoint
to either await program.parseAsync(process.argv) inside an async wrapper (e.g.,
an immediately-invoked async function) or append a .catch handler that logs the
error and exits (ensuring any thrown error from program.parseAsync is captured),
referencing the program.parseAsync invocation so you modify that exact call.
In `@packages/dep-graph/src/graph.ts`:
- Line 1: Prettier formatting failed for the changes (notably the import
statement `import { readdir, readFile } from "node:fs/promises";`); run the
project's formatter (npm run format) and commit the resulting changes so the
import and the rest of packages/dep-graph/src/graph.ts conform to the repository
Prettier rules.
- Around line 73-89: The concurrent Promise.all loop mutates the shared edges
array from multiple async callbacks (apps.map -> collectYamlContent -> rules),
risking data races; fix by having each async map callback build a local array
(e.g., localEdges) of matches instead of pushing into the shared edges, return
that array from the callback, await Promise.all to get an array-of-arrays, then
flatten/concat the results into the shared edges (or replace edges with the
flattened result) so all mutations happen single-threadedly after awaiting.
In `@packages/dep-graph/src/render.ts`:
- Line 1: The file packages/dep-graph/src/render.ts has formatting mismatches
(see the import line `import type { App, Edge } from "./graph.js";`); run the
project's formatter (e.g., npm run format) or apply Prettier to
packages/dep-graph/src/render.ts to reformat the file so it matches CI style
rules, then commit the formatted file.
- Around line 12-16: The Mermaid output currently inserts raw category and app
names (see byCategory iteration using category and app.name) which can produce
invalid Mermaid when names contain spaces or special chars; add a helper (e.g.,
mermaidId or quoteIdentifier) that returns a safely quoted/escaped identifier
(wrap in double quotes or escape problematic chars) and use it when emitting the
subgraph line and node lines (replace `subgraph ${category}` and `${app.name}`
with `subgraph ${mermaidId(category)}` and `${mermaidId(app.name)}`) so all
generated identifiers remain valid Mermaid syntax.
In `@packages/dep-graph/src/rules.ts`:
- Line 1: CI reports formatting errors in packages/dep-graph/src/rules.ts — run
the repository formatter to fix Prettier issues; specifically run npm run format
(or the project's configured Prettier command) and commit the updated formatting
for files including rules.ts so the CI formatting check passes. Ensure you stage
and push the formatted changes.
- Around line 38-41: The three regexes inside the detect function (the patterns
matching "serviceMonitor:\s*\n\s*enabled:", "podMonitor:\s*\n\s*enabled:", and
"kind:\s*(ServiceMonitor|PodMonitor|PrometheusRule)") assume Unix-only newlines;
make them line-ending agnostic by replacing the explicit "\n" with a pattern
that allows optional CR before LF (e.g., use "\r?\n" or a character-class like
"[\r\n]+" as appropriate) and ensure surrounding whitespace matching still
works; update the regex literals in the detect arrow function so they match both
LF and CRLF YAML files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9a37377b-3d18-470a-909e-90d1eac2d383
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
DEPENDENCIES.mdpackage.jsonpackages/dep-graph/package.jsonpackages/dep-graph/src/cli.tspackages/dep-graph/src/graph.tspackages/dep-graph/src/render.tspackages/dep-graph/src/rules.tspackages/dep-graph/tsconfig.json
| "scripts": { | ||
| "start": "tsx src/cli.ts", | ||
| "typecheck": "tsc --noEmit", | ||
| "lint": "eslint .", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if ESLint is available at root or in other packages
rg -l '"eslint"' --glob 'package.json'Repository: hobroker/selfhosted
Length of output: 92
🏁 Script executed:
# Check root package.json for ESLint
cat -n package.json | grep -i eslintRepository: hobroker/selfhosted
Length of output: 45
🏁 Script executed:
# Check if there's a workspaces configuration in root package.json
cat -n package.json | head -50Repository: hobroker/selfhosted
Length of output: 1113
🏁 Script executed:
# Verify packages/dep-graph/package.json doesn't have eslint in devDependencies
cat -n packages/dep-graph/package.jsonRepository: hobroker/selfhosted
Length of output: 811
That's like planning a Pizza Party Friday without actually ordering the pizza!
The lint script references eslint . but ESLint isn't declared as a dependency in this package—and it's not in the root workspace either (only in packages/catalog). When Turbo tries to run this lint task, the eslint executable simply won't be there.
Add ESLint to devDependencies in packages/dep-graph/package.json:
"devDependencies": {
"@types/node": "^25.5.0",
"eslint": "^<version>",
"tsx": "^4.21.0",
"typescript": "^5.9.3"
}You'll also need an .eslintrc config file for this package (check packages/catalog for reference).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dep-graph/package.json` at line 13, Update
packages/dep-graph/package.json to add ESLint as a devDependency and ensure the
lint script can run: in the package.json object add "eslint": "<version>" under
"devDependencies" alongside existing devDeps ("@types/node", "tsx",
"typescript") and run npm/yarn/pnpm install so the eslint binary is available
for the existing "lint": "eslint ." script; also add an .eslintrc configuration
file in packages/dep-graph (copy or adapt the config from packages/catalog) so
eslint has rules to use when the lint script runs.
| @@ -0,0 +1,57 @@ | |||
| import { Command } from "commander"; | |||
There was a problem hiding this comment.
Prettier is feeling left out again — time for some formatting TLC!
CI reports formatting issues. Run npm run format to restore harmony.
🧰 Tools
🪛 GitHub Actions: CI
[warning] 1-1: Prettier reported formatting issues in this file during 'prettier --check .'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dep-graph/src/cli.ts` at line 1, The file has formatting issues
reported by CI; run the project's formatter (npm run format) to apply Prettier
rules and commit the resulting changes so imports/whitespace in
packages/dep-graph/src/cli.ts (e.g., the top-level import of Command) match the
codebase style.
| }, | ||
| ); | ||
|
|
||
| program.parseAsync(process.argv); |
There was a problem hiding this comment.
This promise is floating in space like a balloon at a birthday party nobody caught!
parseAsync() returns a Promise, but it's not awaited. If an error occurs during command execution, it could result in an unhandled promise rejection.
🎈 Proposed fix to handle the async result
-program.parseAsync(process.argv);
+program.parseAsync(process.argv).catch((err) => {
+ console.error(err);
+ process.exit(1);
+});📝 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.
| program.parseAsync(process.argv); | |
| program.parseAsync(process.argv).catch((err) => { | |
| console.error(err); | |
| process.exit(1); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dep-graph/src/cli.ts` at line 57, The call to
program.parseAsync(process.argv) returns a Promise and must be awaited or
handled to avoid unhandled rejections; update the entrypoint to either await
program.parseAsync(process.argv) inside an async wrapper (e.g., an
immediately-invoked async function) or append a .catch handler that logs the
error and exits (ensuring any thrown error from program.parseAsync is captured),
referencing the program.parseAsync invocation so you modify that exact call.
| @@ -0,0 +1,94 @@ | |||
| import { readdir, readFile } from "node:fs/promises"; | |||
There was a problem hiding this comment.
Prettier is sending you a formal complaint — please address it!
CI pipeline shows formatting issues. Run npm run format to clear this up.
🧰 Tools
🪛 GitHub Actions: CI
[warning] 1-1: Prettier reported formatting issues in this file during 'prettier --check .'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dep-graph/src/graph.ts` at line 1, Prettier formatting failed for
the changes (notably the import statement `import { readdir, readFile } from
"node:fs/promises";`); run the project's formatter (npm run format) and commit
the resulting changes so the import and the rest of
packages/dep-graph/src/graph.ts conform to the repository Prettier rules.
| await Promise.all( | ||
| apps.map(async (app) => { | ||
| const content = await collectYamlContent(app.dir); | ||
| for (const rule of rules) { | ||
| if (app.name === rule.provider) continue; // skip self | ||
| if (!appNames.has(rule.provider)) continue; // skip unknown providers | ||
| if (rule.detect(content)) { | ||
| edges.push({ | ||
| from: app.name, | ||
| to: rule.provider, | ||
| label: rule.label, | ||
| optional: rule.optional ?? false, | ||
| }); | ||
| } | ||
| } | ||
| }), | ||
| ); |
There was a problem hiding this comment.
🚨 DATA RACE ALERT! This is worse than when I hit Meredith with my car! 🚨
Multiple concurrent async callbacks are pushing to the shared edges array via Promise.all. JavaScript's Array.push is NOT atomic — concurrent pushes can corrupt the array or lose edges entirely.
While V8's current implementation might handle simple pushes correctly in practice, this is undefined behavior and a code smell that could cause subtle bugs.
🔧 Proposed fix: Collect edges per-app and flatten
export async function buildGraph(
apps: App[],
rules: DependencyRule[],
): Promise<Edge[]> {
const appNames = new Set(apps.map((a) => a.name));
- const edges: Edge[] = [];
- await Promise.all(
- apps.map(async (app) => {
+ const edgeArrays = await Promise.all(
+ apps.map(async (app): Promise<Edge[]> => {
+ const appEdges: Edge[] = [];
const content = await collectYamlContent(app.dir);
for (const rule of rules) {
if (app.name === rule.provider) continue; // skip self
if (!appNames.has(rule.provider)) continue; // skip unknown providers
if (rule.detect(content)) {
- edges.push({
+ appEdges.push({
from: app.name,
to: rule.provider,
label: rule.label,
optional: rule.optional ?? false,
});
}
}
+ return appEdges;
}),
);
+ const edges = edgeArrays.flat();
+
return edges.sort(
(a, b) => a.from.localeCompare(b.from) || a.to.localeCompare(b.to),
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dep-graph/src/graph.ts` around lines 73 - 89, The concurrent
Promise.all loop mutates the shared edges array from multiple async callbacks
(apps.map -> collectYamlContent -> rules), risking data races; fix by having
each async map callback build a local array (e.g., localEdges) of matches
instead of pushing into the shared edges, return that array from the callback,
await Promise.all to get an array-of-arrays, then flatten/concat the results
into the shared edges (or replace edges with the flattened result) so all
mutations happen single-threadedly after awaiting.
| @@ -0,0 +1,70 @@ | |||
| import type { App, Edge } from "./graph.js"; | |||
There was a problem hiding this comment.
Prettier wants to be friends — let's not leave it hanging!
CI reports formatting issues. A quick npm run format will smooth things over.
🧰 Tools
🪛 GitHub Actions: CI
[warning] 1-1: Prettier reported formatting issues in this file during 'prettier --check .'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dep-graph/src/render.ts` at line 1, The file
packages/dep-graph/src/render.ts has formatting mismatches (see the import line
`import type { App, Edge } from "./graph.js";`); run the project's formatter
(e.g., npm run format) or apply Prettier to packages/dep-graph/src/render.ts to
reformat the file so it matches CI style rules, then commit the formatted file.
| for (const [category, categoryApps] of byCategory) { | ||
| lines.push(` subgraph ${category}`); | ||
| for (const app of categoryApps) { | ||
| lines.push(` ${app.name}`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for any app directories with special characters
fd -t d . apps --max-depth 2 | grep -E '[^a-zA-Z0-9/_-]' || echo "All directory names are safe"Repository: hobroker/selfhosted
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Check if the file exists and read the relevant section
if [ -f "packages/dep-graph/src/render.ts" ]; then
echo "=== File found ==="
wc -l "packages/dep-graph/src/render.ts"
echo ""
echo "=== Lines 1-30 ==="
head -30 "packages/dep-graph/src/render.ts" | cat -n
else
echo "File not found at packages/dep-graph/src/render.ts"
# Try to find it
find . -name "render.ts" -type f 2>/dev/null | head -10
fiRepository: hobroker/selfhosted
Length of output: 1197
This Mermaid diagram could become a total nightmare if names ever get weird!
Right now, your app and category names are squeaky-clean and safe. Love that! But here's the thing—if someone adds a category like "Cloud Storage" or "API Gateway v2.0" down the road, this code will create invalid Mermaid syntax. Spaces, quotes, pipes—they're Mermaid's kryptonite!
Think of this like building a house: your foundation is solid now, but why not reinforce it before the storm hits? A quick sanitization function is your defensive MVP move:
🛡️ Add a mermaidId helper to quote identifiers safely
+function mermaidId(name: string): string {
+ // Replace problematic chars or wrap in quotes if needed
+ return /^[\w-]+$/.test(name) ? name : `"${name.replace(/"/g, '\\"')}"`;
+}
+
export function renderMermaid(apps: App[], edges: Edge[]): string {
const byCategory = new Map<string, App[]>();
for (const app of apps) {
@@ -12,9 +17,9 @@ export function renderMermaid(apps: App[], edges: Edge[]): string {
const lines: string[] = ["graph TD"];
for (const [category, categoryApps] of byCategory) {
- lines.push(` subgraph ${category}`);
+ lines.push(` subgraph ${mermaidId(category)}`);
for (const app of categoryApps) {
- lines.push(` ${app.name}`);
+ lines.push(` ${mermaidId(app.name)}`);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dep-graph/src/render.ts` around lines 12 - 16, The Mermaid output
currently inserts raw category and app names (see byCategory iteration using
category and app.name) which can produce invalid Mermaid when names contain
spaces or special chars; add a helper (e.g., mermaidId or quoteIdentifier) that
returns a safely quoted/escaped identifier (wrap in double quotes or escape
problematic chars) and use it when emitting the subgraph line and node lines
(replace `subgraph ${category}` and `${app.name}` with `subgraph
${mermaidId(category)}` and `${mermaidId(app.name)}`) so all generated
identifiers remain valid Mermaid syntax.
| @@ -0,0 +1,67 @@ | |||
| /** | |||
There was a problem hiding this comment.
Prettier is crying in the corner — we need to make it feel included!
The CI pipeline reports formatting issues. Run npm run format to make Prettier happy again. A happy Prettier is a happy team!
🧰 Tools
🪛 GitHub Actions: CI
[warning] 1-1: Prettier reported formatting issues in this file during 'prettier --check .'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dep-graph/src/rules.ts` at line 1, CI reports formatting errors in
packages/dep-graph/src/rules.ts — run the repository formatter to fix Prettier
issues; specifically run npm run format (or the project's configured Prettier
command) and commit the updated formatting for files including rules.ts so the
CI formatting check passes. Ensure you stage and push the formatted changes.
| detect: (c) => | ||
| /serviceMonitor:\s*\n\s*enabled:\s*true/.test(c) || | ||
| /podMonitor:\s*\n\s*enabled:\s*true/.test(c) || | ||
| /kind:\s*(ServiceMonitor|PodMonitor|PrometheusRule)/.test(c), |
There was a problem hiding this comment.
These regex patterns assume Unix line endings — could cause a "That's what she said" moment on Windows!
The patterns use \n for newline matching (e.g., serviceMonitor:\s*\n\s*enabled:). If YAML files have Windows-style CRLF line endings (\r\n), these patterns won't match.
🛠️ Consider making the patterns line-ending agnostic
{
provider: "prometheus-operator",
label: "ServiceMonitor / PodMonitor",
detect: (c) =>
- /serviceMonitor:\s*\n\s*enabled:\s*true/.test(c) ||
- /podMonitor:\s*\n\s*enabled:\s*true/.test(c) ||
+ /serviceMonitor:\s*\r?\n\s*enabled:\s*true/.test(c) ||
+ /podMonitor:\s*\r?\n\s*enabled:\s*true/.test(c) ||
/kind:\s*(ServiceMonitor|PodMonitor|PrometheusRule)/.test(c),
},📝 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.
| detect: (c) => | |
| /serviceMonitor:\s*\n\s*enabled:\s*true/.test(c) || | |
| /podMonitor:\s*\n\s*enabled:\s*true/.test(c) || | |
| /kind:\s*(ServiceMonitor|PodMonitor|PrometheusRule)/.test(c), | |
| { | |
| provider: "prometheus-operator", | |
| label: "ServiceMonitor / PodMonitor", | |
| detect: (c) => | |
| /serviceMonitor:\s*\r?\n\s*enabled:\s*true/.test(c) || | |
| /podMonitor:\s*\r?\n\s*enabled:\s*true/.test(c) || | |
| /kind:\s*(ServiceMonitor|PodMonitor|PrometheusRule)/.test(c), | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dep-graph/src/rules.ts` around lines 38 - 41, The three regexes
inside the detect function (the patterns matching
"serviceMonitor:\s*\n\s*enabled:", "podMonitor:\s*\n\s*enabled:", and
"kind:\s*(ServiceMonitor|PodMonitor|PrometheusRule)") assume Unix-only newlines;
make them line-ending agnostic by replacing the explicit "\n" with a pattern
that allows optional CR before LF (e.g., use "\r?\n" or a character-class like
"[\r\n]+" as appropriate) and ensure surrounding whitespace matching still
works; update the regex literals in the detect arrow function so they match both
LF and CRLF YAML files.
hobroker
force-pushed
the
feat/dep-graph-package
branch
from
8ae2320 to
c08b9a0
Compare
1 participant
Summary
Adds a new
packages/dep-graphworkspace package that scans all app YAML files and generates a Mermaid dependency graph inDEPENDENCIES.md. Distinguishes between required (solid) and optional (dashed) dependencies.dep-graphpackage withsrc/rules.ts(pattern-based detection),src/graph.ts(discovery + edge building),src/render.ts(Mermaid + markdown tables), andsrc/cli.ts(Commander CLI with prettier formatting on output)npm run dep-graphscript at the root to regenerateDEPENDENCIES.mdmetallbas an optional dependency (LoadBalancer IP is non-critical; port-forward works as fallback)metallbServiceMonitor config frommetallb/values.yaml(no longer a hard dep)To add a new dependency rule, append an entry to
RULESinsrc/rules.ts.Type