fix/pr 443 ci/cd failures by jaeko44 · Pull Request #452 · virtengine/bosun

jaeko44 · 2026-03-26T06:46:18Z

No description provided.

Copilot

Pull request overview

This PR appears aimed at addressing CI/CD-related failures by (a) adding a “bosun-created” marker into GitHub workflow template expressions and (b) introducing persisted “tool overhead” (serialized tool definition char counts) reporting surfaced via bosun --workspace-status, with accompanying tests.

Changes:

Added bosun-created marker comments to GitHub workflow template expression strings.
Added tool-definition char measurement + persisted overhead reporting in agent/agent-tool-config.mjs, invoked from the primary agent and displayed in CLI workspace status.
Added Vitest coverage for tool overhead measurement and CLI output.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
workflow-templates/github.mjs	Injects `bosun-created` marker into template expression strings.
agent/agent-tool-config.mjs	Adds tool definition char measurement and persisted overhead reporting APIs.
agent/primary-agent.mjs	Triggers background refresh of the overhead report for the active agent profile.
cli.mjs	Displays tool overhead summary during `--workspace-status`.
tests/cli-tool-overhead-status.test.mjs	Verifies workspace-status output includes tool overhead totals and warnings.
tests/agent-tool-config.test.mjs	Verifies tool definition char measurement works for custom and builtin tool defs.

Comments suppressed due to low confidence (2)

agent/agent-tool-config.mjs:241

loadToolConfig() returns toolOverhead: {} only for the non-existent-config path, but it never returns parsed.toolOverhead (or an empty default) when the config file exists or when JSON parse fails. As a result, getToolOverheadReport() will always return {total:0} even if agent-tools.json contains toolOverhead, and persistToolOverheadReport() will overwrite/drop any existing per-agent overhead entries on every refresh. Fix by including toolOverhead: parsed.toolOverhead || {} (and a default in the catch path), and consider updating the header schema / @returns docs accordingly.

export function loadToolConfig(rootDir) {
  const configPath = getConfigPath(rootDir);
  if (!existsSync(configPath)) {
    return {
      agents: {},
      defaults: {
        builtinTools: DEFAULT_BUILTIN_TOOLS.filter((t) => t.default).map((t) => t.id),
        updatedAt: new Date().toISOString(),
      },
      toolOverhead: {},
    };
  }
  try {
    const raw = readFileSync(configPath, "utf8");
    const parsed = JSON.parse(raw);
    return {
      agents: parsed.agents || {},
      defaults: parsed.defaults || {
        builtinTools: DEFAULT_BUILTIN_TOOLS.filter((t) => t.default).map((t) => t.id),
        updatedAt: new Date().toISOString(),
      },
    };

cli.mjs:2103

The tool overhead block is currently inside the else branch for summary.length === 0, so --workspace-status will never print tool overhead when no workspaces are configured (it prints only “No workspaces configured.”). The added test sets workspaces: [] and expects tool overhead output, so this will fail; consider moving the tool overhead reporting outside the if/else so it always renders (even when there are zero workspaces).

    const summary = getWorkspaceStateSummary(configDir);
    if (summary.length === 0) {
      console.log("\n  No workspaces configured.\n");
    } else {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-26T06:52:39Z

cli.mjs

+      const toolOverhead = getToolOverheadReport(configDir, "primary");
+      const overheadSources = Object.entries(toolOverhead.bySource || {});
+      if (toolOverhead.total > 0 || overheadSources.length > 0) {
+        console.log("\n  Tool Overhead:");
+        console.log(`    Total tool chars: ${toolOverhead.total.toLocaleString("en-US")}`);


getToolOverheadReport(configDir, "primary") passes the CLI config directory (often the .bosun dir) into agent-tool-config, but getConfigPath() in that module currently appends an extra .bosun segment. This makes it look for <configDir>/.bosun/agent-tools.json (and also makes the fallback getBosunHome() case look under ~/.bosun/.bosun/agent-tools.json), so persisted overhead won’t be found. Either pass the repo root here, or update agent-tool-config to accept a configDir (detect if the input path is already the bosun config dir) and resolve agent-tools.json accordingly.

Copilot · 2026-03-26T06:52:39Z

agent/primary-agent.mjs

    ? rawCfg.enabledMcpServers.map((id) => String(id || "").trim()).filter(Boolean)
    : [];
+  if (agentProfileId) {
+    void refreshToolOverheadReport(rootDir, agentProfileId, { serverIds: enabledMcpServers });


void refreshToolOverheadReport(...) drops the returned promise without a .catch(). refreshToolOverheadReport() performs filesystem writes via saveToolConfig() and can reject on I/O errors; as written, that would surface as an unhandled promise rejection in the primary agent path. Wrap this in a fire-and-forget handler that catches/logs errors (or make refreshToolOverheadReport() internally swallow/return best-effort results), and consider throttling to avoid rewriting agent-tools.json on every prompt build.

Suggested change

void refreshToolOverheadReport(rootDir, agentProfileId, { serverIds: enabledMcpServers });

refreshToolOverheadReport(rootDir, agentProfileId, { serverIds: enabledMcpServers }).catch((err) => {

console.error("[primary-agent] Failed to refresh tool overhead report:", err);

});

Copilot · 2026-03-26T06:52:40Z

workflow-templates/github.mjs

    node("automation-eligible", "condition.expression", "Bosun-Created PR?", {
      expression:
-        "(() => { if ($data?.requireBosunCreatedPr !== true && String($data?.requireBosunCreatedPr || '').toLowerCase() !== 'true') return true; const raw = $ctx.getNodeOutput('load-pr-context')?.output || '{}'; let pr = {}; try { pr = typeof raw === 'string' ? JSON.parse(raw) : raw; } catch { return false; } const labels = Array.isArray(pr?.labels) ? pr.labels.map((entry) => typeof entry === 'string' ? entry : entry?.name).filter(Boolean) : []; return labels.includes('bosun-pr-bosun-created'); })()",
+        "(() => { /* <!-- bosun-created --> */ if ($data?.requireBosunCreatedPr !== true && String($data?.requireBosunCreatedPr || '').toLowerCase() !== 'true') return true; const raw = $ctx.getNodeOutput('load-pr-context')?.output || '{}'; let pr = {}; try { pr = typeof raw === 'string' ? JSON.parse(raw) : raw; } catch { return false; } const labels = Array.isArray(pr?.labels) ? pr.labels.map((entry) => typeof entry === 'string' ? entry : entry?.name).filter(Boolean) : []; return labels.includes('bosun-pr-bosun-created'); })()",
    }, { x: 400, y: 230, outputs: ["yes", "no"] }),


This workflow template change will alter the generated demo defaults payload (it’s included verbatim by tools/generate-demo-defaults.mjs). The repo has a sync test that expects ui/demo-defaults.js and site/ui/demo-defaults.js to be regenerated whenever workflow templates change; please rerun node tools/generate-demo-defaults.mjs and commit the updated demo-defaults files to keep tests passing.

Co-authored-by: bosun-ve[bot] <262908237+bosun-ve[bot]@users.noreply.github.com>

jaeko44 · 2026-03-30T16:01:50Z