Skip to content

fix(#6764): fail when agent definition name mismatches requested name - #6766

Open
fullsend-ai-coder[bot] wants to merge 5 commits into
mainfrom
agent/6764-validate-agent-name
Open

fix(#6764): fail when agent definition name mismatches requested name#6766
fullsend-ai-coder[bot] wants to merge 5 commits into
mainfrom
agent/6764-validate-agent-name

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

Validate that the agent definition's frontmatter name: field matches the requested agent name during Bootstrap, for both the Claude and Pi runtimes. Previously, a mismatch (e.g., harness config says coder but the definition declares name: code) caused the runtime to silently fall back to the default agent, producing an unconstrained run with none of the intended agent behavior.

Changes

  • Add validateAgentName() in internal/runtime/claude.go that reads the agent definition, parses the YAML frontmatter name: field, and returns an error when it doesn't match the requested name
  • Call validateAgentName() from ClaudeRuntime.Bootstrap before uploading the agent definition
  • Add inline name validation to PiRuntime.Bootstrap after parsing the agent definition (avoids a redundant file read since pi already parses the definition)
  • When the definition has no frontmatter or no name: field, validation is skipped — the runtime uses its own resolution chain
  • Error message includes both names and advises updating the harness config or definition frontmatter

Testing

  • TestValidateAgentName — table-driven tests covering match, mismatch, empty requested name, no frontmatter, empty frontmatter name, and nonexistent file
  • TestBootstrap_AgentNameMismatch / TestBootstrap_AgentNameMatch — integration tests for Claude runtime Bootstrap
  • TestPiRuntimeBootstrap_AgentNameMismatch / TestPiRuntimeBootstrap_AgentNameMatch — integration tests for Pi runtime Bootstrap
  • All existing internal/runtime/... tests pass
  • 100% coverage on the new validateAgentName function

Closes #6764

Post-script verification

  • Branch is not main/master (agent/6764-validate-agent-name)
  • Secret scan passed (gitleaks — dc0f8e15488fc816f9464c63e644e17de549edd1..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

Claude Code resolves --agent by the frontmatter name: field, not the
filename. When the harness config names an agent "coder" but the
definition declares name: "code", Claude silently falls back to the
default agent — producing an unconstrained run with none of the
intended agent behavior.

Add validateAgentName() to ClaudeRuntime.Bootstrap that reads the
agent definition, parses the frontmatter name: field, and returns an
error when it does not match the requested agent name. Add the same
check inline in PiRuntime.Bootstrap (which already parses the
definition). When the definition has no frontmatter or no name:
field, validation is skipped — the runtime uses its own resolution
chain.

The error message includes both names and advises updating the
harness config or definition frontmatter.

Closes #6764
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 29, 2026 19:52
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Agent PR ready for human review label Aug 29, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 29, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:54 PM UTC · Completed 8:10 PM UTC

Commit: 341aa26 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.08

@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/runtime/claude.go 50.00% 1 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Aug 29, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 29, 2026

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Small focused bug fix (5 files, 188 lines) by a bot author with no protected or security-sensitive paths; elevated Tier 2 churn and fix history in the runtime package is offset by low Tier 1 metadata signals and strong Tier 3 issue alignment. Score unchanged from prior assessment.

Previous run

Risk Assessment: moderate (2/5)

Details

Small focused bug fix (4 files, 183 lines) by a bot author with 50% test file ratio and no protected or security-sensitive paths; elevated Tier 2 fix/revert history is offset by low Tier 1 metadata signals and well-scoped issue alignment. Score unchanged from prior assessment.

Previous run (2)

Risk Assessment: moderate (2/5)

Details

Small focused bug fix (4 files, 188 lines) by a bot author with 50% test file ratio and no protected or security-sensitive paths; elevated Tier 2 fix/revert history is offset by low Tier 1 metadata signals and well-scoped issue alignment. Score unchanged from prior assessment.

Previous run (3)

Risk Assessment: moderate (2/5)

Details

Small focused bug fix (4 files, 183 lines) by a bot author with 50% test file ratio and no protected or security-sensitive paths; elevated Tier 2 fix/revert history (9.5 avg) is offset by low Tier 1 metadata signals and well-scoped issue alignment.

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 29, 2026

Copy link
Copy Markdown

Review

Findings

Low

  • [fail-open] internal/runtime/bootstrap.go:58validateAgentName silently swallows parsePiAgent errors by writing to stderr and returning nil. If a Claude agent definition has malformed frontmatter, the name mismatch guard is skipped. In practice this is not exploitable: Claude Code does its own native parsing, and a definition without valid frontmatter returns an empty Name that causes validateAgentNameMatch to bail via the empty-string guard — the guard only activates when both names are non-empty.
Previous run

Review

Findings

Low

  • [code-organization] internal/runtime/claude.go:522validateAgentNameMatch is a shared helper called from both claude.go (via validateAgentName) and pi_bootstrap.go, but it is defined in claude.go. The codebase convention places cross-runtime unexported helpers in their own files (sanitize.go for sanitizeOutput, bootstrap.go for BootstrapInput). Placing a shared helper inside one runtime's file creates a misleading ownership signal.
    Remediation: Move validateAgentNameMatch (and optionally validateAgentName) to bootstrap.go alongside BootstrapInput, or to a new validate.go file.

  • [error-handling-idiom] internal/runtime/claude.go:556 — The stderr message uses a "warning: skipping ..." prefix that has no precedent in the package. All other fmt.Fprintf(os.Stderr, ...) calls in internal/runtime use either a label prefix ("Skill %q:", "Plugin %q:", "[%s]") or a bare descriptive phrase without a severity word.
    Remediation: Align with the existing label style, e.g.: fmt.Fprintf(os.Stderr, "Agent name validation: skipped for %s: %v\n", agentPath, err).


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (2)

Review

Findings

Low

  • [fail-open] internal/runtime/claude.go:553validateAgentName swallows parsePiAgent errors (returning nil), creating asymmetric behavior compared to PiRuntime.Bootstrap which propagates the same error. The fail-open is not practically exploitable (if parsePiAgent cannot parse a name, there is no name to mismatch against) but reduces operator visibility when definitions have structural issues.
    Remediation: Consider logging a warning when parsePiAgent fails so operators have visibility.

  • [naming-convention] internal/runtime/claude.go:523 — The package uses validate* for functions that return an error on invalid input (validateConfigRuntime, validateHTTPURL). The new checkAgentName breaks this convention.
    Remediation: Rename checkAgentName to validateAgentNameMatch to align with the established convention.

  • [error-handling-idiom] internal/runtime/claude.go:529 — Error message uses a Unicode em-dash (U+2014) and multi-sentence format with explanatory prose. All other fmt.Errorf calls in the package use ASCII punctuation and terse noun-phrase messages.
    Remediation: Shorten to a terse diagnostic: fmt.Errorf("agent name mismatch: requested %q but definition declares %q", requestedName, definitionName).


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (3)

Review

Findings

Medium

  • [fail-open] internal/runtime/claude.go:530validateAgentName swallows parsePiAgent errors (returning nil) for malformed frontmatter that parsePiAgent reports as hard errors (e.g., broken fence lines, unterminated frontmatter). This silently skips name validation on broken definitions, while PiRuntime.Bootstrap correctly propagates the parse error (line 93). A malformed agent definition with a name mismatch would bypass the guard on ClaudeRuntime but fail on PiRuntime.
    Remediation: Propagate parsePiAgent errors rather than swallowing them. Only skip validation when parsePiAgent succeeds but def.Name is empty (no frontmatter or unnamed agent).

  • [code-organization] internal/runtime/pi_bootstrap.go:95 — The agent-name mismatch check duplicates the logic and the multi-line error format string from validateAgentName in claude.go. The codebase convention is to share cross-runtime validation via package-level helpers (duplicateDestinationNameError, agentDestName, parsePiAgent). The helper's signature (agentPath string) forces redundant IO in one runtime and prevents reuse in the other.
    Remediation: Refactor to func validateAgentName(requestedName, definedName string) error — accept the already-parsed definition name. Call from both ClaudeRuntime.Bootstrap and PiRuntime.Bootstrap.

Low

  • [fail-open] internal/runtime/claude.go:525validateAgentName returns nil when os.ReadFile fails, skipping validation. In practice, sandbox.UploadFile will also fail on the same path (it calls os.Lstat on the local path), so this is not exploitable — but it diverges from PiRuntime's fail-closed pattern where the read error is propagated.
    Remediation: Read the file in ClaudeRuntime.Bootstrap and pass bytes to the validator, mirroring the Pi pattern.

  • [error-handling-idiom] internal/runtime/claude.go:531 — Error message uses a Unicode em-dash (U+2014) and multi-sentence format with imperative remediation advice. All other fmt.Errorf calls in claude.go and pi_bootstrap.go use ASCII punctuation and terse noun-phrase messages (e.g., "creating runtime config dirs: %w").
    Remediation: Replace the em-dash with -- or restructure, and shorten to the diagnostic fact.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

@ralphbean

Copy link
Copy Markdown
Member

/fs-fix is there any way to reduce code duplication here?

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Fix · ⚠️ Cancelled · Started 8:12 PM UTC · Ended 8:12 PM UTC

Commit: 341aa26 · View workflow run →

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 8:14 PM UTC · Completed 8:20 PM UTC

Commit: 341aa26 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.18

…lication

The agent-name-mismatch validation logic and error message were
duplicated between validateAgentName (claude.go) and inline code in
PiRuntime.Bootstrap (pi_bootstrap.go). Extract a shared checkAgentName
helper that both call sites delegate to.

Addresses review feedback on #6766
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (human-triggered)

Extracted shared checkAgentName helper to eliminate duplicated agent-name-mismatch validation logic and error message between claude.go and pi_bootstrap.go. All existing tests pass unchanged.

Fixed (1):

  1. reduce code duplication between Claude and Pi agent name validation (internal/runtime/claude.go): Extracted a shared checkAgentName(requestedName, definitionName string) helper that contains the comparison logic and error message. validateAgentName in claude.go now delegates to it after reading/parsing the file, and PiRuntime.Bootstrap calls it directly instead of duplicating the inline check and format string.

Tests: passed

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 29, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:22 PM UTC · Completed 8:40 PM UTC

Commit: 5a87c33 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.26

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 8:42 PM UTC · Completed 8:49 PM UTC

Commit: 5a87c33 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.48

- Rename checkAgentName to validateAgentNameMatch to follow the
  established validate* naming convention in the package
- Shorten mismatch error message to terse ASCII diagnostic matching
  the existing fmt.Errorf style in the package
- Log a warning to stderr when parsePiAgent fails in validateAgentName
  so operators have visibility into structural definition issues

Addresses review feedback on #6766
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 2 (bot-triggered)

Addressed all 3 review findings: renamed checkAgentName to validateAgentNameMatch for naming consistency, shortened error message to terse ASCII diagnostic, and added stderr warning when parsePiAgent fails.

Fixed (3):

  1. naming-convention: checkAgentName breaks validate convention* (internal/runtime/claude.go): Renamed checkAgentName to validateAgentNameMatch to align with the established validate* naming convention (validateConfigRuntime, validateHTTPURL). Updated call site in pi_bootstrap.go.
  2. error-handling-idiom: error message uses Unicode em-dash and verbose prose (internal/runtime/claude.go): Shortened the mismatch error to a terse ASCII diagnostic matching the package style: fmt.Errorf("agent name mismatch: requested %q but definition declares %q", ...).
  3. fail-open: validateAgentName swallows parsePiAgent errors silently (internal/runtime/claude.go): Added fmt.Fprintf(os.Stderr, ...) warning when parsePiAgent fails so operators have visibility into structural definition issues.

Tests: passed

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 29, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:51 PM UTC · Completed 9:06 PM UTC

Commit: 1886fbc · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.64

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 9:08 PM UTC · Completed 9:15 PM UTC

Commit: 1886fbc · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.53

Move validateAgentNameMatch and validateAgentName from claude.go to
bootstrap.go alongside BootstrapInput — both are cross-runtime bootstrap
helpers, and the codebase convention places shared helpers in their own
files (cf. sanitize.go).

Align the stderr message with the package's label-style prefix convention:
"Agent name validation: skipped for ..." replaces the precedent-less
"warning: skipping ..." prefix.

Addresses review feedback on #6766
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 3 (bot-triggered)

Addressed both review findings: moved shared validation helpers from claude.go to bootstrap.go for proper code organization, and aligned the stderr message format with the package's label-style prefix convention.

Fixed (2):

  1. validateAgentNameMatch shared helper defined in claude.go instead of a shared file (internal/runtime/bootstrap.go): Moved validateAgentNameMatch and validateAgentName from claude.go to bootstrap.go alongside BootstrapInput, matching the codebase convention of placing cross-runtime helpers in their own files (cf. sanitize.go for sanitizeOutput)
  2. stderr message uses unprecedented warning: prefix (internal/runtime/bootstrap.go): Changed stderr format from 'warning: skipping agent name validation for %s: %v' to 'Agent name validation: skipped for %s: %v' to align with the package's label-style prefix convention (e.g. 'Skill %q:', 'Plugin %q:', '[%s]')

Tests: passed

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 29, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:17 PM UTC · Completed 9:33 PM UTC

Commit: d1684cf · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.90

@fullsend-ai-review
fullsend-ai-review Bot dismissed stale reviews from themself August 29, 2026 21:33

Superseded by updated review

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Aug 29, 2026
@ralphbean

Copy link
Copy Markdown
Member

/fs-fix fix the open code duplication (code-organization) finding

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 5:14 PM UTC · Completed 5:25 PM UTC

Commit: d1684cf · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.22

… duplication

Remove the validateAgentName file-reading wrapper from bootstrap.go and
inline its logic in ClaudeRuntime.Bootstrap. This leaves a single shared
helper (validateAgentNameMatch) that both runtimes call directly with
pre-extracted names, eliminating the duplicated read→parse→validate
pattern between the wrapper and PiRuntime.Bootstrap.

Addresses #6766
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 4 (human-triggered)

Removed validateAgentName wrapper from bootstrap.go to eliminate code duplication between the wrapper and PiRuntime.Bootstrap. Both runtimes now call validateAgentNameMatch directly. Updated tests to cover the shared comparison helper.

Fixed (1):

  1. code duplication (code-organization): validateAgentName wrapper duplicates the read-parse-validate pattern used inline by PiRuntime.Bootstrap (internal/runtime/bootstrap.go): Removed the validateAgentName file-reading wrapper from bootstrap.go, leaving only validateAgentNameMatch as the single shared helper. Inlined the read+parse+validate logic in ClaudeRuntime.Bootstrap in claude.go. Both runtimes now call validateAgentNameMatch directly with pre-extracted names, eliminating the duplicated pattern.

Tests: passed

Strategy change: Iterations 1-3 added/moved helper functions. Iteration 4 takes the opposite approach: removes the intermediate wrapper entirely, collapsing to a single shared helper with inline callers.

Decision points
  • Strategy escalation at iteration 4: instead of adding or moving wrapper functions (iterations 1-3), removed the wrapper entirely and inlined the Claude-specific file I/O (alternatives: Keep two-layer abstraction (validateAgentName wrapping validateAgentNameMatch), Remove wrapper and inline in ClaudeRuntime.Bootstrap; rationale: The wrapper duplicated the read-parse-validate pattern that PiRuntime does inline. Collapsing to a single shared helper (validateAgentNameMatch) with runtime-specific callers eliminates the duplication while preserving each runtime's error handling semantics.)

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@ralphbean
ralphbean enabled auto-merge August 31, 2026 18:25
@ralphbean
ralphbean added this pull request to the merge queue Aug 31, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 31, 2026
@ralphbean
ralphbean added this pull request to the merge queue Aug 31, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge ready-for-review Agent PR ready for human review risk/moderate PR risk: moderate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fail when the requested Claude agent name is unavailable

1 participant