Conversation
Closes the three open CodeRabbit findings from #466 not yet shipped (items #2 and #4 already landed via #470 and 9792b0c). #467 finding #1 (.claude/commands/rollcall.md): Hard-coded operator path /home/josh/dev/... → relative path scripts/agent-rollcall.sh with explicit "from repo root" guidance. Works for any clone; matches every other repo-script reference. #467 finding #3 (README.md env table): Add a dedicated env-table row for WORKSTACEAN_INTERNAL_HOST so it's discoverable by anyone overriding the docker-network default. Cross-references WORKSTACEAN_PUBLIC_BASE_URL. #467 finding #5 (src/index.ts startup-validator): validateActionExecutors() ran BEFORE loadWorkspacePlugins(), so executor registrars shipped as workspace plugins were falsely flagged in strict mode. It also ran only once at startup, so config.reload of actions.yaml bypassed the fail-loud guard. Fix: extract a runWiringValidator(reason) helper, call it AFTER all plugin loading (core + registered + workspace), and re-run inside the CONFIG_RELOAD subscriber after loadActionsYaml(). New "[reload-validator]" log tag distinguishes the call sites. Tests: 1054/1054 pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR fixes a script execution path to use relative paths, updates environment variable documentation for URL fallback behavior and the internal hostname, and refactors wiring validation to execute both at startup (after plugin loading) and during config reloads. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/index.ts`:
- Around line 575-582: The reload validator is mislabeling reload-time failures
as startup because validateActionExecutors currently hard-codes the log tag and
agentId; update the call site in runWiringValidator to pass explicit options
(e.g., logTag and agentId) when invoking validateActionExecutors for "reload" so
the logs show "[reload-validator]" and agentId "reload-validator", and modify
validateActionExecutors to accept and use those new options (e.g., logTag and
agentId) instead of the current hard-coded values in its logging and
unwired-reporting logic.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9e1aa2e2-d973-4555-993a-d0adf07c40be
📒 Files selected for processing (3)
.claude/commands/rollcall.mdREADME.mdsrc/index.ts
Comment on lines
+575
to
+582
| const unwired = validateActionExecutors(actionRegistry, executorRegistry, { | ||
| bus, | ||
| throwOnUnwired: process.env.WORKSTACEAN_STRICT_WIRING === "1", | ||
| }); | ||
| if (unwired.length === 0) { | ||
| const tag = reason === "startup" ? "[startup-validator]" : "[reload-validator]"; | ||
| console.log(`${tag} All ${actionRegistry.size} action(s) have a registered executor.`); | ||
| } |
There was a problem hiding this comment.
Reload failures still log as startup validator.
On Line 575, runWiringValidator("reload") still delegates to validateActionExecutors, which currently hard-codes [startup-validator] logs and agentId: "startup-validator" for unwired actions (src/planner/validate-action-executors.ts:104-154). This makes reload-time failures look like startup failures in ops alerts.
Suggested fix
-const { validateActionExecutors } = await import("./planner/validate-action-executors.js");
+const { validateActionExecutors } = await import("./planner/validate-action-executors.js");
function runWiringValidator(reason: "startup" | "reload"): void {
+ const tag = reason === "startup" ? "[startup-validator]" : "[reload-validator]";
const unwired = validateActionExecutors(actionRegistry, executorRegistry, {
bus,
throwOnUnwired: process.env.WORKSTACEAN_STRICT_WIRING === "1",
+ logTag: tag,
+ agentId: reason === "startup" ? "startup-validator" : "reload-validator",
});
if (unwired.length === 0) {
- const tag = reason === "startup" ? "[startup-validator]" : "[reload-validator]";
console.log(`${tag} All ${actionRegistry.size} action(s) have a registered executor.`);
}
}(Companion change needed in src/planner/validate-action-executors.ts to accept/use logTag and agentId options.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/index.ts` around lines 575 - 582, The reload validator is mislabeling
reload-time failures as startup because validateActionExecutors currently
hard-codes the log tag and agentId; update the call site in runWiringValidator
to pass explicit options (e.g., logTag and agentId) when invoking
validateActionExecutors for "reload" so the logs show "[reload-validator]" and
agentId "reload-validator", and modify validateActionExecutors to accept and use
those new options (e.g., logTag and agentId) instead of the current hard-coded
values in its logging and unwired-reporting logic.
mabry1985
pushed a commit
that referenced
this pull request
Apr 22, 2026
…s-strategy back-merge The prior back-merge of main → dev (#468) was force-squashed when CodeRabbit froze, dropping main as an ancestor of dev. This back-merge re-establishes that ancestry, but the ort/ours strategy preserved main's pre-#474 validator block alongside dev's new runWiringValidator() helper — both were present. Drop the older duplicate so dev's #474 layout stands: validator runs once, AFTER loadWorkspacePlugins(), and re-runs on config.reload. Tests: 1059/1059 pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the three remaining open CodeRabbit findings from #466's review on the v0.7.22 promotion. Items #2 (ttlMs
Number.isFinite) and #4 (gatepr-remediator-skill-executoron GitHub creds) already shipped via #470 and 9792b0c.Findings cleared in this PR
#1 —
.claude/commands/rollcall.mdhard-coded path (🔴 per CodeRabbit)Replaced
/home/josh/dev/protoWorkstacean/scripts/agent-rollcall.shwith the relativescripts/agent-rollcall.shand explicit "from the repo root" instruction. Works for any operator's clone.#3 — README env table missing
WORKSTACEAN_INTERNAL_HOST(🟡)Added a dedicated row that cross-references
WORKSTACEAN_PUBLIC_BASE_URL. Anyone running outside the default docker network now finds the override directly.#5 — startup wiring validator load-order + reload coverage (🟠)
Two real gaps:
loadWorkspacePlugins(), so executor registrars shipped as workspace plugins were falsely flagged in strict mode.actions.yamlentries loaded viaTOPICS.CONFIG_RELOADbypassed the fail-loud guard until next restart.Fix:
runWiringValidator(reason: "startup" | "reload")helper.CONFIG_RELOADsubscriber afterloadActionsYaml().[reload-validator]log tag distinguishes the call sites.What's NOT in this PR
Test plan
bun test— 1054/1054 pass:dev: confirm[startup-validator]line still emits at boot:dev: trigger aconfig.reloadand verify[reload-validator]log line appears🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
WORKSTACEAN_PUBLIC_BASE_URLis unset.WORKSTACEAN_INTERNAL_HOSTenvironment variable.Improvements