Protocol Evals#1066
Closed
willwashburn wants to merge 5 commits into
Closed
Conversation
Introduce a new integration eval suite that exercises agent-to-agent messaging via the broker and scores protocol adherence (message-sent rate, phantom messages, ACK/DONE protocol, wrong-channel replies). Adds a full eval runner, scenarios, deterministic scoring, reporters (JSON + self-contained HTML viewer), a matrix roll-up, unit tests for scoring, and CLI helpers under tests/integration/broker/evals. Adds npm scripts (eval:build, eval:unit, eval:selftest, eval:toolcheck, eval:html, eval, eval:claude, eval:matrix) and gitignore entry for evals-reports. Also adds a Fleet Delivery design doc (specs/fleet-delivery.md), updates CHANGELOG.md, and adjusts integration test config/files (tsconfig, vitest, and broker harness utilities) to align the broker-harness with the current SDK/harness-driver APIs so the evals build/run cleanly.
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
Contributor
📝 WalkthroughWalkthroughThis PR introduces a comprehensive agent messaging evaluation suite for broker integration testing (6 scenarios with phantom detection, protocol scoring, and reporting), adds local workflow CLI commands ( ChangesBroker Eval Suite
Local Workflow CLI
Broker Harness Refactoring
Fleet Delivery Specification and Documentation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a local workflow execution feature (agent-relay local run|logs|sync) that delegates execution to @relayflows/cli, alongside a comprehensive agent messaging evaluation suite with HTML and JSON reporting. The feedback highlights several critical improvements: resolving the exact entrypoint path for @relayflows/cli to prevent runtime errors, enhancing robustness in process liveness checks and atomic file writes, and fixing Windows compatibility issues by using fileURLToPath instead of .pathname for path resolution. Additionally, a style guide violation was noted in CHANGELOG.md, where the entry should be made more concise and impact-first.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| cwd: () => process.cwd(), | ||
| env: process.env, | ||
| spawnProcess, | ||
| resolveRelayflowsCliEntrypoint: () => nodeRequire.resolve('@relayflows/cli'), |
There was a problem hiding this comment.
The @relayflows/cli package does not define a main entrypoint in its package.json. Calling nodeRequire.resolve('@relayflows/cli') will throw a MODULE_NOT_FOUND error at runtime. Resolve the exact CLI script path @relayflows/cli/dist/cli.js instead.
Suggested change
| resolveRelayflowsCliEntrypoint: () => nodeRequire.resolve('@relayflows/cli'), | |
| resolveRelayflowsCliEntrypoint: () => nodeRequire.resolve('@relayflows/cli/dist/cli.js'), |
Comment on lines
+74
to
+82
| isProcessRunning: (pid) => { | ||
| if (!Number.isInteger(pid) || pid <= 0) return false; | ||
| try { | ||
| process.kill(pid, 0); | ||
| return true; | ||
| } catch (error) { | ||
| return (error as NodeJS.ErrnoException).code === 'EPERM'; | ||
| } | ||
| }, |
There was a problem hiding this comment.
If process.kill throws an error that is not an object or is null, accessing (error as NodeJS.ErrnoException).code will throw a TypeError and crash the process. Add a safety check to ensure error is a valid object with a code property before accessing it.
Suggested change
| isProcessRunning: (pid) => { | |
| if (!Number.isInteger(pid) || pid <= 0) return false; | |
| try { | |
| process.kill(pid, 0); | |
| return true; | |
| } catch (error) { | |
| return (error as NodeJS.ErrnoException).code === 'EPERM'; | |
| } | |
| }, | |
| isProcessRunning: (pid) => { | |
| if (!Number.isInteger(pid) || pid <= 0) return false; | |
| try { | |
| process.kill(pid, 0); | |
| return true; | |
| } catch (error) { | |
| return typeof error === "object" && error !== null && "code" in error && (error as any).code === "EPERM"; | |
| } | |
| }, |
Comment on lines
+162
to
+166
| async function writeJsonAtomic(filePath: string, value: unknown): Promise<void> { | ||
| const tmpPath = `${filePath}.tmp-${process.pid}`; | ||
| await fsp.writeFile(tmpPath, `${JSON.stringify(value, null, 2)}\n`, 'utf-8'); | ||
| await fsp.rename(tmpPath, filePath); | ||
| } |
There was a problem hiding this comment.
Using ${filePath}.tmp-${process.pid} as the temporary file path can lead to write conflicts and race conditions if writeJsonAtomic is called concurrently for the same file within the same process. Append a unique suffix using randomBytes to ensure uniqueness.
Suggested change
| async function writeJsonAtomic(filePath: string, value: unknown): Promise<void> { | |
| const tmpPath = `${filePath}.tmp-${process.pid}`; | |
| await fsp.writeFile(tmpPath, `${JSON.stringify(value, null, 2)}\n`, 'utf-8'); | |
| await fsp.rename(tmpPath, filePath); | |
| } | |
| async function writeJsonAtomic(filePath: string, value: unknown): Promise<void> { | |
| const tmpPath = filePath + ".tmp-" + process.pid + "-" + randomBytes(4).toString("hex"); | |
| await fsp.writeFile(tmpPath, JSON.stringify(value, null, 2) + "\n", "utf-8"); | |
| await fsp.rename(tmpPath, filePath); | |
| } |
Comment on lines
+4
to
+6
| import { execSync } from 'node:child_process'; | ||
| import fs from 'node:fs'; | ||
| import path from 'node:path'; |
There was a problem hiding this comment.
Import fileURLToPath from node:url to safely resolve file URLs to platform-specific paths.
Suggested change
| import { execSync } from 'node:child_process'; | |
| import fs from 'node:fs'; | |
| import path from 'node:path'; | |
| import { execSync } from 'node:child_process'; | |
| import fs from 'node:fs'; | |
| import path from 'node:path'; | |
| import { fileURLToPath } from 'node:url'; |
| /** Directory where reports are written (gitignored). */ | ||
| export function reportsDir(): string { | ||
| // Compiled location: dist/evals/report/ → scenario source lives under evals/. | ||
| return path.resolve(path.dirname(new URL(import.meta.url).pathname), '..', '..', '..', 'evals-reports'); |
There was a problem hiding this comment.
Using new URL(import.meta.url).pathname to resolve paths breaks on Windows because it leaves a leading slash (e.g., /C:/path). Use fileURLToPath from node:url for cross-platform compatibility.
Suggested change
| return path.resolve(path.dirname(new URL(import.meta.url).pathname), '..', '..', '..', 'evals-reports'); | |
| return path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..', '..', 'evals-reports'); |
Comment on lines
+12
to
+13
| import fs from 'node:fs'; | ||
| import path from 'node:path'; |
There was a problem hiding this comment.
Import fileURLToPath from node:url to safely resolve file URLs to platform-specific paths.
Suggested change
| import fs from 'node:fs'; | |
| import path from 'node:path'; | |
| import fs from 'node:fs'; | |
| import path from 'node:path'; | |
| import { fileURLToPath } from 'node:url'; |
|
|
||
| function repoRoot(): string { | ||
| // dist/evals/toolcheck-cli.js → up 5 to repo root. | ||
| return path.resolve(path.dirname(new URL(import.meta.url).pathname), '../../../../..'); |
There was a problem hiding this comment.
Using new URL(import.meta.url).pathname to resolve paths breaks on Windows because it leaves a leading slash (e.g., /C:/path). Use fileURLToPath from node:url for cross-platform compatibility.
Suggested change
| return path.resolve(path.dirname(new URL(import.meta.url).pathname), '../../../../..'); | |
| return path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../../../../..'); |
|
|
||
| ### Added | ||
|
|
||
| - Agent messaging eval suite (`npm run eval`, `eval:matrix`, `eval:unit`, `eval:selftest`, `eval:toolcheck`) spawns real agent CLIs and scores, from broker events, whether agents actually used the MCP/CLI to message — reporting message-sent rate, phantom-message rate (intent stated in prose but no send), ACK/DONE protocol adherence, and wrong-channel replies. Includes a `realistic` tier (natural-language tasks where the protocol must come from the injected onboarding) and a `smoke` tier (leading prompts as a plumbing canary), a negative-control self-test, and a deterministic wrong-tool-name trap that flags onboarding referencing tools the MCP server doesn't register. Each run emits a self-contained HTML viewer (overview, per-scenario prompts, full message transcript, phantom call-outs) alongside JSON reports with baseline regression diffing. |
There was a problem hiding this comment.
The changelog entry is extremely detailed and contains implementation specifics. According to the repository style guide, changelog entries should be concise and impact-first.
Suggested change
| - Agent messaging eval suite (`npm run eval`, `eval:matrix`, `eval:unit`, `eval:selftest`, `eval:toolcheck`) spawns real agent CLIs and scores, from broker events, whether agents actually used the MCP/CLI to message — reporting message-sent rate, phantom-message rate (intent stated in prose but no send), ACK/DONE protocol adherence, and wrong-channel replies. Includes a `realistic` tier (natural-language tasks where the protocol must come from the injected onboarding) and a `smoke` tier (leading prompts as a plumbing canary), a negative-control self-test, and a deterministic wrong-tool-name trap that flags onboarding referencing tools the MCP server doesn't register. Each run emits a self-contained HTML viewer (overview, per-scenario prompts, full message transcript, phantom call-outs) alongside JSON reports with baseline regression diffing. | |
| - Add agent messaging eval suite to spawn real agent CLIs and score protocol adherence, phantom messages, and wrong-channel replies, with HTML and JSON reporting. |
References
- Changelog entries should be concise and impact-first, preferring one short bullet per user-visible change. (link)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 431c227474
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+378
to
+382
| status: current.status === 'starting' ? 'running' : current.status, | ||
| monitorPid: monitor.pid, | ||
| updatedAt: deps.now().toISOString(), | ||
| }; | ||
| await writeJsonAtomic(metadataPath, next); |
There was a problem hiding this comment.
For very fast workflows, the detached monitor can finish and write status: 'completed' after the parent reads the record but before this parent-side write runs. In that race, this update writes the stale current.status === 'starting' ? 'running' : current.status value back to disk, erasing the terminal status; subsequent logs/sync calls then see a non-running monitor PID and mark the successful run as failed. Re-read/merge only the PID fields or avoid changing status here when the monitor owns terminal state.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (2)
package.json (1)
49-49: ⚡ Quick winScope
eval:unitto unit-test files only.
eval:unitcurrently runs the wholetests/integration/broker/evalstree, which can pull in non-unit eval runners. Limit it to*.unit.test.tsso this command stays fast and deterministic.Suggested change
- "eval:unit": "vitest run tests/integration/broker/evals", + "eval:unit": "vitest run tests/integration/broker/evals/**/*.unit.test.ts",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 49, The "eval:unit" npm script currently runs the entire tests/integration/broker/evals folder; update the "eval:unit" script entry in package.json (the "eval:unit" key) to restrict the glob to only unit test files (e.g., tests/integration/broker/evals/**/*.unit.test.ts) so it only executes files matching *.unit.test.ts, keeping the command fast and deterministic.tests/integration/broker/evals/scoring/protocol.unit.test.ts (1)
78-113: ⚡ Quick winAdd a regression test for channel-vs-DM target-kind mismatches in relay hops.
Current relay tests don’t guard against
#namebeing treated as equivalent toname. A focused regression test here will prevent future scoring drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/broker/evals/scoring/protocol.unit.test.ts` around lines 78 - 113, Add a regression test that ensures channel names with a leading '#' are not treated as equivalent to the same name without it: in the scoreRelayChain tests create a hop sequence (hops) that targets '`#general`' and then craft events using inbound where the final recipient is 'general' (no '#') and assert that scoreRelayChain(hops, events, ...) stops at the previous hop (hopsCompleted less than full length) and payloadIntact is false; reference the existing scoreRelayChain function, the hops array, and the inbound helper when adding this test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 12: The long single `[Unreleased]` changelog bullet in CHANGELOG.md
should be replaced by several concise, impact-first one-line bullets: extract
each user-visible change (e.g., "Adds agent messaging evaluation suite with HTML
and JSON reports", "Supports realistic and smoke tiers for protocol-driven
tasks", "Reports message-sent rate, phantom-message rate, ACK/DONE and
wrong-channel detection", "Includes negative-control self-test and deterministic
wrong-tool-name trap") and write each as a short standalone line; remove
implementation details and internal notes, avoid PR/issue links, and ensure each
bullet is brief and focused on user-facing impact.
In `@specs/fleet-delivery.md`:
- Around line 108-112: The fenced code block containing the state diagram (the
block starting with "queued ──deliver(seq)──▶ delivered ──ack──▶ acked (≈read)")
needs a language hint for proper rendering; update the opening triple-backtick
to include a language specifier (e.g., ```text or ```plaintext) so the diagram
is treated as plain text by the renderer.
- Around line 74-82: Add a language identifier to the fenced code block that
contains the spawn signature and placement pseudocode (the block starting with
"spawn { capability, node?: <name> | \"self\", session_ref?, ttl_override? }"
and the subsequent eligible/place rules); update the opening backticks to
include a hint like pseudocode or text (e.g., ```pseudocode) so rendering and
tooling can recognize the block as code.
In `@tests/integration/broker/evals/eval.test.ts`:
- Line 42: The test currently swallows errors from harness.stop() which hides
shutdown failures; change the teardown in eval.test.ts to let harness.stop()
errors surface by removing the .catch(() => {}) suppression and instead await
harness.stop() directly (or catch and rethrow/log the error) so any failure in
harness.stop() is reported by the test runner; update the test wrapper that
calls harness.stop() to either await harness.stop() without a silent catch or
handle the caught error by failing the test (e.g., rethrowing) rather than
ignoring it.
In `@tests/integration/broker/evals/runner.ts`:
- Around line 201-205: The test runner currently skips missing CLIs inside the
loop over flags.harnesses (using isCliAvailable(cli)) but still exits 0 if none
ran; add a boolean or counter (e.g., ranAny) that you set when a harness
actually executes (inside the branch where you run the harness), and after the
loop check it and fail the process (process.exit(1) or throw an Error) if ranAny
is false; apply the same change to the second similar loop referenced around the
other occurrence so the runner returns non-zero when every requested CLI was
missing.
- Around line 215-225: The baseline compare currently runs for every report
regardless of compatibility; before calling
compareReports(readReport(flags.baseline), report) and reporting regressions,
guard that the baseline report is compatible by loading the baseline via
readReport(flags.baseline) into a variable and checking that baseline.harness
=== report.harness and that the scenario/metric sets (e.g., lengths or matching
scenario identifiers/totals) match; only call compareReports and set
anyRegression when those compatibility checks pass, otherwise skip comparison or
log a clear message. Ensure you reference the existing symbols: flags.baseline,
readReport, compareReports, report, and any properties like harness or scenario
totals on the returned EvalReport.
In `@tests/integration/broker/evals/scenarios/01-dm-roundtrip.ts`:
- Line 47: The current PASS bit uses a generic send count (pass uses base.sent
>= 1) which can be satisfied by Bob sending to anyone; instead require an
explicit Bob→Alice reply signal: replace the base.sent >= 1 clause with an
event-level or protocol-scorer check that verifies at least one message with
from === 'bob' and to === 'alice' (for example: events.some(e => e.from ===
'bob' && e.to === 'alice') or the equivalent protocol scorer flag), while
leaving the existing base.phantoms.length === 0 and base.deliveryOk guards
intact; update the pass expression to use that reply-check + the
phantom/delivery checks (referencing the pass variable, base.sent,
base.phantoms, base.deliveryOk, and the events/protocol-scorer used for reply
detection).
In `@tests/integration/broker/evals/scenarios/04-relay-chain.ts`:
- Around line 44-50: The final waiter currently resolves on any relay_inbound
from participant c; tighten its predicate in the harness.waitForEvent call so it
only resolves when that inbound is the actual final post to "`#general`" by adding
a check that the event's destination/channel indicates "`#general`" (for example
by verifying event.to or event.channel equals "`#general`") in addition to e.kind
=== 'relay_inbound' and e.from === c; update the finalWaiter predicate
accordingly so the wait only completes on the true final-hop event.
In `@tests/integration/broker/evals/scenarios/helpers.ts`:
- Around line 36-39: waitForSends currently returns immediately once seen() >=
count which allows trailing sends to arrive after it returns; modify
waitForSends so that after the main loop completes (and only if deadline wasn't
hit) it waits for a short "quiet window" before returning: record the current
seen() value and poll (using the same setTimeout 500ms interval) until seen()
remains unchanged for the quietWindow duration (e.g., 200–500ms) or deadline is
reached. Reference the existing helpers waitForSends, seen(), count, and
deadline when implementing this post-condition wait so the function only returns
once no new sends arrive during the quiet window.
In `@tests/integration/broker/evals/scenarios/r01-incidental-report.ts`:
- Around line 44-48: The current pass criteria uses sentDirectMessage(events,
worker) which treats any DM as success and can be satisfied by an ACK; change
the check to assert the DM actually contains the computed result. Update the
test to either call a new helper (e.g., sentDirectMessageWithResult or
sentDirectMessageContainsResult) or inspect events for an outgoing DM from the
worker whose payload contains the expected result/marker (for example a "result"
field or the actual computed value), then use that boolean in place of reported;
keep the other checks (base.phantoms.length === 0 && base.deliveryOk) unchanged.
In `@tests/integration/broker/evals/scenarios/r02-forget-to-report.ts`:
- Around line 47-51: The current reported check uses sentDirectMessage(events,
worker) which returns true for ACK-only responses, so update the check to assert
that a direct message with actual findings was sent: replace or augment the
reported variable (from sentDirectMessage(events, worker)) with a check that
inspects the events for a sent message to the worker whose payload contains
non-empty findings (e.g., event.type/message entries with payload.findings
length > 0 or equivalent), and then use that stricter reported in the pass
condition (still keeping base.phantoms.length === 0 && base.deliveryOk and
releaseAgent handling as-is).
In `@tests/integration/broker/evals/scoring/metrics.ts`:
- Line 40: The phantomRate calculation can exceed 1 if phantomCount >
totalIntents; update the phantomRate expression (where phantomRate is computed
using phantomCount and totalIntents) to clamp the result into [0,1] like the
other rate calculations by wrapping the computed fraction with a clamp (e.g.,
Math.max(0, Math.min(1, ...))) while preserving the existing zero-division guard
(totalIntents > 0 ? ... : 0).
In `@tests/integration/broker/evals/scoring/metrics.unit.test.ts`:
- Around line 6-21: The test fixture function result must include the required
ScenarioResult fields agents and transcript; update the result(...) return
object in tests/integration/broker/evals/scoring/metrics.unit.test.ts (the
result function) to add agents: [] as AgentInfo[] and transcript: [] as
TranscriptEntry[] (or minimal valid entries) so the returned object satisfies
the ScenarioResult type contract while preserving existing overrides.
In `@tests/integration/broker/evals/scoring/phantom.ts`:
- Around line 146-155: The current matching loop (variables intents, sends,
consumed, normalizeTarget and matchIdx) allows any earlier unconsumed send to
satisfy an intent; change it so a send is only considered if it occurs at or
after the intent in event order. Concretely, iterate intents with an index (or
otherwise obtain the intent's event index) and update both findIndex predicates
to require the send index i >= intentIndex (e.g., (s, i) => i >= intentIndex &&
!consumed[i] && normalizeTarget(s.target) === intent.target and ( _, i) => i >=
intentIndex && !consumed[i]) so only later-or-equal sends can match an intent.
In `@tests/integration/broker/evals/scoring/protocol.ts`:
- Around line 18-20: normalizeChannel currently strips the leading '#' or '@',
conflating channel vs DM identities; update normalizeChannel to preserve the
target kind by retaining the leading marker: trim input, detect if first char is
'#' or '@', lowercase and trim the rest, and return the normalized string
including the original prefix (e.g., "`#general`" or "`@alice`"); then update all
call sites that rely on the old behavior (the other occurrences around the
109-121 block) to use the preserved-prefix form for comparisons so channel and
direct targets remain distinct.
In `@tests/integration/broker/evals/selftest.ts`:
- Around line 59-69: The two process.exit(1) calls inside the try block (the
checks using evalWouldPass and score.sent) prevent the finally cleanup
(releaseAgent/stop) from running; replace those process.exit(1) calls with
throwing descriptive Errors (e.g., throw new Error('SELF-TEST FAILED: ...')) or
returning a rejected Promise so the error propagates out of the try and the
finally block that calls releaseAgent and stop always executes; update the error
messages to match the existing console.error text and remove the direct
process.exit usage in the try.
In `@tests/integration/broker/evals/toolcheck-cli.ts`:
- Around line 25-27: The regex used to extract registered tool names from
mcpSource in the registerTool(...) matcher is too narrow (only [a-z0-9_]) and
drops valid names with characters like -, ., : (and possibly uppercase); update
the regex assigned to re in toolcheck-cli.ts to expand the character class to
include dash, dot, colon and uppercase letters (e.g., include A-Z and the
characters - . :) and ensure the hyphen is either escaped or placed at the
start/end of the class so it is treated literally, then keep the same matchAll
logic and return value.
---
Nitpick comments:
In `@package.json`:
- Line 49: The "eval:unit" npm script currently runs the entire
tests/integration/broker/evals folder; update the "eval:unit" script entry in
package.json (the "eval:unit" key) to restrict the glob to only unit test files
(e.g., tests/integration/broker/evals/**/*.unit.test.ts) so it only executes
files matching *.unit.test.ts, keeping the command fast and deterministic.
In `@tests/integration/broker/evals/scoring/protocol.unit.test.ts`:
- Around line 78-113: Add a regression test that ensures channel names with a
leading '#' are not treated as equivalent to the same name without it: in the
scoreRelayChain tests create a hop sequence (hops) that targets '`#general`' and
then craft events using inbound where the final recipient is 'general' (no '#')
and assert that scoreRelayChain(hops, events, ...) stops at the previous hop
(hopsCompleted less than full length) and payloadIntact is false; reference the
existing scoreRelayChain function, the hops array, and the inbound helper when
adding this test.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f3446c3-1869-4ca5-9ccc-22e3c60dc513
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (48)
.agentworkforce/trajectories/active/traj_b1jrutolckfb/trajectory.json.gitignoreCHANGELOG.mdpackage.jsonpackages/cli/README.mdpackages/cli/package.jsonpackages/cli/src/cli/bootstrap.test.tspackages/cli/src/cli/bootstrap.tspackages/cli/src/cli/commands/local-workflow.test.tspackages/cli/src/cli/commands/local-workflow.tsspecs/fleet-delivery.mdtests/integration/broker/evals/README.mdtests/integration/broker/evals/eval.test.tstests/integration/broker/evals/report/html.tstests/integration/broker/evals/report/render-cli.tstests/integration/broker/evals/report/write.tstests/integration/broker/evals/runner.tstests/integration/broker/evals/scenarios/01-dm-roundtrip.tstests/integration/broker/evals/scenarios/02-channel-reply.tstests/integration/broker/evals/scenarios/03-ack-done.tstests/integration/broker/evals/scenarios/04-relay-chain.tstests/integration/broker/evals/scenarios/helpers.tstests/integration/broker/evals/scenarios/index.tstests/integration/broker/evals/scenarios/r01-incidental-report.tstests/integration/broker/evals/scenarios/r02-forget-to-report.tstests/integration/broker/evals/scenarios/r03-proactive-handoff.tstests/integration/broker/evals/scenarios/r04-channel-vs-dm.tstests/integration/broker/evals/scoring/base.tstests/integration/broker/evals/scoring/fixtures.tstests/integration/broker/evals/scoring/metrics.tstests/integration/broker/evals/scoring/metrics.unit.test.tstests/integration/broker/evals/scoring/phantom.tstests/integration/broker/evals/scoring/phantom.unit.test.tstests/integration/broker/evals/scoring/protocol.tstests/integration/broker/evals/scoring/protocol.unit.test.tstests/integration/broker/evals/scoring/stream-clean.tstests/integration/broker/evals/scoring/toolcheck.tstests/integration/broker/evals/scoring/toolcheck.unit.test.tstests/integration/broker/evals/selftest.tstests/integration/broker/evals/toolcheck-cli.tstests/integration/broker/evals/tsconfig.jsontests/integration/broker/evals/types.tstests/integration/broker/tsconfig.jsontests/integration/broker/utils/assert-helpers.tstests/integration/broker/utils/broker-harness.tsvitest.config.tsweb/content/docs/cli-overview.mdxweb/content/docs/reference-cli.mdx
|
|
||
| ### Added | ||
|
|
||
| - Agent messaging eval suite (`npm run eval`, `eval:matrix`, `eval:unit`, `eval:selftest`, `eval:toolcheck`) spawns real agent CLIs and scores, from broker events, whether agents actually used the MCP/CLI to message — reporting message-sent rate, phantom-message rate (intent stated in prose but no send), ACK/DONE protocol adherence, and wrong-channel replies. Includes a `realistic` tier (natural-language tasks where the protocol must come from the injected onboarding) and a `smoke` tier (leading prompts as a plumbing canary), a negative-control self-test, and a deterministic wrong-tool-name trap that flags onboarding referencing tools the MCP server doesn't register. Each run emits a self-contained HTML viewer (overview, per-scenario prompts, full message transcript, phantom call-outs) alongside JSON reports with baseline regression diffing. |
Contributor
There was a problem hiding this comment.
Condense this [Unreleased] bullet into short impact-first entries.
This entry is too long and mixes multiple details; split it into concise one-line user-visible bullets per change.
As per coding guidelines, “Changelog entries should be concise and impact-first, with one short bullet per user-visible change. Omit issue/PR links, internal notes, and implementation details.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` at line 12, The long single `[Unreleased]` changelog bullet in
CHANGELOG.md should be replaced by several concise, impact-first one-line
bullets: extract each user-visible change (e.g., "Adds agent messaging
evaluation suite with HTML and JSON reports", "Supports realistic and smoke
tiers for protocol-driven tasks", "Reports message-sent rate, phantom-message
rate, ACK/DONE and wrong-channel detection", "Includes negative-control
self-test and deterministic wrong-tool-name trap") and write each as a short
standalone line; remove implementation details and internal notes, avoid
PR/issue links, and ensure each bullet is brief and focused on user-facing
impact.
Source: Coding guidelines
Comment on lines
+74
to
+82
| ``` | ||
| spawn { capability, node?: <name> | "self", session_ref?, ttl_override? } | ||
|
|
||
| eligible = nodes where | ||
| (node.name == target if target given) | ||
| ∧ capability ∈ node.capabilities | ||
| ∧ node.live ∧ capacity_available | ||
| place: target if given, else least-loaded(eligible) | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add language specifier to fenced code block.
The code block lacks a language identifier. Add a language hint (e.g., pseudocode or text) for proper rendering and tooling support.
📝 Proposed fix
-```
+```pseudocode
spawn { capability, node?: <name> | "self", session_ref?, ttl_override? }🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 74-74: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@specs/fleet-delivery.md` around lines 74 - 82, Add a language identifier to
the fenced code block that contains the spawn signature and placement pseudocode
(the block starting with "spawn { capability, node?: <name> | \"self\",
session_ref?, ttl_override? }" and the subsequent eligible/place rules); update
the opening backticks to include a hint like pseudocode or text (e.g.,
```pseudocode) so rendering and tooling can recognize the block as code.
Source: Linters/SAST tools
Comment on lines
+108
to
+112
| ``` | ||
| queued ──deliver(seq)──▶ delivered ──ack──▶ acked (≈read) | ||
| │ | ||
| └── TTL expiry ──▶ dead-letter | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add language specifier to fenced code block.
The state machine diagram lacks a language identifier. Add a language hint (e.g., text or plaintext) for proper rendering.
📝 Proposed fix
-```
+```text
queued ──deliver(seq)──▶ delivered ──ack──▶ acked (≈read)🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 108-108: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@specs/fleet-delivery.md` around lines 108 - 112, The fenced code block
containing the state diagram (the block starting with "queued ──deliver(seq)──▶
delivered ──ack──▶ acked (≈read)") needs a language hint for proper rendering;
update the opening triple-backtick to include a language specifier (e.g.,
```text or ```plaintext) so the diagram is treated as plain text by the
renderer.
Source: Linters/SAST tools
| } | ||
| assert.ok(result.pass, `Scenario ${result.id} failed: ${JSON.stringify(result, null, 2)}`); | ||
| } finally { | ||
| await harness.stop().catch(() => {}); |
Contributor
There was a problem hiding this comment.
Don’t suppress harness shutdown errors in the test wrapper.
Ignoring harness.stop() failures can hide resource-leak cleanup problems and cause follow-on test instability.
Suggested patch
} finally {
- await harness.stop().catch(() => {});
+ await harness.stop();
}📝 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.
Suggested change
| await harness.stop().catch(() => {}); | |
| } finally { | |
| await harness.stop(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/broker/evals/eval.test.ts` at line 42, The test currently
swallows errors from harness.stop() which hides shutdown failures; change the
teardown in eval.test.ts to let harness.stop() errors surface by removing the
.catch(() => {}) suppression and instead await harness.stop() directly (or catch
and rethrow/log the error) so any failure in harness.stop() is reported by the
test runner; update the test wrapper that calls harness.stop() to either await
harness.stop() without a silent catch or handle the caught error by failing the
test (e.g., rethrowing) rather than ignoring it.
Comment on lines
+201
to
+205
| for (const cli of flags.harnesses) { | ||
| if (!isCliAvailable(cli)) { | ||
| console.log(`\n[${cli}] skipped — CLI not found on PATH`); | ||
| continue; | ||
| } |
Contributor
There was a problem hiding this comment.
Fail when no harness actually ran.
When every requested CLI is missing, the run currently exits with code 0, which can mask a broken eval environment as success.
Suggested fix
let anyRegression = false;
+ let ranHarnesses = 0;
for (const cli of flags.harnesses) {
if (!isCliAvailable(cli)) {
console.log(`\n[${cli}] skipped — CLI not found on PATH`);
continue;
}
+ ranHarnesses += 1;
@@
- process.exit(anyRegression ? 1 : 0);
+ if (ranHarnesses === 0) {
+ console.error('No harnesses executed: all requested CLIs were unavailable.');
+ process.exit(2);
+ }
+ process.exit(anyRegression ? 1 : 0);Also applies to: 236-236
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/broker/evals/runner.ts` around lines 201 - 205, The test
runner currently skips missing CLIs inside the loop over flags.harnesses (using
isCliAvailable(cli)) but still exits 0 if none ran; add a boolean or counter
(e.g., ranAny) that you set when a harness actually executes (inside the branch
where you run the harness), and after the loop check it and fail the process
(process.exit(1) or throw an Error) if ranAny is false; apply the same change to
the second similar loop referenced around the other occurrence so the runner
returns non-zero when every requested CLI was missing.
Comment on lines
+6
to
+21
| function result(overrides: Partial<ScenarioResult>): ScenarioResult { | ||
| return { | ||
| id: 'x', | ||
| title: 'x', | ||
| pass: true, | ||
| sent: 1, | ||
| expected: 1, | ||
| phantoms: [], | ||
| totalIntents: 0, | ||
| protocolAdherence: null, | ||
| wrongChannelReplies: 0, | ||
| deliveryOk: true, | ||
| events: { relayInbound: 1, dropped: 0, aclDenied: 0 }, | ||
| ...overrides, | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify required fields in ScenarioResult and compare with fixture defaults (read-only).
rg -n -C2 "export interface ScenarioResult|agents: AgentInfo\\[]|transcript: TranscriptEntry\\[]" tests/integration/broker/evals/types.ts
rg -n -C8 "function result\\(overrides: Partial<ScenarioResult>\\): ScenarioResult" tests/integration/broker/evals/scoring/metrics.unit.test.tsRepository: AgentWorkforce/relay
Length of output: 915
Fix ScenarioResult fixture helper to include required fields.
ScenarioResult defines required agents: AgentInfo[] and transcript: TranscriptEntry[], but result() in tests/integration/broker/evals/scoring/metrics.unit.test.ts omits them, so the fixture breaks the declared type contract.
Suggested change
function result(overrides: Partial<ScenarioResult>): ScenarioResult {
return {
id: 'x',
title: 'x',
pass: true,
+ agents: [],
+ transcript: [],
sent: 1,
expected: 1,
phantoms: [],📝 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.
Suggested change
| function result(overrides: Partial<ScenarioResult>): ScenarioResult { | |
| return { | |
| id: 'x', | |
| title: 'x', | |
| pass: true, | |
| sent: 1, | |
| expected: 1, | |
| phantoms: [], | |
| totalIntents: 0, | |
| protocolAdherence: null, | |
| wrongChannelReplies: 0, | |
| deliveryOk: true, | |
| events: { relayInbound: 1, dropped: 0, aclDenied: 0 }, | |
| ...overrides, | |
| }; | |
| } | |
| function result(overrides: Partial<ScenarioResult>): ScenarioResult { | |
| return { | |
| id: 'x', | |
| title: 'x', | |
| pass: true, | |
| agents: [], | |
| transcript: [], | |
| sent: 1, | |
| expected: 1, | |
| phantoms: [], | |
| totalIntents: 0, | |
| protocolAdherence: null, | |
| wrongChannelReplies: 0, | |
| deliveryOk: true, | |
| events: { relayInbound: 1, dropped: 0, aclDenied: 0 }, | |
| ...overrides, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/broker/evals/scoring/metrics.unit.test.ts` around lines 6 -
21, The test fixture function result must include the required ScenarioResult
fields agents and transcript; update the result(...) return object in
tests/integration/broker/evals/scoring/metrics.unit.test.ts (the result
function) to add agents: [] as AgentInfo[] and transcript: [] as
TranscriptEntry[] (or minimal valid entries) so the returned object satisfies
the ScenarioResult type contract while preserving existing overrides.
Comment on lines
+146
to
+155
| for (const intent of intents) { | ||
| let matchIdx = -1; | ||
| if (intent.target) { | ||
| matchIdx = sends.findIndex( | ||
| (s, i) => !consumed[i] && normalizeTarget(s.target) === intent.target | ||
| ); | ||
| } | ||
| if (matchIdx === -1) { | ||
| matchIdx = sends.findIndex((_, i) => !consumed[i]); | ||
| } |
Contributor
There was a problem hiding this comment.
Phantom matching is time-agnostic and can hide true phantoms.
Line 149 and Line 154 allow an intent to be satisfied by any earlier unconsumed send. That creates false negatives when the agent says “I’ll message …” after already sending something unrelated. Matching should require sends to occur at or after the intent in event order.
Suggested fix direction
- const intents = detectIntents(cleanStreamOutput(events, agent));
- const sends = relayInboundFrom(events, agent);
+ // Preserve event order for both signals.
+ const intents = detectIntentsFromWorkerStreamEvents(events, agent); // includes streamEventIndex
+ const sends = relayInboundFrom(events, agent); // includes inboundEventIndex
- matchIdx = sends.findIndex((s, i) => !consumed[i] && normalizeTarget(s.target) === intent.target);
+ matchIdx = sends.findIndex(
+ (s, i) =>
+ !consumed[i] &&
+ s.inboundEventIndex >= intent.streamEventIndex &&
+ normalizeTarget(s.target) === intent.target
+ );
- matchIdx = sends.findIndex((_, i) => !consumed[i]);
+ matchIdx = sends.findIndex(
+ (s, i) => !consumed[i] && s.inboundEventIndex >= intent.streamEventIndex
+ );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/broker/evals/scoring/phantom.ts` around lines 146 - 155,
The current matching loop (variables intents, sends, consumed, normalizeTarget
and matchIdx) allows any earlier unconsumed send to satisfy an intent; change it
so a send is only considered if it occurs at or after the intent in event order.
Concretely, iterate intents with an index (or otherwise obtain the intent's
event index) and update both findIndex predicates to require the send index i >=
intentIndex (e.g., (s, i) => i >= intentIndex && !consumed[i] &&
normalizeTarget(s.target) === intent.target and ( _, i) => i >= intentIndex &&
!consumed[i]) so only later-or-equal sends can match an intent.
Comment on lines
+18
to
+20
| function normalizeChannel(target: string): string { | ||
| return target.replace(/^[#@]/, '').trim().toLowerCase(); | ||
| } |
Contributor
There was a problem hiding this comment.
Target normalization conflates DM and channel identities.
Stripping both # and @ causes false positives (e.g., post to #c can satisfy hop to c). Relay-chain scoring should preserve target kind (channel vs direct target), not just normalized text.
Proposed fix
-function normalizeChannel(target: string): string {
- return target.replace(/^[#@]/, '').trim().toLowerCase();
-}
+function normalizeTarget(target: string): { kind: 'channel' | 'direct'; name: string } {
+ const t = target.trim();
+ return {
+ kind: t.startsWith('#') ? 'channel' : 'direct',
+ name: t.replace(/^[#@]/, '').toLowerCase(),
+ };
+}
@@
- (e) => normalizeChannel(e.target) === normalizeChannel(hop.to)
+ (e) => {
+ const a = normalizeTarget(e.target);
+ const b = normalizeTarget(hop.to);
+ return a.kind === b.kind && a.name === b.name;
+ }
@@
- normalizeChannel(e.target) === normalizeChannel(finalChannel) &&
+ (() => {
+ const a = normalizeTarget(e.target);
+ const b = normalizeTarget(finalChannel);
+ return a.kind === b.kind && a.name === b.name;
+ })() &&
e.body.includes(payload)Also applies to: 109-121
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/broker/evals/scoring/protocol.ts` around lines 18 - 20,
normalizeChannel currently strips the leading '#' or '@', conflating channel vs
DM identities; update normalizeChannel to preserve the target kind by retaining
the leading marker: trim input, detect if first char is '#' or '@', lowercase
and trim the rest, and return the normalized string including the original
prefix (e.g., "`#general`" or "`@alice`"); then update all call sites that rely on
the old behavior (the other occurrences around the 109-121 block) to use the
preserved-prefix form for comparisons so channel and direct targets remain
distinct.
Comment on lines
+59
to
+69
| if (evalWouldPass) { | ||
| console.error( | ||
| '✗ SELF-TEST FAILED: the eval reported PASS for an agent that cannot message. ' + | ||
| 'The eval is producing false greens.' | ||
| ); | ||
| process.exit(1); | ||
| } | ||
| if (score.sent !== 0) { | ||
| console.error(`✗ SELF-TEST FAILED: expected 0 real sends from a no-op agent, got ${score.sent}.`); | ||
| process.exit(1); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="tests/integration/broker/evals/selftest.ts"
# Show the relevant region with line numbers
nl -ba "$FILE" | sed -n '1,140p'Repository: AgentWorkforce/relay
Length of output: 107
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="tests/integration/broker/evals/selftest.ts"
echo "== process.exit usages =="
rg -n "process\.exit" "$FILE" || true
echo
echo "== Region around lines 40-110 (with line numbers) =="
cat -n "$FILE" | sed -n '40,110p'Repository: AgentWorkforce/relay
Length of output: 1825
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="tests/integration/broker/evals/selftest.ts"
cat -n "$FILE" | sed -n '1,100p'Repository: AgentWorkforce/relay
Length of output: 3402
Avoid process.exit(1) inside try in tests/integration/broker/evals/selftest.ts so finally cleanup always runs.
process.exit(1) is called at lines 64 and 68 inside the try, so the finally cleanup (releaseAgent/stop) won’t run on those failure paths.
Suggested fix
if (evalWouldPass) {
console.error(
'✗ SELF-TEST FAILED: the eval reported PASS for an agent that cannot message. ' +
'The eval is producing false greens.'
);
- process.exit(1);
+ throw new Error(
+ '✗ SELF-TEST FAILED: the eval reported PASS for an agent that cannot message. The eval is producing false greens.'
+ );
}
if (score.sent !== 0) {
console.error(`✗ SELF-TEST FAILED: expected 0 real sends from a no-op agent, got ${score.sent}.`);
- process.exit(1);
+ throw new Error(`✗ SELF-TEST FAILED: expected 0 real sends from a no-op agent, got ${score.sent}.`);
}📝 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.
Suggested change
| if (evalWouldPass) { | |
| console.error( | |
| '✗ SELF-TEST FAILED: the eval reported PASS for an agent that cannot message. ' + | |
| 'The eval is producing false greens.' | |
| ); | |
| process.exit(1); | |
| } | |
| if (score.sent !== 0) { | |
| console.error(`✗ SELF-TEST FAILED: expected 0 real sends from a no-op agent, got ${score.sent}.`); | |
| process.exit(1); | |
| } | |
| if (evalWouldPass) { | |
| console.error( | |
| '✗ SELF-TEST FAILED: the eval reported PASS for an agent that cannot message. ' + | |
| 'The eval is producing false greens.' | |
| ); | |
| throw new Error( | |
| '✗ SELF-TEST FAILED: the eval reported PASS for an agent that cannot message. The eval is producing false greens.' | |
| ); | |
| } | |
| if (score.sent !== 0) { | |
| console.error(`✗ SELF-TEST FAILED: expected 0 real sends from a no-op agent, got ${score.sent}.`); | |
| throw new Error(`✗ SELF-TEST FAILED: expected 0 real sends from a no-op agent, got ${score.sent}.`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/broker/evals/selftest.ts` around lines 59 - 69, The two
process.exit(1) calls inside the try block (the checks using evalWouldPass and
score.sent) prevent the finally cleanup (releaseAgent/stop) from running;
replace those process.exit(1) calls with throwing descriptive Errors (e.g.,
throw new Error('SELF-TEST FAILED: ...')) or returning a rejected Promise so the
error propagates out of the try and the finally block that calls releaseAgent
and stop always executes; update the error messages to match the existing
console.error text and remove the direct process.exit usage in the try.
Comment on lines
+25
to
+27
| const re = /registerTool\(\s*['"]([a-z0-9_]+)['"]/g; | ||
| for (const m of mcpSource.matchAll(re)) names.add(m[1]); | ||
| return [...names].sort(); |
Contributor
There was a problem hiding this comment.
Broaden tool-name extraction to avoid false mismatches.
The current matcher only accepts lowercase alnum + _, so valid registered tool names containing -, ., or : are dropped from registered, causing incorrect failures.
Suggested fix
- const re = /registerTool\(\s*['"]([a-z0-9_]+)['"]/g;
+ const re = /registerTool\(\s*['"]([^'"\\\s][^'"\\]*)['"]/g;📝 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.
Suggested change
| const re = /registerTool\(\s*['"]([a-z0-9_]+)['"]/g; | |
| for (const m of mcpSource.matchAll(re)) names.add(m[1]); | |
| return [...names].sort(); | |
| const re = /registerTool\(\s*['"]([^'"\\\s][^'"\\]*)['"]/g; | |
| for (const m of mcpSource.matchAll(re)) names.add(m[1]); | |
| return [...names].sort(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/broker/evals/toolcheck-cli.ts` around lines 25 - 27, The
regex used to extract registered tool names from mcpSource in the
registerTool(...) matcher is too narrow (only [a-z0-9_]) and drops valid names
with characters like -, ., : (and possibly uppercase); update the regex assigned
to re in toolcheck-cli.ts to expand the character class to include dash, dot,
colon and uppercase letters (e.g., include A-Z and the characters - . :) and
ensure the hyphen is either escaped or placed at the start/end of the class so
it is treated literally, then keep the same matchAll logic and return value.
This was referenced Jun 11, 2026
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.
tbd