Align JavaScript MCP scripts log renderer with Copilot output style

[Copilot]

Summary

Update the JavaScript MCP scripts log renderer to match Copilot-style tool output formatting.

Changes

• Render MCP tool activity as Copilot-style lines, e.g. ● <tool> (MCP: <server>) · args: "..."
• Render tool result previews on indented tree lines with └
• Preserve fallback output for non-tool logs so diagnostics are not lost
• Use dynamic MCP server labels from parsed log entries instead of a hardcoded server name
• Refactor tool call/result extraction to marker-based string parsing for safer, clearer parsing
• Harden parsing behavior and diagnostics matching
• Update parse_mcp_scripts_logs.test.cjs expectations for the new renderer format and fallback behavior
• Remove unintended .github/mcp.json from this branch

Validation

• npx vitest run parse_mcp_scripts_logs.test.cjs ✅
• make agent-finish ⚠️ fails due to pre-existing unrelated testifylint issues:
  • pkg/stats/spec_test.go (float-compare suggestions)
  • pkg/testutil/spec_test.go (require-error suggestion)

These lint failures were present before this task and are not caused by this change.

[Copilot]

Pull request overview

Updates the JavaScript MCP scripts log renderer to output Copilot-style tool activity lines (with args and an indented result preview), while retaining a fallback mode for non-tool logs and updating tests to match the new formatting.

Changes:

• Refactors generatePlainTextSummary to detect tool invocations/results and render them as Copilot-style lines.
• Adds a fallback renderer for runs with no recognized tool calls (capped output).
• Updates parse_mcp_scripts_logs.test.cjs expectations to match the new output format and truncation behavior.

Show a summary per file

File	Description
actions/setup/js/parse_mcp_scripts_logs.cjs	Implements Copilot-style rendering for MCP tool calls/results and a capped fallback renderer.
actions/setup/js/parse_mcp_scripts_logs.test.cjs	Updates unit tests to assert the new renderer output format and fallback truncation.

Copilot's findings

Tip

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

Comments suppressed due to low confidence (3)

actions/setup/js/parse_mcp_scripts_logs.cjs:261

• The fallback path truncates and whitespace-normalizes every raw log line to 150 chars (replace(/\s+/g, " ")). This can remove important troubleshooting detail (e.g., long error messages, structured blobs) even though this mode is intended to preserve non-tool logs. Consider avoiding whitespace normalization in fallback mode and/or increasing the per-line limit (or making it configurable) so diagnostics aren’t unintentionally lost.

  if (renderedCalls.length === 0) {
    // Fallback to raw logs when no recognizable tool calls exist.
    let lineCount = 0;
    for (const entry of logEntries) {
      if (lineCount >= 200) {
        lines.push(`... (truncated, showing first 200 lines of ${logEntries.length} total entries)`);
        break;
      }
      if (entry.raw) {
        lines.push(truncate(entry.message, 150));
      } else {
        const server = entry.serverName ? `[${entry.serverName}] ` : "";
        lines.push(truncate(`${server}${entry.message}`.trim(), 150));
      }

actions/setup/js/parse_mcp_scripts_logs.cjs:226

• The backend-tool parsing ignores the serverID=... field present in the log line and uses entry.serverName for the (MCP: ...) label. This can mislabel the MCP server being invoked (e.g., logs emitted by a gateway would show MCP: mcp-gateway instead of mcpscripts). Consider extracting serverID from the message and using that for the MCP label (falling back to entry.serverName only when serverID is absent).

    // Parse: "callBackendTool ... toolName=gh, args=map[args:pr view ...]"
    const backendToolPrefix = "toolName=";
    const backendArgsPrefix = "args=map[args:";
    const backendToolIndex = message.indexOf(backendToolPrefix);
    const backendArgsIndex = message.indexOf(backendArgsPrefix);
    if (backendToolIndex !== -1 && backendArgsIndex !== -1 && backendArgsIndex > backendToolIndex) {
      const toolPart = message.slice(backendToolIndex + backendToolPrefix.length, backendArgsIndex);
      const tool = toolPart.replace(",", "").trim();
      const argsStart = backendArgsIndex + backendArgsPrefix.length;
      const argsEnd = message.indexOf("]", argsStart);
      const argsRaw = argsEnd === -1 ? message.slice(argsStart) : message.slice(argsStart, argsEnd);
      const argsDisplay = ` · args: "${truncate(argsRaw, 90)}"`;
      const callIndex = renderedCalls.push({
        tool,
        serverName: entry.serverName || "mcpscripts",
        argsDisplay,
        resultPreview: "",
      });
      addPending(tool, callIndex - 1);
      continue;

actions/setup/js/parse_mcp_scripts_logs.cjs:272

• When at least one tool call is recognized, the summary renders only renderedCalls (plus limited error|failed diagnostics) and drops all other non-tool log lines entirely. That means startup/config/warning/info lines are lost in the most common case (tool usage present), which undermines the stated goal of preserving fallback diagnostics. Consider appending a small “Additional logs” section with a capped number of unmatched lines (or always including a capped raw-log tail) so important context isn’t discarded.

  for (const call of renderedCalls) {
    lines.push(`● ${call.tool} (MCP: ${call.serverName || "mcpscripts"})${call.argsDisplay}`);
    if (call.resultPreview) {
      lines.push(`  └ ${call.resultPreview}`);
    }
  }

• Files reviewed: 2/2 changed files
• Comments generated: 2

[Copilot]

This backend-tool test expects MCP: mcp-gateway, but the sample log line includes serverID=mcpscripts (likely the MCP server being invoked). If the renderer intends (MCP: ...) to reflect the target server, this expectation should be updated (and/or the renderer should extract and use serverID).

Suggested change

	expect(summary).toContain('● gh (MCP: mcp-gateway) · args: "pr view 26450 --repo github/gh-aw --json number"');
	expect(summary).toContain('● gh (MCP: mcpscripts) · args: "pr view 26450 --repo github/gh-aw --json number"');

[Copilot]

argsDisplay wraps the argument string in double quotes without escaping. If the underlying command/args contain " (common for shell/CLI invocations), the rendered line becomes ambiguous/broken (e.g. · args: "foo" bar"). Consider producing the quoted value via JSON.stringify(truncatedArgs) (or equivalent escaping) so embedded quotes/backslashes are rendered safely.

This issue also appears in the following locations of the same file:

• line 207
• line 248
• line 267

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 88/100

✅ Excellent test quality

Metric	Value
New/modified tests analyzed	28
✅ Design tests (behavioral contracts)	28 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	17 (61%)
Duplicate test clusters	0
Test inflation detected	No (ratio: 0.39)
🚨 Coding-guideline violations	None

---

Test Classification Details

View all 28 test classifications

Test	File	Classification	Notes
should parse valid mcp-scripts log line with standard format	parse_mcp_scripts_logs.test.cjs	✅ Design	Verifies parsed fields: timestamp, serverName, message, raw
should parse log line with tool registration message	parse_mcp_scripts_logs.test.cjs	✅ Design	Behavioral variant of format parsing
should parse log line with tool execution message	parse_mcp_scripts_logs.test.cjs	✅ Design	Behavioral variant of format parsing
should parse log line with error message	parse_mcp_scripts_logs.test.cjs	✅ Design	Verifies error-message content is preserved
should handle log line with extra whitespace	parse_mcp_scripts_logs.test.cjs	✅ Design	Edge case: whitespace trimming behavior
should return raw entry for unparseable log line	parse_mcp_scripts_logs.test.cjs	✅ Design	Edge case: raw=true for unrecognised format
should return raw entry for empty line	parse_mcp_scripts_logs.test.cjs	✅ Design	Edge case: empty string input
should parse log line with brackets in message	parse_mcp_scripts_logs.test.cjs	✅ Design	Edge case: special chars in message body
should render Copilot-style MCP tool lines with args and result preview	parse_mcp_scripts_logs.test.cjs	✅ Design	Core behavioral contract for new Copilot output style
should render backend tool call format in Copilot-style	parse_mcp_scripts_logs.test.cjs	✅ Design	Alternative input format renders correctly
should include multiple tool calls	parse_mcp_scripts_logs.test.cjs	✅ Design	Verifies all calls appear in output
should include diagnostics for error lines	parse_mcp_scripts_logs.test.cjs	✅ Design	Edge case: error diagnostics section appended
should fall back to raw logs when no tool calls are present	parse_mcp_scripts_logs.test.cjs	✅ Design	Edge case: fallback rendering path
should handle empty log entries (generatePlainTextSummary)	parse_mcp_scripts_logs.test.cjs	✅ Design	Edge case: empty input → empty string
should limit fallback raw logs to 200 lines	parse_mcp_scripts_logs.test.cjs	✅ Design	Boundary condition: truncation at 200 lines
should generate markdown summary with details/summary structure	parse_mcp_scripts_logs.test.cjs	✅ Design	Verifies markdown structure and statistics table
should generate markdown summary with tool execution details	parse_mcp_scripts_logs.test.cjs	✅ Design	Verifies tool execution table in output
should generate markdown summary with error details	parse_mcp_scripts_logs.test.cjs	✅ Design	Edge case: error section in markdown
should generate markdown summary with full logs section	parse_mcp_scripts_logs.test.cjs	✅ Design	Verifies full log section including raw entries
should handle empty log entries (generateMCPScriptsSummary)	parse_mcp_scripts_logs.test.cjs	✅ Design	Edge case: all stats show 0
should not show tool executions section when no tools called	parse_mcp_scripts_logs.test.cjs	✅ Design	Edge case: conditional section absent
should not show errors section when no errors	parse_mcp_scripts_logs.test.cjs	✅ Design	Edge case: conditional section absent
should handle missing logs directory	parse_mcp_scripts_logs.test.cjs	✅ Design	Edge case: missing directory → graceful no-op
should handle empty logs directory	parse_mcp_scripts_logs.test.cjs	✅ Design	Edge case: empty dir → graceful no-op
should process log files and generate summaries	parse_mcp_scripts_logs.test.cjs	✅ Design	Happy path: verifies summary written to GitHub Actions
should handle errors gracefully	parse_mcp_scripts_logs.test.cjs	✅ Design	Edge case: unexpected error → setFailed with code
should process multiple log files	parse_mcp_scripts_logs.test.cjs	✅ Design	Behavioral: both files processed and logged
should handle empty log entries (main)	parse_mcp_scripts_logs.test.cjs	✅ Design	Edge case: blank-line-only log → no-op message

---

Flagged Tests — Requires Review

No tests require review. All 28 tests are classified as design tests.

---

Notable Strengths

• New Copilot output style fully exercised: The two key rendering tests (should render Copilot-style MCP tool lines with args and result preview and should render backend tool call format in Copilot-style) directly pin the new ● tool (MCP: server) · args: "..." and └ result format as a behavioral contract — any regression in output format will be caught immediately.
• Boundary condition covered: The 200-line truncation test with 250 entries is a good boundary test.
• Filesystem mocking is appropriate: vi.spyOn(fs, ...) for external I/O and global.core for the GitHub Actions runtime are both legitimate mock targets.
• Conditional rendering tested: Tests for absent **Tool Executions** and **Errors** sections enforce correct conditional logic.

Minor Observation (non-blocking)

The main function tests assert on mockCore.info call content (e.g., "Found 1 mcp-scripts log file") which couples tests slightly to the exact wording of log messages. This is a borderline design/implementation choice — the tests remain valuable since logging IS the observable interface for a GitHub Actions step, but consider using expect.stringContaining on broader patterns rather than specific phrasing if these strings change frequently.

---

Language Support

Tests analyzed:

• 🟨 JavaScript (*.test.cjs): 28 tests (vitest) — actions/setup/js/parse_mcp_scripts_logs.test.cjs

---

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). No coding-guideline violations detected. Test inflation ratio: 0.39 (well under the 2:1 threshold with +58 test lines vs. +150 production lines).

---

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

• Assert on observable outputs, return values, or state changes
• Cover error paths and boundary conditions
• Would catch a behavioral regression if deleted
• Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

• Assert on internal function calls (mocking internals)
• Only test the happy path with typical inputs
• Break during legitimate refactoring even when behavior is correct
• Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

References: §24522996412

🧪 Test quality analysis by Test Quality Sentinel · ● 750.1K · ◷

[github-actions]

✅ Test Quality Sentinel: 88/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 28 tests verify behavioral contracts with strong edge case coverage across parsing, rendering, and main function paths.