parser: adopt structured duplicate-name validation errors for inline skills and sub-agents

[Copilot]

pkg/parser returned plain duplicate-name errors for inline skills/sub-agents, which lacked field context and remediation guidance. This change introduces structured validation output at those sites so duplicate definitions report reason + actionable suggestion consistently.

• Structured duplicate-name errors in parser extraction paths

  • Replaced duplicate-name fmt.Errorf(...) returns in:
    • inline_skill_extractor.go
    • sub_agent_extractor.go
  • Both now emit NewValidationError(field, value, reason, suggestion) with:
    • field: skills / sub-agents
    • value: duplicate name
    • reason: duplicate name already defined
    • suggestion: rename or remove one duplicate definition.

• Parser-local validation error helper

  • Added pkg/parser/validation_error.go with parser-scoped ValidationError + NewValidationError(...).
  • Error formatting includes field/value/reason/suggestion in a deterministic message shape.

• Duplicate-name test coverage updated

  • Updated duplicate-name assertions in:
    • inline_skill_extractor_test.go
    • sub_agent_extractor_test.go
  • Tests now verify structured message content (field, value, reason, suggestion), not just generic “duplicate” text.

return NewValidationError(
    "skills",
    name,
    "duplicate name already defined",
    fmt.Sprintf("Rename one of the duplicate skills or remove the extra `%s` definition.", name),
)

• Fixes [deep-report] Adopt NewValidationError for duplicate-name errors in pkg/parser #54582

---

Run: https://github.com/github/gh-aw/actions/runs/32563890634> Generated by 👨‍🍳 PR Sous Chef · gpt54 · 19.3 AIC · ⌖ 8.31 AIC · ⊞ 9.5K · ◷

_{Comment /souschef to run again}

---

run https://github.com/github/gh-aw/actions/runs/32565294954> Generated by 👨‍🍳 PR Sous Chef · gpt54 · 11.9 AIC · ⌖ 8.18 AIC · ⊞ 9.5K · ◷

_{Comment /souschef to run again}

[github-actions]

PR Triage

• Category: refactor
• Risk: low
• Priority score: 30/100 (impact 10, urgency 5, quality 15)
• Recommended action: defer
• CI status: no_checks

Generated by 🔧 PR Triage Agent · auto · 82.9 AIC · ⌖ 6.65 AIC · ⊞ 8.3K · ◷

[Copilot]

Pull request overview

Introduces actionable duplicate-name validation for inline skills and sub-agents.

Changes:

• Adds structured parser validation errors.
• Updates duplicate-name extraction paths and tests.

Show a summary per file

File	Description
pkg/parser/validation_error.go	Adds parser validation-error types.
pkg/parser/inline_skill_extractor.go	Returns actionable duplicate-skill errors.
pkg/parser/inline_skill_extractor_test.go	Verifies skill error details.
pkg/parser/sub_agent_extractor.go	Returns actionable duplicate-agent errors.
pkg/parser/sub_agent_extractor_test.go	Verifies agent error details.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Balanced

[gh-aw-bot]

@copilot this PR still needs forward progress before maintainer review.

• Failed checks on the current head:
  • impacted-go-tests: https://github.com/github/gh-aw/actions/runs/32550824880/job/97003128404
  • lint-go-custom: https://github.com/github/gh-aw/actions/runs/32550824880/job/97003128654
• Please refresh the branch if needed, address the failing checks, and leave a concise maintainer handoff with the exact validations rerun.
• Then run the pr-finisher skill to wrap up reviewer feedback and final cleanup.

Run: https://github.com/github/gh-aw/actions/runs/32563890634

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 19.3 AIC · ⌖ 8.31 AIC · ⊞ 9.5K · ◷
_{Comment /souschef to run again}

[Copilot]

@copilot this PR still needs forward progress before maintainer review.

• Failed checks on the current head:...

Investigated both failures:

• lint-go-custom: the errormessage analyzer scopes checks to files in the PR's changed-file diff, which now includes inline_skill_extractor.go/sub_agent_extractor.go (touched by this PR). Two pre-existing fmt.Errorf("invalid inline skill/sub-agent end marker: %w", err) messages in those files tripped the negative-language rule. Reworded them to "inline skill/sub-agent end marker should reference a valid ... name: %w" in fdcb (see latest commit) — no behavior change, just wording. Confirmed clean by running the same scoped linter invocation locally (-errormessage -errormessage.changed-files=<diff files> plus the largefunc/wasm variants from cgo.yml).
• impacted-go-tests: failed with unable to determine merge-base from BASE_REF=origin/main — that run's cached checkout didn't have origin/main resolvable, so git merge-base errored before any test selection happened. This is a checkout/cache environment issue, not caused by this PR's diff: locally, git fetch origin main + git merge-base origin/main HEAD resolves fine, and make test-impacted-go BASE_REF=origin/main CI_COVERAGE_SOURCE_BRANCH=main passes (ok pkg/parser). Should resolve automatically on rerun with a fresh checkout cache.

Validations rerun locally on the latest commit:

• go test ./pkg/parser/... → pass
• make test-impacted-go BASE_REF=origin/main CI_COVERAGE_SOURCE_BRANCH=main → pass
• Scoped custom linters (errormessage + largefunc, native and GOOS=js GOARCH=wasm) matching lint-go-custom's exact flags → no findings

[gh-aw-bot]

@copilot
Please refresh the branch if needed and continue with the pr-finisher skill.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 11.9 AIC · ⌖ 8.18 AIC · ⊞ 9.5K · ◷
_{Comment /souschef to run again}

[Copilot]

@copilot
Please refresh the branch if needed and continue with the pr-finisher skill.
...

Status check on this pass:

• Local validation: go build ./..., go test ./pkg/parser/..., make fmt all pass on the current head; no code changes were needed beyond the prior commit (fdcb…) that fixed the lint-go-custom/impacted-go-tests failures.
• Open review thread (pkg/parser/validation_error.go:8-9, duplicate ValidationError/NewValidationError API vs workflow.WorkflowValidationError): I evaluated a package-boundary translation fix in pkg/workflow/compiler_orchestrator_tools.go. That file already has ~25 pre-existing errormessage-lint violations ("failed to …: %w" wrapping / negative language), and the CI errormessage analyzer scopes to whole changed files, not diff hunks — so touching that file at all, even for an unrelated one-line change, would pull in all of those pre-existing findings and fail lint-go-custom. Fixing those is out of scope for this PR, so I reverted that attempt rather than bundle an unrelated cleanup with this fix. Leaving the thread open for a maintainer decision on whether a wider refactor (e.g. a shared dependency-neutral validation-error package, done as its own PR) is wanted.
• CI: the latest run on this head (a9e31e5) shows action_required with no jobs — it needs a maintainer to approve/re-trigger the workflow run; the agent can't do that.
• Mergeable: PR shows blocked, pending the two requested reviews.

[gh-aw-bot]

@copilot please refresh the branch if needed and continue with the pr-finisher skill

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 12.5 AIC · ⌖ 8.2 AIC · ⊞ 7K · ◷
_{Comment /souschef to run again}

[Copilot]

@copilot please refresh the branch if needed and continue with the pr-finisher skill

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 12...

Branch is already up to date with main (merge-base matches origin/main HEAD, no rebase needed). No code changes required: go build ./... and go test ./pkg/parser/... pass on the current head (a9e31e5). The remaining CGO/CWI checks on this head show action_required, meaning they're waiting on a maintainer to approve/trigger the workflow run — this isn't something the agent can resolve. The open review thread on pkg/parser/validation_error.go remains for maintainer decision as noted in the prior update.

[gh-aw-bot]

@copilot this PR still needs forward progress before maintainer review.

• The open review thread on pkg/parser/validation_error.go still needs a maintainer-facing resolution or a scoped follow-up; please either address it on the current head or leave a concise explanation in-thread for the chosen boundary.
• Refresh the branch if needed, rerun the exact parser validations touched by the final change, and then run the pr-finisher skill.
• Leave a concise handoff with exact commands rerun and any remaining human follow-up.

Failed checks listed on the current head:

• impacted-go-tests: https://github.com/github/gh-aw/actions/runs/32550824880/job/97003128404
• lint-go-custom: https://github.com/github/gh-aw/actions/runs/32550824880/job/97003128654

Run: https://github.com/github/gh-aw/actions/runs/32568456526

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 11 AIC · ⌖ 8.2 AIC · ⊞ 7K · ◷
_{Comment /souschef to run again}

[Copilot]

@copilot this PR still needs forward progress before maintainer review.

• The open review thread on pkg/parser/validation_error.go still needs a maintainer-facing resolution or a scoped follow-up; ...

Resolved in f866ce5: extracted a shared, dependency-neutral pkg/validationerror package (Payload struct + Format(...) helper + a ValidationError interface). pkg/parser.ValidationError and pkg/workflow.WorkflowValidationError now both embed validationerror.Payload, so formatting fixes live in one place and callers can uniformly detect either type via errors.As(err, &validationerror.ValidationError) — added TestValidationError_UniformAcrossPackages in pkg/workflow/error_recovery_test.go to cover this. WorkflowValidationError keeps its own Severity/Category/Timestamp/File/Line/Column fields, since those are workflow-specific.

Validations rerun on this head: go build ./...; go test ./pkg/parser/... ./pkg/workflow/... ./pkg/validationerror/... (all pass); make fmt (clean); scoped errormessage/largefunc custom linters (native, matching lint-go-custom's exact flags) against every touched file (no findings); make test-impacted-go BASE_REF=origin/main CI_COVERAGE_SOURCE_BRANCH=main (pass). code_review and codeql_checker tools were also run — no actionable findings.

[github-actions]

PR Triage

Category: refactor | Risk: low | Priority: low | Score: 38/100
Recommended action: defer

Structured duplicate-name validation errors for parser. Reviewer commented only, no CI data.

_{Automated triage — run 32572524009}

Generated by 🔧 PR Triage Agent · auto · 65.4 AIC · ⌖ 6.04 AIC · ⊞ 8.3K · ◷

[gh-aw-bot]

@copilot this PR still needs forward progress before maintainer review.

• Please refresh the branch if possible and run the pr-finisher skill before handoff.
• Revisit any unresolved review feedback, rerun the relevant validations, and leave a concise maintainer handoff with exact commands and outcomes.

Run: https://github.com/github/gh-aw/actions/runs/32573431024

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 9.09 AIC · ⌖ 8.39 AIC · ⊞ 9.5K · ◷
_{Comment /souschef to run again}

[Copilot]

@copilot this PR still needs forward progress before maintainer review.

• Please refresh the branch if possible and run the pr-finisher skill before handoff....

Branch refreshed: merged main (2 commits, no conflicts) in the latest commit. The pkg/parser/validation_error.go thread was already resolved in f866ce5 via the shared pkg/validationerror package; re-verified it still holds after the merge.

Validations rerun on the current head: go build ./...; go test ./pkg/parser/... ./pkg/workflow/... ./pkg/validationerror/... (pass); make fmt (clean); make test-impacted-go BASE_REF=origin/main CI_COVERAGE_SOURCE_BRANCH=main (pass); scoped errormessage/custom linters matching lint-go-custom's exact flags against all changed Go files (no findings). code_review and codeql_checker tools also ran clean.