Repository navigation
fix(ci): repair holmes comment template indentation - #187
Conversation
Summary by CodeRabbitRelease Notes
WalkthroughRefactors CI to upload/download a dashboard-template artifact and replace inline shell steps with a reusable composite action; updates CLI to accept bundle-dir/history-file paths and resilient path resolution; Moriarty.predictionData no longer emits null eta/confidence; adds end-to-end CLI tests and CI diagnostics entries. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Runner as Workflow Runner
participant Action as run-holmes-command
participant CLI as node cli.mjs
participant Art as dashboard-template artifact
GH->>Runner: start wesley-holmes job
Runner->>Runner: checkout repository
Runner->>Action: run investigate/verify/predict (command, bundle-dir, history-file)
Action->>CLI: invoke node cli.mjs with resolved paths
CLI-->>Action: produce JSON + MD reports
Action-->>Runner: copy reports/* into workspace
Runner->>Art: upload `dashboard-template` (wesley-generate)
Runner->>Art: download `dashboard-template` in comment-report
Runner->>Runner: assemble dashboard by locating and copying downloaded `index.html`
Runner->>Runner: upload assembled `holmes-dashboard` artifact
sequenceDiagram
autonumber
participant Caller as Caller
participant Moriarty as Moriarty.predictionData
Caller->>Moriarty: request predictionData()
Moriarty->>Moriarty: compute base, latest, velocity, flags, patterns
Moriarty->>Moriarty: const result = { ...base, status:'OK', latest, velocity, plateauDetected, regressionDetected, patterns }
alt eta present
Moriarty->>Moriarty: result.eta = eta
end
alt confidence present
Moriarty->>Moriarty: result.confidence = confidence
end
Moriarty-->>Caller: return result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (2)
👮 Files not reviewed due to content moderation or server errors (2)
🧰 Additional context used📓 Path-based instructions (2).github/workflows/!({cli-tests.yml,cli-quick.yml})📄 CodeRabbit inference engine (AGENTS.md)
Files:
.github/workflows/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-10-06T09:11:26.403ZApplied to files:
📚 Learning: 2025-10-13T16:54:18.932ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
🚢 SHIPME[REALM] PASS — sha b1a1448 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
🔍 The Case of Pull Request #187🕵️ SHA-lock HOLMES's InvestigationReport missing for holmes 🩺 Dr. WATSON's VerificationReport missing for watson 🔮 Professor MORIARTY's PredictionsReport missing for moriartyMachine-readable reports: holmes-report.json · watson-report.json · moriarty-report.json (see workflow artifacts).Filed at 221B Repository Street |
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/wesley-holmes.yml(5 hunks)CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000001.jsonl(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
.github/workflows/!({cli-tests.yml,cli-quick.yml})
📄 CodeRabbit inference engine (AGENTS.md)
Do not duplicate CLI end-to-end/bats checks in the main CI workflows without approval
Files:
.github/workflows/wesley-holmes.yml
.github/workflows/**
📄 CodeRabbit inference engine (AGENTS.md)
.github/workflows/**: Only touch .github/workflows/* when fixing a deterministic break (syntax/path/quoting) or reducing obviously wasteful runtime/duplication, and provide a revert path in the PR description
Use least privilege in workflows; do not widen permissions or add secrets without approval
Files:
.github/workflows/wesley-holmes.yml
🔇 Additional comments (1)
CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000001.jsonl (1)
32-36: Machine chronicle entries document the workflow repairs.The five new log records correctly document the investigation and repair sequence. The JSONL format is valid, timestamps are sequential, and the notes field captures the technical essence of each action.
The "observations_on_humanity" quips are consistent with the existing file's tone, though one might question whether a machine log should editorialize about its creators' relationship with whitespace and nulls.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/wesley-holmes/src/cli.mjs (1)
131-158: The report command is COMPLETELY BROKEN and you didn't even touch it.Look at lines 131-132:
const bundle = loadBundle(); const history = loadHistory();You refactored
loadBundleto REQUIRE abundlePathparameter (line 27). You refactoredloadHistoryto REQUIREhistoryPathandbundleDirparameters (line 39). Then you left thereportcommand calling them with ZERO arguments.What happens?
loadBundle(undefined)flows intoresolvePath(undefined, path.resolve('.wesley/bundle.json')), which returns the fallback—so it accidentally still works for the default case. ButloadHistory(undefined, undefined)will computedefaultHistory = path.join(undefined ?? '.wesley', 'history.json'), which becomespath.join('.wesley', 'history.json'). Again, accidentally correct for the default.So it's not fatally broken, but it's SLOPPY AS HELL. You've introduced explicit parameterization everywhere else but left this one command implicitly relying on undefined behavior. If someone later changes the default logic in
resolvePathorloadHistory, this silent reliance on undefined will bite them.Fix it:
.action(options => { - const bundle = loadBundle(); - const history = loadHistory(); + const opts = program.optsWithGlobals(); + const bundlePath = path.join(resolvePath(opts.bundleDir, '.wesley'), 'bundle.json'); + const bundleDir = resolvePath(opts.bundleDir, '.wesley'); + const bundle = loadBundle(bundlePath); + const history = loadHistory(opts.historyFile, bundleDir); const holmes = new Holmes(bundle);Match the pattern you used in the other commands. Don't leave landmines for future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.github/actions/run-holmes-command/action.yml(1 hunks).github/workflows/wesley-holmes.yml(6 hunks)CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000001.jsonl(1 hunks)packages/wesley-holmes/src/cli.mjs(6 hunks)packages/wesley-holmes/test/cli-options.test.mjs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
.github/workflows/!({cli-tests.yml,cli-quick.yml})
📄 CodeRabbit inference engine (AGENTS.md)
Do not duplicate CLI end-to-end/bats checks in the main CI workflows without approval
Files:
.github/workflows/wesley-holmes.yml
.github/workflows/**
📄 CodeRabbit inference engine (AGENTS.md)
.github/workflows/**: Only touch .github/workflows/* when fixing a deterministic break (syntax/path/quoting) or reducing obviously wasteful runtime/duplication, and provide a revert path in the PR description
Use least privilege in workflows; do not widen permissions or add secrets without approval
Files:
.github/workflows/wesley-holmes.yml
🧠 Learnings (2)
📚 Learning: 2025-10-06T08:35:11.213Z
Learnt from: CR
PR: flyingrobots/wesley#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-06T08:35:11.213Z
Learning: Applies to packages/wesley-host-node/bin/wesley.mjs : Place the CLI executable at packages/wesley-host-node/bin/wesley.mjs (not in wesley-cli)
Applied to files:
packages/wesley-holmes/src/cli.mjs
📚 Learning: 2025-10-06T08:35:11.213Z
Learnt from: CR
PR: flyingrobots/wesley#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-06T08:35:11.213Z
Learning: Applies to packages/wesley-host-node/package.json : Map the CLI binary in host-node package.json "bin" field to bin/wesley.mjs
Applied to files:
packages/wesley-holmes/src/cli.mjs
🧬 Code graph analysis (2)
packages/wesley-holmes/src/cli.mjs (2)
packages/wesley-host-node/src/adapters/WesleyFileWriter.mjs (2)
historyPath(97-97)history(98-98)packages/wesley-holmes/test/cli-options.test.mjs (1)
bundleDir(14-14)
packages/wesley-holmes/test/cli-options.test.mjs (1)
packages/wesley-holmes/src/cli.mjs (1)
bundleDir(115-115)
🔇 Additional comments (13)
packages/wesley-holmes/src/cli.mjs (6)
27-37: loadBundle error messages now reference resolved paths—good.The refactor to accept an explicit
bundlePathparameter and resolve it before reading is solid. Error messages correctly report the resolved path, which makes debugging easier when paths go sideways.One thing: you're catching ALL errors and only checking
error?.code === 'ENOENT'. If the file exists but is unreadable (permissions, corrupted JSON, whatever), you get the generic "Unable to read" message. That's acceptable, but you could destructure the error code in the catch for slightly better error classification. Not a blocker.
39-47: loadHistory with silent fallback is correct for its use case.Returning
{ points: [] }on any read/parse failure is the right defensive behavior for a history file that may legitimately not exist yet. The bundleDir parameter propagation is clean.
65-66: New CLI options are well-named and documented.
--bundle-dirand--history-filegive users explicit control over paths. The defaults (.wesley for bundle-dir, unspecified for history-file) align with the rest of the codebase.
80-82: investigate command correctly wires bundle-dir option.You're reading
opts.bundleDirfrom the global options, resolving it, then joining withbundle.json. Logic is sound.
97-99: verify command correctly wires bundle-dir option.Same pattern as investigate. No issues.
114-116: predict command correctly wires bundle-dir and history-file options.You resolve
bundleDirand pass bothopts.historyFileandbundleDirtoloadHistory. The function handles the logic. Clean.packages/wesley-holmes/test/cli-options.test.mjs (2)
9-11: Path computation is brittle but acceptable for in-repo tests.You're walking up from
__filenameto findrepoRoot, then constructingcliPathby walking down intopackages/wesley-holmes/src/cli.mjs. This assumes the test file stays at its current depth and the CLI stays at its current location.For an end-to-end test living in the same monorepo, this is fine—if the structure changes, the test will fail loudly. But don't copy this pattern to published packages or external test harnesses.
12-97: runCli helper is functional but has coverage gaps.The function creates an isolated temp environment, writes deterministic bundle/history fixtures, spawns the CLI, asserts exit status and output shape, then cleans up. That's the right pattern for an e2e test.
However:
Weak markdown assertion: Line 89 checks
stdout.includes('#'). Any output with a hash will pass. A comment, a shebang line, whatever. You're not validating that it's actually markdown-formatted HOLMES output. Either check for a known string like"SHA-lock HOLMES"(which you do in the individual tests, fair), or match a markdown heading pattern.No error-case coverage: You test the happy path (investigate and predict succeed). What happens if the bundle is missing? Corrupted JSON? Invalid schema? The CLI should fail gracefully, and you should have tests for that.
Missing command coverage: You test
investigateandpredict. What aboutverify,report, andweights? The PR touches CLI option handling across all commands, so you should have at least smoke tests for each.The current tests are fine for what they cover, but you've left obvious gaps. Add tests for error cases and the other commands, or accept that you're shipping blind in those areas.
CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000001.jsonl (1)
32-37: Log entries document the repair sequence clearly.The six new entries trace the diagnostic and fix process for the HOLMES workflow breakage:
- Investigation of YAML parse failure
- Indentation fix for comment template
- CLI invocation path adjustment
- Moriarty null-value handling
- Checkout step restoration
- Unified CLI refactor with composite action
Each entry references the files touched and includes relevant context. For a machine-readable chronicle, this is solid documentation of the iterative repair process.
The "observations_on_humanity" quips are... fine. If you're going to anthropomorphize your CI, at least you're consistent about it.
.github/workflows/wesley-holmes.yml (4)
46-51: Dashboard template artifact is a good fix, but uploading only index.html is fragile.You're uploading
docs/holmes-dashboard/index.htmlas thedashboard-templateartifact. This eliminates the redundant checkout in the comment-report job (addressing the past review concern). Good.But you're uploading ONLY the HTML file. If the dashboard template ever needs CSS, JS, images, or other assets, this breaks. Why not upload the entire
docs/holmes-dashboard/directory?Change to:
- name: "📤 Upload Dashboard Template" uses: actions/upload-artifact@v4 with: name: dashboard-template - path: docs/holmes-dashboard/index.html + path: docs/holmes-dashboard/ if-no-files-found: errorThen adjust line 355 to:
- cp dashboard-template/index.html dashboard/index.html + cp -r dashboard-template/* dashboard/This future-proofs the artifact for multi-file templates.
88-91: Composite action eliminates the duplicate code cancer.You've replaced the three nearly-identical shell blocks (investigate, verify, predict) with calls to
./.github/actions/run-holmes-command. This directly addresses the past review comment about copy-paste madness.The action encapsulates the directory juggling, CLI invocation, and report file management. Each call now specifies only the
commandandreport-nameinputs. Clean.Based on past review comments.
Also applies to: 120-123, 153-156
345-349: Dashboard assembly now uses the artifact, but assumes single-file structure.You download the
dashboard-templateartifact (lines 345-349) and copy from it at line 355. This eliminates the redundant checkout that the past review comment flagged.However, line 355 assumes
dashboard-template/index.htmlexists. If the artifact structure changes or the upload step fails silently, this will break. Add a check:if [ ! -f dashboard-template/index.html ]; then echo "Dashboard template missing index.html" >&2 exit 1 fiOr better yet, upload the entire dashboard directory (see my comment on lines 46-51) and use
cp -r.Also applies to: 355-355
421-426: Template formatting is now consistent and correct.The indentation fix ensures the multiline string is inside the script block (closing backtick at line 426 after the italic line). The separator spacing is now uniform—no extra blank lines, matching the pattern in lines 411-418.
This resolves the past review comment about sloppy formatting and the original YAML parse error that broke the workflow.
Based on past review comments.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/wesley-holmes.yml (1)
396-410: Remove this dead-code fallback—it will never execute with your current artifact configuration.You're uploading
path: reports/holmesas artifactholmes-report, then downloading withmerge-multiple: trueto.. The extracted path isreports/holmes/holmes-report.md(lines 400, 418-420). The fallback checkingreports/reports/holmes/holmes-report.md(line 401) is dead code—there's zero way it gets triggered.This is cargo-cult programming at its finest. Stop adding defensive fallbacks for paths that don't exist. Either:
- Remove lines 401-405—the fallback serves no purpose, or
- Fix your artifact structure so it's actually consistent and doesn't need band-aids.
Don't leave this technical debt festering. It confuses the next person who touches this code.
♻️ Duplicate comments (1)
packages/wesley-holmes/src/cli.mjs (1)
20-22: This resolvePath function has an inconsistent contract.If
targetis provided and relative, you resolve it. Iftargetis missing, you returnfallbackwithout resolving it. So callers must pre-resolve the fallback? That's not obvious from the function signature.Either:
- Document that fallback must be absolute, OR
- Resolve the fallback too:
const chosen = target || fallback; return path.isAbsolute(chosen) ? chosen : path.resolve(chosen);Your current code works because all fallbacks happen to be pre-resolved, but the function's behavior is inconsistent and will confuse future maintainers.
Apply this diff to make the behavior consistent:
function resolvePath(target, fallback) { - return !target ? fallback : (path.isAbsolute(target) ? target : path.resolve(target)); + const chosen = target || fallback; + return path.isAbsolute(chosen) ? chosen : path.resolve(chosen); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.github/actions/run-holmes-command/action.yml(1 hunks).github/workflows/wesley-holmes.yml(7 hunks)CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000001.jsonl(1 hunks)packages/wesley-holmes/src/cli.mjs(7 hunks)packages/wesley-holmes/test/cli-options.test.mjs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
.github/workflows/!({cli-tests.yml,cli-quick.yml})
📄 CodeRabbit inference engine (AGENTS.md)
Do not duplicate CLI end-to-end/bats checks in the main CI workflows without approval
Files:
.github/workflows/wesley-holmes.yml
.github/workflows/**
📄 CodeRabbit inference engine (AGENTS.md)
.github/workflows/**: Only touch .github/workflows/* when fixing a deterministic break (syntax/path/quoting) or reducing obviously wasteful runtime/duplication, and provide a revert path in the PR description
Use least privilege in workflows; do not widen permissions or add secrets without approval
Files:
.github/workflows/wesley-holmes.yml
🧠 Learnings (2)
📚 Learning: 2025-10-06T08:35:11.213Z
Learnt from: CR
PR: flyingrobots/wesley#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-06T08:35:11.213Z
Learning: Applies to packages/wesley-host-node/bin/wesley.mjs : Place the CLI executable at packages/wesley-host-node/bin/wesley.mjs (not in wesley-cli)
Applied to files:
packages/wesley-holmes/src/cli.mjs
📚 Learning: 2025-10-06T08:35:11.213Z
Learnt from: CR
PR: flyingrobots/wesley#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-06T08:35:11.213Z
Learning: Applies to packages/wesley-host-node/package.json : Map the CLI binary in host-node package.json "bin" field to bin/wesley.mjs
Applied to files:
packages/wesley-holmes/src/cli.mjs
🧬 Code graph analysis (2)
packages/wesley-holmes/src/cli.mjs (2)
packages/wesley-holmes/test/cli-options.test.mjs (3)
bundlePath(86-86)historyPath(91-91)bundleDir(77-77)packages/wesley-host-node/src/adapters/WesleyFileWriter.mjs (2)
historyPath(97-97)history(98-98)
packages/wesley-holmes/test/cli-options.test.mjs (2)
packages/wesley-holmes/src/cli.mjs (5)
bundleDir(112-112)bundleDir(129-129)bundlePath(78-78)bundlePath(95-95)bundlePath(130-130)packages/wesley-holmes/src/Watson.mjs (1)
stderr(234-234)
🔇 Additional comments (15)
.github/actions/run-holmes-command/action.yml (3)
3-18: Inputs are now properly documented.Good. You fixed the undocumented environment variable disaster from before. Now callers can actually understand what this action needs without having to read the implementation. This should have been here from day one.
63-66: CLI invocation looks correct.You're passing absolute paths and explicit options. This is the right way to invoke the CLI from a workflow. No magic, no implicit assumptions about working directory.
70-73: Artifact paths are now absolute. Good.You fixed the relative path fragility. Everything uses
$GITHUB_WORKSPACEnow, so it won't break if someone runs this action from a subdirectory. This is how it should have been written the first time..github/workflows/wesley-holmes.yml (4)
46-50: Dashboard template artifact reduces checkout redundancy.You're uploading the template once from the first job, then downloading it later instead of checking out the entire repo again. This saves time and is cleaner. Good fix for the past review comment.
86-93: Duplicate code eliminated with composite action. Finally.You were copy-pasting the same shell block three times like an amateur. Now you've extracted it into a reusable action and the workflow is actually maintainable. This is what you should have done from the start, but at least you fixed it.
357-380: Dashboard assembly is defensive and correct.You use
findto locate the template instead of assuming a fixed path, validate it exists, then copy it. If the artifact layout changes, this won't silently fail. Good defensive programming.
422-441: Template indentation fix addresses the main PR objective.The YAML now parses correctly. The closing backtick is inside the script block. The spacing around separators is consistent. This is what the PR was supposed to fix, and it's fixed.
CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000001.jsonl (1)
32-38: Documentation accurately records the repair actions.The timeline entries match the changes made in this PR. Clear notes for future agents. This is how you document machine history.
packages/wesley-holmes/src/cli.mjs (4)
24-34: loadBundle path resolution is correct.You resolve the path first, then use the resolved path in error messages. Clear and correct. No magic relative path assumptions.
36-44: loadHistory gracefully handles missing history files.You default to an empty history instead of crashing. This is the right behavior for a history file that's built up over time. Good.
57-63: CLI options are clean and well-documented.
--bundle-dirand--history-fileare explicit and have sensible defaults. The help text explains what's needed. Good UX.
72-158: Command actions consistently use the new path resolution.All four commands (investigate, verify, predict, report) follow the same pattern: resolve paths from options, load data, generate output. The implementation is consistent and correct.
packages/wesley-holmes/test/cli-options.test.mjs (3)
75-107: Test fixture utility is well-designed.Isolated temp directories, configurable scenarios, proper cleanup. This is how you write test utilities. Good work.
109-153: runCli utility covers both success and failure cases.You spawn the real CLI, validate output, check artifacts, and clean up properly. The
finallyblock ensures cleanup even on assertion failures. This is proper end-to-end testing.
181-229: Test coverage is finally comprehensive. About damn time.You were called out for testing only 2 of 5 commands. Now you have all 5 commands covered, plus error paths for missing files and corrupted JSON. This is what you should have submitted the first time.
The tests are meaningful and check actual behavior, not just "does it exit 0". Good.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/wesley-holmes.yml (2)
397-411: Broken artifact path resolution — comment won’t find the MD reports.
download-artifact@v4 with merge-multiple nests by artifact name and often preserves parent dirs. Your two guesses miss the real paths.Replace readReport with a robust recursive finder:
- const readReport = (status, artifactDir, filename) => { - if (status !== 'success') { - return `_Report unavailable: upstream job status **${status}**_`; - } - const target = path.join('reports', artifactDir, filename); - const altTarget = path.join('reports', 'reports', artifactDir, filename); - if (!fs.existsSync(target)) { - if (fs.existsSync(altTarget)) { - return fs.readFileSync(altTarget, 'utf8'); - } - console.warn(`Expected report file missing: ${target}`); - return `_Report missing for ${artifactDir}_`; - } - return fs.readFileSync(target, 'utf8'); - }; + const findFirst = (root, filename, mustContain) => { + const stack = [root]; + while (stack.length) { + const dir = stack.pop(); + let entries = []; + try { entries = fs.readdirSync(dir, { withFileTypes: true }); } catch { continue; } + for (const e of entries) { + const full = path.join(dir, e.name); + if (e.isDirectory()) stack.push(full); + else if (e.isFile() && e.name === filename && (!mustContain || full.includes(`${path.sep}${mustContain}${path.sep}`))) { + return full; + } + } + } + return null; + }; + + const readReport = (status, reportName, filename) => { + if (status !== 'success') return `_Report unavailable: upstream job status **${status}**_`; + const hit = findFirst('reports', filename, reportName); + if (!hit) { + console.warn(`Report not found: ${reportName}/${filename}`); + return `_Report missing for ${reportName}_`; + } + return fs.readFileSync(hit, 'utf8'); + };Calls below stay the same.
1-67: ---This workflow addition violates repo policy—revert path MUST be documented in PR description.
The file is NEW, not a modification. It adds a full CI job with composite action invocation and artifact uploads. This is NOT a deterministic syntax fix or duplication reduction—it's a substantial new workflow. Per your repository standards, this requires explicit revert steps in the PR description.
Cannot verify compliance from sandbox, but the demand is non-negotiable.
♻️ Duplicate comments (5)
.github/workflows/wesley-holmes.yml (5)
88-94: Finally DRY. Composite action use looks sane.
You killed the copy-paste blocks. Good.
122-128: Repeatable invocation via action — approved.
Consistent args; avoids the prior directory gymnastics.
157-163: Same here for MORIARTY.
Consistent, minimal, readable.
351-356: Good: template now comes from artifact, not yet another checkout.
This fixes the waste flagged earlier.
437-441: Formatting fixed. Finally.
YAML parses; separators consistent; backtick inside script. Ship it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/wesley-holmes.yml(7 hunks)CHRONICLES_OF_THE_MACHINE-KIND_VOL_00000001.jsonl(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
.github/workflows/!({cli-tests.yml,cli-quick.yml})
📄 CodeRabbit inference engine (AGENTS.md)
Do not duplicate CLI end-to-end/bats checks in the main CI workflows without approval
Files:
.github/workflows/wesley-holmes.yml
.github/workflows/**
📄 CodeRabbit inference engine (AGENTS.md)
.github/workflows/**: Only touch .github/workflows/* when fixing a deterministic break (syntax/path/quoting) or reducing obviously wasteful runtime/duplication, and provide a revert path in the PR description
Use least privilege in workflows; do not widen permissions or add secrets without approval
Files:
.github/workflows/wesley-holmes.yml
1 participant
Summary
website/and add starter styling assetsTesting
Revert Plan
git revert -m 1 <merge-commit-sha>to roll back this PR; this will delete .github/actions/run-holmes-command/action.yml, restore .github/workflows/docs-site.yml to the MkDocs deployment, and remove thewebsite/directory.