Repository navigation
Conversation
- Add tools/run-output-evals.sh, a local runner that plays scripted multi-turn cases through the Claude Code CLI with only the plugin under test loaded, then grades the transcript and the generated files - Add plugins/solace-messaging-skills/evals/output-evals.json with 11 cases across the three skills, 5 marked must_pass - Grade with deterministic checks (assistant text, tool use, generated files, mvn compile, live sol-jcsmp release match) and a fixed-model LLM judge for routing and grounding criteria - Add an opt-in live_verify grader that runs the generated project's verify.sh against a broker named by OUTPUT_EVAL_BROKER_* variables and skips loudly when they are unset - Document the corpus format, run instructions, gate, and local-only status in the evals README - Model the runner on tools/run-trigger-evals.sh (isolation via --plugin-dir and a scratch CLAUDE_CONFIG_DIR, 90% gate, must_pass)
- Harden the Debug row in solace-application-development/references/jcsmp.md so the documentation redirect is the entire debug answer; memory-derived hypotheses, interim checks, and fixes are forbidden - State in solace-topic-best-practices/SKILL.md that the skill answers in chat only and never writes files - Both gaps surfaced as failing output-eval cases (appdev-debug-redirect-negative and topics-no-files-negative); both cases pass after these edits
aelsammak
changed the title
- Remove exact case counts from the evals README so the prose does not drift from the corpus - Remove the "No CI wiring" section from the evals README - Take broker credentials for live_verify from exported OUTPUT_EVAL_BROKER_* variables only; drop the optional env file - Report in the run summary whether live_verify executed, skipped, or never ran because the case failed before grading - Drop the run.sh and port details from two application-development case prompts; no grader depended on them
- Replace the Quickstart failover word check with a code grep for the reconnect block; the Step 6 checklist report legitimately names HA failover - Run the LLM judge tool-free (--tools "") from the run directory so a tool call cannot spend its only turn and project settings stay out of the judge session - Add a compile grader to the Solace Suggested case and make compile and maven_release_match iterate every generated pom for two-project layouts - Align the debug-redirect judge with the Debug row: fail troubleshooting from any source, treat a restated symptom as the acknowledgement - Accept bold field labels in the design summary greps (design mode leaves formatting to the model) - Validate OUTPUT_EVAL_RUNS as a positive integer (a zero value crashed bash 3.2 on an empty array expansion) - Classify a timeout kill (exit 124) as INFRA in compile and live_verify - Carry the cause onto turn-level INFRA lines and print a RETRY line before the second attempt - Show the live-verify skip tag on FAIL lines; document that a max_turns hit fails the case - Drop the run-duration claim from the evals README; mention output evals in the root README Verified locally: claude-sonnet-5 and claude-opus-5 legs over the six affected cases pass, with live verify green against a real broker on both models.
aelsammak
marked this pull request as ready for review
September 9, 2026 13:38
| skill="$(jq -r '.skill' <<<"$case_json")" | ||
| must="$(jq -r '.must_pass // false' <<<"$case_json")" | ||
| max_turns="$(jq -r '.max_turns // 25' <<<"$case_json")" | ||
| NTURNS="$(jq -r '.turns | length' <<<"$case_json")" |
There was a problem hiding this comment.
jq is not standard on windows bash shell even for the claude git bash requirement. And there is a lot of jq. Makes me wonder if the run evals should be in python or for better performance go.
Author
There was a problem hiding this comment.
I will be opening a separate ticket to migrate this to python or go
mtite-sol
reviewed
Sep 9, 2026
mtite-sol
approved these changes
Sep 9, 2026
mtite-sol
left a comment
mtite-sol
left a comment
There was a problem hiding this comment.
Added some non blocking suggestions but overall code LGTM!
| "max_turns": 80, | ||
| "graders": [ | ||
| { "type": "assistant_grep", "pattern": "[Ff]ailover", "expect": "present", "turn": 3 }, | ||
| { "type": "java_disclaimer" }, |
There was a problem hiding this comment.
Non blocking suggestion: java_disclaimer and maven_release_match are rules that apply to any generated code, but right now each case that generates code has to remember to include them, and if someone forgets nothing fails. Could the runner just run them on every case instead? They'd pass when there's no .java or pom.xml to check. Then they can be removed from the two cases that have them and new cases pick them up automatically. compile should stay explicit since it's expensive.
full disclosure, I've never used evals before so just suggesting this based on experience with other test frameworks 😆
Author
There was a problem hiding this comment.
I do see your point but, in the future when we expand the scope of this skill to handle more of our APIs in other languages we wouldn't want to enforce language specific rules to them. So having them optional for now will make more sense in the future!
There was a problem hiding this comment.
Thinking about it more - maven_release_match global rule doesn't make sense but I think my point about the disclaimer is still valid. I meant for it to be api agnostic - the check could look at all generated source files (*.java today, *.py / *.go etc. as other API skills land) and pass when either the source files contain the disclaimer, or when no source files are generated. It would fail when files are generated and any do not contain the disclaimer. That way nobody has to remember to add it to each test per API, and a new code-generating case can't silently skip the disclaimer check.
That being said, I don't think this needs to happen as part of this PR, just something to think about.
Author
There was a problem hiding this comment.
Yes that is a good point! Something to definitely remember for when we go to add more languages 😄
- Add tools/output-evals.schema.json (JSON Schema draft 2020-12) that defines the output-eval corpus with per-grader field rules and a description for every field, in response to review feedback - Rename the corpus fields turns to user_turns and max_turns to max_agent_turns to separate the scripted user messages from the CLI's agent-turn budget; update the runner and the corpus accordingly - Point the evals README and the runner header at the schema; the runner keeps enforcing the structural subset at start-up - Add a prerequisites list and a transcript-to-corpus mapping table (observed with Claude Code 2.1.266) to the evals README
- Resolve GNU timeout with a gtimeout fallback for the mvn compile and verify.sh bounds, and warn once when neither is on PATH and a selected case needs Maven, instead of running unbounded silently - List timeout as an optional prerequisite in the evals README with the unbounded fallback and the macOS install hint - State in both README credential notes that an existing claude login does not satisfy the runner, because the throwaway config directory carries no login
Local run of the output evals on WindowsRan both legs from this branch unmodified, on Windows 11 (Cygwin bash 5.3, JDK 11.0.31 Temurin, Maven 3.6.3,
ᴹ = The suite works well and the docs are accurate — I got it running with no changes to the runner. Both 1. The runner ignores
|
| leg | denials |
|---|---|
claude-sonnet-5 |
5 × Bash |
claude-opus-5 |
2 × Bash, 1 × PowerShell |
On appdev-quickstart-implement-full, sonnet issued a compound command:
cp '...\references\jcsmp\scripts\verify.sh' './verify.sh' && chmod +x ./verify.sh && ls -la
which came back denied:
This Bash command contains multiple operations. The following parts require approval:
cp '...',chmod +x ./verify.sh,ls -laClaude requested permissions to edit
...\references\jcsmp\scripts\verify.shwhich is a sensitive file.
It then fell back to Read → Write, re-emitting all 55 KB of verify.sh through the model. Opus used a single-command cp with forward slashes, which was allowed, and passed.
So --allowedTools Bash does not grant unrestricted Bash: the multi-operation splitter still gates compound commands, and a sensitive-file guard fires on the plugin's own reference path. The measured behavior was distorted exactly as the header says it should not be.
Suggestion: every result event already carries permission_denials. Treating a non-empty array as INFRA would have surfaced this as an infrastructure problem rather than scoring it as a skill failure. Worth considering regardless of platform, since a denial can flip any case.
2. No .gitattributes — the identical_to grader can't pass on a Windows checkout
With core.autocrlf=true (the Git for Windows installer default) the whole tree checks out CRLF. The reference verify.sh is then 56,406 bytes with 1,017 CRs, while the model writes LF (55,389 bytes). Strip the CRs and the MD5s are identical (63f3c5bf…), so the content was right — only the byte comparison failed. This is the only identical_to grader in the corpus, so it's a single point of failure for a must_pass case.
Suggestion: add .gitattributes with *.sh text eol=lf (or * text=auto eol=lf repo-wide) and renormalize.
Worth noting the Windows behavior is arguably more correct here. implement-mode.md:18 and :117 require "a copy of the bundled scripts/verify.sh". On a Unix checkout, sonnet's retyped LF output would have matched byte-for-byte and passed — silently hiding that it never copied the file, which is precisely the drift this grader exists to catch. On Windows the grader accidentally distinguishes a real copy from a retype. If catching that is intended, it may deserve a grader that doesn't depend on line-ending luck.
3. feedback-support-email-positive — a real sonnet-specific contract gap
Not environmental. Sonnet's draft omitted Steps to reproduce entirely, though the feedback is bug-shaped and SKILL.md:151 requires the section in that case. It also emitted Notes / None, where SKILL.md:152 says Notes is omitted when empty.
Opus, on the identical session, produced usable steps and correctly dropped Notes:
Steps to reproduce
1. Ask the solace-application-development skill to generate a JCSMP subscriber
consuming from a queue with guaranteed messaging.
2. Inspect the generated onReceive handler.
3. Observe ackMessage() is invoked before the processing call rather than after
it returns.
So the case is well formed — the steps are derivable from the reported bug, and there's no need to seed the prompt with a real invocation history. This looks like a genuine model-dependent gap in the skill. The Notes padding isn't currently graded and might be worth an assistant_grep absent-check.
Doc nits
timeoutis an undocumented dependency. It bounds themvnandverify.shcalls; without it both run unbounded and the prerequisite list doesn't mention it.- An existing
claudelogin does not satisfy the credential requirement, because the scratchCLAUDE_CONFIG_DIRdiscards it by design. That's the first thing anyone hits — one sentence in the README would save the confusion. mvnon PATH is documented, but note the compile cases fail closed before any case runs, which is good; just worth knowing a Maven wrapper dist works fine as a substitute.
Local caveats
Two things specific to my machine that may not generalize: an rtk shim rewrites commands (ls → rtk ls), which added a part to sonnet's compound command and may have contributed to that split; and the sensitive-file guard on the plugin reference path may come from my local settings. Neither changes finding #1 — the runner currently can't distinguish "the skill behaved badly" from "a denial forced a fallback."
…ripts - Read permission_denials from every turn's result event: a denial of a tool the runner grants is INFRA at once, with the tool and command on the case line and no retry, because the same policy would deny the retry; a denial of any other tool is tagged on the case line - Correct the header claim that the allowlist prevents denials; managed settings, hooks, and command shims can still deny a granted tool, and a denied cp became a lossy retype in a Windows run - Add .gitattributes with *.sh text eol=lf so Windows checkouts keep shell scripts LF for bash and for the byte-identity grader on verify.sh - Document Git Bash and LF on Windows, and denials as INFRA, in the evals README
…edback skill - Make Steps to reproduce mandatory for bug-shaped feedback and derive it from the report when the session holds no skill invocation; a Windows sonnet run dropped the section under the old wording - Drop the Notes heading when empty and forbid None or N/A filler under it - State that Read is the skill's only tool and that the plugin manifest is read at its announced path, never located with a shell command; three-run checks showed sonnet running find for the manifest - Add a grader to feedback-support-email-positive that forbids a line consisting only of None - Three sonnet runs of the case pass by majority vote after these edits
- Prescribe the copy command in implement-mode Step 4: cat the bundled script into the project root and chmod it, and forbid cp on the bundled file - Claude Code protects the plugin directory and blocks a cp that names a file inside it, even with Bash allowed; the blocked copy fell back to a retype of the script that must stay byte-identical, and the runner now reports that denial as INFRA - Reproduced on macOS with the runner's flags: cp denied, cat redirect allowed and byte-identical - Two sonnet runs of appdev-quickstart-implement-full with this wording used the cat form with zero denials; the second passed every grader
- Add exclude_pattern to the four bare Bash-absent graders so a bare echo, true, or : (plus ls and pwd on the topics negative) no longer fails a case; a redirect, a chained command, a search, or a transport call still does - Sonnet ended feedback turns with echo done in about a third of runs and opus listed the directory once before it refused to write the file, while the graded content was correct every time - Tell the grounding judge that deriving the consequences of the fetched rules for the scenario is applying the guidance, and tell every judge that WebFetch results are the tool's summaries, not the page; a summary remark that guidance was absent had failed a correct opus answer - Correct the README Windows note: git checkout -- . leaves a stale CRLF checkout in place because git treats the files as unchanged; document the delete-and-checkout recipe
- Add one sentence to step 3: when the user's scenario needs a decision the fetched page does not cover, say so before suggesting one - The grounding judge already accepts flagged extrapolation, and sonnet folded command topics into the page's template unflagged in 2 of 4 runs - Fifteen single sonnet runs of topics-grounded-answer pass after the edit; on the final tree the full sonnet leg is 11/11 and the opus leg 10/11, live verify skipped
cjwmorgan-sol
approved these changes
Sep 10, 2026
cjwmorgan-sol
left a comment
cjwmorgan-sol
left a comment
There was a problem hiding this comment.
Tried running the evals locally again and passed with the 90% gate.
Had some offline discussions with @aelsammak as I'm not sure the pass fail gate measure is accurate/reflective enough, but that would be out of scope for this change.
Really appreciate the output-evals.schema.json to document the structure of output-evals.json. I hope this can be used in verification of output-eval.json additions/substractions later as well!
Change look ready for merge to me!
aelsammak
added a commit
that referenced
this pull request
Sep 11, 2026
* EBP-2849: Enforce plugin version bumps on release PRs and marketplace sync (#2) * EBP-2849: Enforce plugin version bumps on release PRs and marketplace sync - Add tools/check-plugin-versions.sh requiring a strict semver bump in plugin.json when plugin content (excluding the top-level evals/ corpus) changes relative to the PR base - Gate the version check to PRs targeting main, matching the EBP-2829 decision that bumps happen only in release PRs - Validate marketplace.json stays in sync with plugins/ on every run: entries resolve to existing plugin dirs, names match, no duplicates, every plugin dir listed - Wire both checks into the validate CI job as steps; checkout uses fetch-depth 0 so the merge base is reachable * EBP-2849: Address Claude PR review fixes * EBP-2849: Bump setup-node from Node 20 to Node 24 - Node 20 reached end of life in April 2026 and GitHub Actions runners remove it in fall 2026 - Node 24 is the active LTS and satisfies the Claude Code CLI requirement of Node 18 or newer - Applies to both setup-node usages (validate and trigger-evals jobs) * DATAGO-147743: Pin CI actions to commit SHAs and pin the Claude Code CLI install (#3) - Pin actions/checkout, setup-node, setup-java, and upload-artifact to the full commit SHAs their v4 tags currently resolve to, with the tag in a trailing comment - Pin the Claude Code CLI install to 2.1.231 via a workflow-level CLAUDE_CODE_VERSION env value shared by both install steps - Mutable tags and unpinned npm installs let a compromised upstream publish execute in CI; the trigger-evals job holds the ANTHROPIC_API_KEY secret * EBP-2889: Address session-analysis feedback on solace-application-development (#4) * EBP-2889: Address session-analysis feedback on solace-application-development - Promote buried mandates to SKILL.md invariants: AI-assisted disclaimer header, verification artifacts (checklist + verify.sh + hooks), and the live-broker heads-up; extend the doc-grounding invariant to ban substitute channels and the version invariant to ban the solrsearch index - Surface the Step 0 door-question gate and the environment-discovery rule (a found broker/config never answers a question) in jcsmp.md, design-mode.md, and implement-mode Step 0 - Split implement-mode.md into a core file plus three per-leaf wiring files so the core fits a single Read; state the generation file-output contract up front and move the checklist write into Step 4 generation output - Redesign verification to be shape-agnostic: verify.sh gains an app stage driven by a generated verify-hooks.sh (START_CMD/TRIGGER_CMD/READY_MARKER/PASS_MARKER), a warn-only conformance preflight, and a live-run heads-up line; curl may trigger but never render the verdict - Replace the log4j2 2.26.0 pin with outcome rules: resolve the highest stable 2.x from authoritative metadata, framework-owned SLF4J backends win, config file required, com.solacesystems loggers never silenced, and the 2.17.1 Log4Shell floor enforced as a check - Bind doc grounding to each design question and redefine the Grounding docs field to fetched-pages-only; add a Generation conformance group to the verification checklist - Remove the mandated Core Concepts intro from SKILL.md and prerequisites.md; prerequisites is now broker acquisition only - Solace Suggested decoupling: keep the two-project default but require a surfaced conflict with three resolutions on single-deliverable asks; never merge silently - Rename the Java API Release Notes link to JCSMP API Release Notes (the JAVA_API_RN download serves the JCSMP release notes) and point sample logging comments at the Step 3 rule - Root-caused from 10 analyzed sessions; violations and fixes tracked in EBP-2889 * EBP-2889: Route build requests through Design mode before Implement - Key mode routing on the design contract instead of request phrasing; a build prompt without a contract enters Design first, including web and embedded shapes - Add the design-contract gate to jcsmp.md and make the summary a hard precondition in Implement Step 2 (valid sources: same-session summary, explicit summary in chat, saved solace-design.md) - Add the topology rule for edits to existing apps: topology-changing edits re-enter Design, mechanical edits go straight to Implement - Add Design mode's fully-specified path: a prompt that answers every tree question gets one merged confirm at close-out - Dedupe the broker-access question in Implement Step 1 when Design Step 0 already confirmed it this session - Keep the door question an isolated Implement Step 0; a confirmed design never answers it * EBP-2889: Carry sample best-practice comments into generated code - Add the comments-follow-their-code contract to implement-mode Step 4: a construct carried from a sample keeps that construct's comment (names adapted), dropped demo harness takes its comments with it, and fresh messaging code that applies a documented practice gets a short comment naming it - Add a binary item to the checklist's Generation conformance group so a run that strips the comments fails its own checklist; keep it outside the preflight's first-four mechanical checks - Align the Step 6 group description with the new non-preflight item - Evidence: a generated embedded Direct Pub/Sub project carried sample constructs without their attached best-practice comments, while every explicitly mandated one-line comment survived (slot-bound rules get followed; unbound ones do not) * EBP-2889: Remove the Non-Secure sub-mode from Solace Suggested - Solace Suggested is TLS-secure by definition: tcps:// with server-certificate validation, requiring a TLS-configured broker; there is no non-secure variant and no Step 0 sub-question, so the door question stays a clean three-way choice - On a broker without TLS, the skill states the requirement and offers Custom with the equivalent knobs minus the secure session (the developer's explicit choice) or Quickstart; it never generates a plaintext session under the Solace Suggested name - Collapse the overlay's Step 1 fork to the TLS build only and delete the plaintext session section; the no-TLS case routes back to the Step 0 door - Update the checklist branch notes, the Step 6 tailoring examples, the Custom span description, and the comment-contract bullet accordingly - Feedback source: live session screenshot showed the door question flattened into four options with Non-Secure preselected * EBP-2889: State Quickstart's plaintext posture explicitly - Make explicit that Quickstart has no TLS requirement and connects with whatever scheme the host carries; a plaintext tcp:// dev broker is a normal Quickstart target - Guard against over-generalizing the new Solace Suggested TLS requirement onto the Quickstart door * EBP-2889: Harden verify.sh teardown against orphaned app processes - Make shutdown_long_running and the cleanup trap signal the process group (kill -- -PID) instead of the bare PID; every launch site backgrounds under set -m, so each recorded PID leads its own group - Close the macOS bash 3.2 leak where a compound START_CMD keeps the bash -c wrapper alive and a PID-directed kill stopped only the wrapper, orphaning the app (bash 4.4+ execs the final command) - Add a post-wait KILL sweep for any group member that outlives the leader - Document the SIGKILL residual: no trap can run on a hard kill, and set -m places children outside the script's group, so that orphan risk cannot be closed from inside the script - Swap shellcheck disable=SC1091 for the source=/dev/null idiom on the verify-hooks.sh source line - Addresses PR #4 review comments; verified on bash 3.2/4.4/5.0/5.1/5.2 (macOS, Ubuntu 20/22/24.04, Debian 12, openSUSE Leap 15.5): the trap runs on INT/TERM, rc semantics stay 130, and no process leaks * EBP-2889: Remove stale line numbers and wave-plan relics from comments - Remove all sample line-number references from implement-guaranteed-pubsub.md; the numbers drifted from the current samples and the text already names each construct - Remove the stale line-number reference from the ENDPOINT_DENIED comment in verify.sh - Drop the 'NEW (waves 2-4)' annotations and the 'Placeholder stages' header in verify.sh; the direct and request-reply stages are fully implemented - Addresses PR #4 inline review feedback on comment maintenance * EBP-2889: Bound the app-stage trigger and isolate verify-hooks.sh imports - Run TRIGGER_CMD backgrounded in its own process group, polled to PUBLISH_WAIT_S, then escalate SIGTERM/SIGKILL; the app stage's trigger was the script's only unbounded wait, so a wedged app could hang an unattended run forever - Keep the marker watch after a hung trigger; the markers render the verdict, and the publish may have landed even when the HTTP response never returned - Reuse PUB_PID for the trigger so the EXIT trap's group kill covers an orphaned trigger too - Source verify-hooks.sh in a child bash and import only the four contract values via declare -p; a stray cd, variable, or function in the generated file can no longer clobber the observer state or force a false PASS - Add --max-time to the template curl and document the four-assignment contract in implement-mode.md - Verified on macOS bash 3.2.57: a hostile hooks file (status=0, TIMEOUT_S override, cd, stray stdout) plus a 300s trigger exits 1 in 18s with no leaked processes; the pass path exits 0 - Addresses Claude-review findings 3 and 4 from PR #4 review feedback * EBP-2889: Note the documented reply-to shape in the guaranteed requestor sample - Add a comment at the setReplyTo site linking the canonical Queues page (well-known-queues section), which documents anonymous/temporary queues in the reply-to field for request/reply - The comments-follow-their-code contract carries the note into generated adaptations * EBP-2938: Add output evals for solace-messaging-skills (#6) * EBP-2938: Add output-contract evals for solace-messaging-skills - Add tools/run-output-evals.sh, a local runner that plays scripted multi-turn cases through the Claude Code CLI with only the plugin under test loaded, then grades the transcript and the generated files - Add plugins/solace-messaging-skills/evals/output-evals.json with 11 cases across the three skills, 5 marked must_pass - Grade with deterministic checks (assistant text, tool use, generated files, mvn compile, live sol-jcsmp release match) and a fixed-model LLM judge for routing and grounding criteria - Add an opt-in live_verify grader that runs the generated project's verify.sh against a broker named by OUTPUT_EVAL_BROKER_* variables and skips loudly when they are unset - Document the corpus format, run instructions, gate, and local-only status in the evals README - Model the runner on tools/run-trigger-evals.sh (isolation via --plugin-dir and a scratch CLAUDE_CONFIG_DIR, 90% gate, must_pass) * EBP-2938: Fix two skill contract gaps found by the output evals - Harden the Debug row in solace-application-development/references/jcsmp.md so the documentation redirect is the entire debug answer; memory-derived hypotheses, interim checks, and fixes are forbidden - State in solace-topic-best-practices/SKILL.md that the skill answers in chat only and never writes files - Both gaps surfaced as failing output-eval cases (appdev-debug-redirect-negative and topics-no-files-negative); both cases pass after these edits * EBP-2938: Simplify output-eval docs, broker input, and case prompts - Remove exact case counts from the evals README so the prose does not drift from the corpus - Remove the "No CI wiring" section from the evals README - Take broker credentials for live_verify from exported OUTPUT_EVAL_BROKER_* variables only; drop the optional env file - Report in the run summary whether live_verify executed, skipped, or never ran because the case failed before grading - Drop the run.sh and port details from two application-development case prompts; no grader depended on them * EBP-2938: Address review findings on the output-eval runner and corpus - Replace the Quickstart failover word check with a code grep for the reconnect block; the Step 6 checklist report legitimately names HA failover - Run the LLM judge tool-free (--tools "") from the run directory so a tool call cannot spend its only turn and project settings stay out of the judge session - Add a compile grader to the Solace Suggested case and make compile and maven_release_match iterate every generated pom for two-project layouts - Align the debug-redirect judge with the Debug row: fail troubleshooting from any source, treat a restated symptom as the acknowledgement - Accept bold field labels in the design summary greps (design mode leaves formatting to the model) - Validate OUTPUT_EVAL_RUNS as a positive integer (a zero value crashed bash 3.2 on an empty array expansion) - Classify a timeout kill (exit 124) as INFRA in compile and live_verify - Carry the cause onto turn-level INFRA lines and print a RETRY line before the second attempt - Show the live-verify skip tag on FAIL lines; document that a max_turns hit fails the case - Drop the run-duration claim from the evals README; mention output evals in the root README Verified locally: claude-sonnet-5 and claude-opus-5 legs over the six affected cases pass, with live verify green against a real broker on both models. * EBP-2938: Add a corpus JSON Schema and rename the turn fields - Add tools/output-evals.schema.json (JSON Schema draft 2020-12) that defines the output-eval corpus with per-grader field rules and a description for every field, in response to review feedback - Rename the corpus fields turns to user_turns and max_turns to max_agent_turns to separate the scripted user messages from the CLI's agent-turn budget; update the runner and the corpus accordingly - Point the evals README and the runner header at the schema; the runner keeps enforcing the structural subset at start-up - Add a prerequisites list and a transcript-to-corpus mapping table (observed with Claude Code 2.1.266) to the evals README * EBP-2938: Document the timeout dependency and the credential requirement - Resolve GNU timeout with a gtimeout fallback for the mvn compile and verify.sh bounds, and warn once when neither is on PATH and a selected case needs Maven, instead of running unbounded silently - List timeout as an optional prerequisite in the evals README with the unbounded fallback and the macOS install hint - State in both README credential notes that an existing claude login does not satisfy the runner, because the throwaway config directory carries no login * EBP-2938: Treat permission denials as INFRA and pin LF endings for scripts - Read permission_denials from every turn's result event: a denial of a tool the runner grants is INFRA at once, with the tool and command on the case line and no retry, because the same policy would deny the retry; a denial of any other tool is tagged on the case line - Correct the header claim that the allowlist prevents denials; managed settings, hooks, and command shims can still deny a granted tool, and a denied cp became a lossy retype in a Windows run - Add .gitattributes with *.sh text eol=lf so Windows checkouts keep shell scripts LF for bash and for the byte-identity grader on verify.sh - Document Git Bash and LF on Windows, and denials as INFRA, in the evals README * EBP-2938: Require repro steps and a Read-only tool contract in the feedback skill - Make Steps to reproduce mandatory for bug-shaped feedback and derive it from the report when the session holds no skill invocation; a Windows sonnet run dropped the section under the old wording - Drop the Notes heading when empty and forbid None or N/A filler under it - State that Read is the skill's only tool and that the plugin manifest is read at its announced path, never located with a shell command; three-run checks showed sonnet running find for the manifest - Add a grader to feedback-support-email-positive that forbids a line consisting only of None - Three sonnet runs of the case pass by majority vote after these edits * EBP-2938: Copy the bundled verify.sh with cat instead of cp - Prescribe the copy command in implement-mode Step 4: cat the bundled script into the project root and chmod it, and forbid cp on the bundled file - Claude Code protects the plugin directory and blocks a cp that names a file inside it, even with Bash allowed; the blocked copy fell back to a retype of the script that must stay byte-identical, and the runner now reports that denial as INFRA - Reproduced on macOS with the runner's flags: cp denied, cat redirect allowed and byte-identical - Two sonnet runs of appdev-quickstart-implement-full with this wording used the cat form with zero denials; the second passed every grader * EBP-2938: Ignore no-op commands and summary remarks in the output evals - Add exclude_pattern to the four bare Bash-absent graders so a bare echo, true, or : (plus ls and pwd on the topics negative) no longer fails a case; a redirect, a chained command, a search, or a transport call still does - Sonnet ended feedback turns with echo done in about a third of runs and opus listed the directory once before it refused to write the file, while the graded content was correct every time - Tell the grounding judge that deriving the consequences of the fetched rules for the scenario is applying the guidance, and tell every judge that WebFetch results are the tool's summaries, not the page; a summary remark that guidance was absent had failed a correct opus answer - Correct the README Windows note: git checkout -- . leaves a stale CRLF checkout in place because git treats the files as unchanged; document the delete-and-checkout recipe * EBP-2938: Flag decisions beyond the fetched page in the topics skill - Add one sentence to step 3: when the user's scenario needs a decision the fetched page does not cover, say so before suggesting one - The grounding judge already accepts flagged extrapolation, and sonnet folded command topics into the page's template unflagged in 2 of 4 runs - Fifteen single sonnet runs of topics-grounded-answer pass after the edit; on the final tree the full sonnet leg is 11/11 and the opus leg 10/11, live verify skipped * EBP-3061: Bump solace-messaging-skills to 0.1.1 for release (#7) - Bump version in plugins/solace-messaging-skills/.claude-plugin/plugin.json from 0.1.0 to 0.1.1 - Patch release per the pre-1.0.0 rules: behavior-preserving fixes and documentation since 0.1.0 - Release-prep PR into dev, the only PR into dev that may touch a plugin version
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
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.
What is the purpose of this change?
Trigger evals only check that the right skill fires. Nothing checks that a skill's output honors its contract (design before code, the door question, scrubbed feedback drafts, grounded answers). This PR adds a local output-eval suite for
solace-messaging-skillsand fixes two skill contract gaps that the suite found.How is this accomplished?
tools/run-output-evals.sh, modeled ontools/run-trigger-evals.sh. It runs oneclaude -pinvocation per scripted user turn and continues the session with--resume, under--plugin-dirand a scratchCLAUDE_CONFIG_DIR. The subject gets the full tool allowlist, including bare Bash, so headless denials do not distort the measured behavior.plugins/solace-messaging-skills/evals/output-evals.json: 11 cases (5 application development, 4 feedback, 2 topic best practices), 5 markedmust_pass(the four forbidden-behavior negatives plus the compile anchor). A case passes only when the target skill fired, every grader passed, and no turn hit themax_turnscap.assistant_grep,tool_use,file_exists,file_grep,java_disclaimer,compile(a realmvn compileof every generated pom, so the two-project Solace Suggested layout is proven whole),maven_release_match(every generated pom carries the live sol-jcsmp release),llm_judge(tool-free, fixed judge model, defaultclaude-sonnet-5), andlive_verify(opt-in: runs the generated project'sverify.sh roundtripagainst a broker named by fourOUTPUT_EVAL_BROKER_*variables, skips loudly when none is set, errors on a partial set, and keeps the values out of the subject model's environment). Both the Quickstart case and the Solace Suggested case compile their output.must_passfailure or infrastructure failure fails the run. Atimeoutkill counts as infrastructure, a turn-level infrastructure failure prints its cause, and a retry is announced.solace-application-development/references/jcsmp.md: the documentation redirect is the entire debug answer. The suite caught the model appending memory-derived checks after the redirect.solace-topic-best-practices/SKILL.mdthat the skill answers in chat only and never writes files. The suite caught it writing atopic-conventions.mdwhen asked to.Anything reviews should focus on/be aware of?
claude-sonnet-5. Initial full legs, one run per case:claude-sonnet-510 of 11 (a case-script ambiguity, fixed and re-run green) andclaude-opus-510 of 11 (a grader false positive, scoped and re-run green). Both skill fixes were validated by re-running their cases green.claude-sonnet-56 of 6;claude-opus-55 of 6, where the one failure was the tightened debug judge misreading a restated symptom as a hypothesis. The criteria now say a restated symptom is the acknowledgement, and the case re-ran green on both models.live_verifyhas run green against a real broker on both models: the generated Quickstart project'sverify.sh roundtripcompleted end to end. Each run leaves one durable queue on the broker.SSL_TRUST_STORE, the solrsearch text in the pom and checklist), so absent-checks are scoped to a tool or construct, never to a bare substring. A word check on prose fails when the skill itself requires a sentence that contains the word (the Step 6 checklist report names HA failover on every door), so the Quickstart posture is graded on the generated code instead. A judge criteria that bans hypotheses must say that restating the reported symptom is the acknowledgement.topics-no-files-negativeis notmust_pass.caffeinate -i; machine sleep surfaces as INFRA failures.