feat(harness): add optional role and slug fields (ADR-0045 PR 2)

[ggallen]

Summary

• Adds optional role and slug fields to the Harness struct with validation
• role uses the same lowercase pattern as mintcore.RolePattern (^[a-z][a-z0-9_-]*$, no double hyphens) without importing it to avoid coupling harness → mintcore
• slug accepts GitHub App slug format (^[a-zA-Z0-9][a-zA-Z0-9_-]*$)
• Both fields are optional in Phase 1 — validated only when present

Part of the ADR-0045 implementation plan (PR 2 of 7). No dependencies on other PRs.

Test plan

• go test ./internal/harness/... — all 84 tests pass (6 new)
• TestHarnessesLoadAndValidate — scaffold templates still load
• make go-vet — clean
• make lint — passes
• Backward compat: harness without role/slug loads with empty strings

🤖 Generated with Claude Code

[github-actions]

Site preview

Preview: https://32801cb8-site.fullsend-ai.workers.dev

Commit: 246f73cdac023417f9174c188de93b506ccb72ac

[fullsend-ai-review]

🤖 Review · Started 4:04 PM UTC
Commit: 4ed6da4 · View workflow run →

[ggallen]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:31 PM UTC · Completed 4:39 PM UTC
Commit: 4ed6da4 · View workflow run →

[fullsend-ai-review]

Review

Findings

Low

• [missing-field-documentation] docs/ADRs/0024-harness-definitions.md:309 — ADR-0024 documents the canonical harness YAML schema but does not include the new role and slug fields. ADR-0045 already documents these fields extensively. Since role/slug are optional Phase 1 fields with no runtime consumers, this can be deferred to a later PR in the series.

Info

• [edge-case] internal/harness/harness.go:19 — The validRoleName regex permits trailing hyphens and underscores (e.g., role-). This is consistent with mintcore.RolePattern and mintcore.ValidateRoleName, so it is not a divergence bug.

• [comment-placement] internal/harness/harness.go:19 — The validRoleName regex has an inline comment explaining the duplication rationale, but other regex patterns in the same var block (validAgentName, validModelName, validPluginName) have no inline comments. The comment adds value by documenting the intentional mintcore duplication.

• [struct-field-alignment] internal/harness/harness.go:196 — The Harness struct has pre-existing inconsistent field alignment. The new Role and Slug fields correctly match the pattern of their adjacent fields.

• [missing-field-documentation] docs/guides/user/customizing-agents.md:11 — The user guide does not include role and slug fields. Since these are optional Phase 1 fields with no runtime consumers, updating user-facing docs before the fields have runtime effect would be premature.

Previous run

Review

Findings

Low

• [error-message-accuracy] internal/harness/harness.go:256 — The role validation error message states (allowed: a-z, 0-9, _, -) but the regex enforces that the role must start with a lowercase letter. The error message omits this start-character constraint, which could confuse users when inputs like 1role are rejected.
  Remediation: Update the error message to mention the start-character constraint.

• [error-message-accuracy] internal/harness/harness.go:262 — The slug validation error message states (allowed: a-z, A-Z, 0-9, _, -) but the regex enforces that the slug must start with an alphanumeric character. The error message omits this start-character constraint.
  Remediation: Update the error message to mention the start-character constraint.

• [missing-field-documentation] docs/ADRs/0024-harness-definitions.md:309 — ADR-0024 documents the canonical harness YAML schema but does not include the new role and slug fields. ADR-0045 already documents these fields extensively. Since role/slug are optional Phase 1 fields with no runtime consumers, this can be deferred to a later PR in the series.

• [missing-field-documentation] docs/guides/user/customizing-agents.md:11 — The user guide does not include role and slug fields. Since these are optional Phase 1 fields with no runtime consumers, updating user-facing docs before the fields have runtime effect would be premature.

Info

• [edge-case] internal/harness/harness.go:19 — The validRoleName regex permits trailing hyphens and underscores (e.g., role-). This is consistent with mintcore.RolePattern and mintcore.ValidateRoleName, so it is not a divergence bug.

• [scope-coherence] internal/harness/harness.go:196 — PR correctly limits scope to only role/slug field additions, confirming proper scope isolation per ADR-0045 implementation plan PR 2 specification.

• [architecture-alignment] internal/harness/harness.go:199 — Role pattern correctly mirrors mintcore.RolePattern with intentional duplication to avoid coupling harness→mintcore. Double-hyphen rejection matches mintcore.ValidateRoleName behavior.

Previous run (2)

Review

Findings

Medium

• [missing-field-documentation] docs/ADRs/0024-harness-definitions.md:309 — ADR-0024 documents the canonical harness YAML schema but does not include the new role and slug fields. ADR-0045 already documents these fields extensively, and they are optional in Phase 1 with no runtime consumers yet, so this is not urgent — but ADR-0024 remains the primary schema reference and should be updated before the fields become required.

• [missing-field-documentation] docs/guides/user/customizing-agents.md:11 — The user guide shows minimal harness configuration and optional fields but does not include role and slug. Since the fields are optional and not yet consumed, updating user-facing guides may be more appropriate when the fields gain runtime significance in a later PR.

Low

• [error-message-consistency] internal/harness/harness.go:256 — The role and slug validation error messages embed the start-character constraint within the allowed character list (e.g., contains invalid characters (allowed: a-z, 0-9, _, -, must start with lowercase letter)). Existing validators (agent, model, plugin) use the simpler format contains invalid characters (allowed: <list>) without additional constraints in the parenthetical. The double-hyphen check correctly uses a separate error message. Consider standardizing the format.

• [naming-consistency] internal/harness/harness.go:19 — The regex variable validSlugName uses a Name suffix, while validRole does not. The existing convention (validAgentName, validModelName, validPluginName) consistently uses the Name suffix. Either both new variables should have it (validRoleName, validSlugName) or both should omit it.

• [architecture-alignment] internal/harness/harness.go:18 — validRole intentionally duplicates mintcore.RolePattern to avoid coupling harness→mintcore. Consider adding a code comment noting this duplication and the mintcore source location, so future maintainers know to keep them in sync.

• [outdated-example] docs/ADRs/0024-harness-definitions.md:434 — The example triage and code harness definitions in ADR-0024 (lines 434, 492) do not include role or slug fields. Low priority while the fields remain optional in Phase 1.

• [missing-field-documentation] docs/guides/user/customizing-agents.md:245 — The harness customization example does not include role or slug fields. Low priority while the fields remain optional.

Info

• [edge-case] internal/harness/harness.go:19 — The validRole regex permits trailing hyphens and underscores (e.g., role-). This is consistent with mintcore.RolePattern and mintcore.ValidateRoleName, so it is not a divergence bug.

• [test-coverage-gap] internal/harness/harness_test.go:1103 — Tests do not cover boundary inputs like trailing-hyphen roles (e.g., "role-"), but these are intentionally valid per the regex pattern. Test coverage for actually-invalid inputs is adequate.

• [scope-coherence] internal/harness/harness.go:196 — PR correctly implements only role/slug fields without touching ForgeConfig or merge logic, confirming proper scope isolation per ADR-0045.

• [struct-field-ordering] internal/harness/harness.go:196 — Role and Slug fields are placed after Description and before Image, grouping with metadata fields. Reasonable organizational choice.

• [validation-rule-documentation] docs/ADRs/0045-forge-portable-harness-schema.md:254 — ADR-0045 documents the validation rules for role and slug, matching the implementation.

Previous run (3)

Review

Findings

Low

• [error-message-consistency] internal/harness/harness.go:256 — The role validation error message uses is invalid (must start with ...) while existing validators (model, plugin) use contains invalid characters (allowed: ...). The slug error message uses a hybrid: contains invalid characters (must start with ...). This creates three different error message patterns for structurally similar field validations. Consider standardizing to the established contains invalid characters (allowed: ...) pattern, with a separate message for the double-hyphen constraint.

Info

• [edge-case] internal/harness/harness.go:19 — The validRole regex permits trailing hyphens and underscores (e.g., role-). This is consistent with mintcore.RolePattern and mintcore.ValidateRoleName, so it is not a divergence bug.
• [test-coverage-gap] internal/harness/harness_test.go:1103 — Tests do not cover boundary inputs like trailing-hyphen roles (e.g., "role-"), but these are intentionally valid per the regex pattern. Test coverage for actually-invalid inputs is adequate.
• [struct-field-ordering] internal/harness/harness.go:196 — Role and Slug fields are placed after Description and before Image, which is a reasonable grouping with other metadata fields.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:57 PM UTC · Completed 5:08 PM UTC
Commit: 4ed6da4 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 5:13 PM UTC · Completed 5:23 PM UTC
Commit: 4ed6da4 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 5:32 PM UTC · Completed 5:42 PM UTC
Commit: 4ed6da4 · View workflow run →

[ralphbean]

LGTM.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 8:12 PM UTC · Completed 8:22 PM UTC
Commit: 246f73c · View workflow run →

[fullsend-ai-retro]

Retro: PR #2128 — feat(harness): add optional role and slug fields

This human-authored PR went through 4 review cycles as the author iteratively addressed bot feedback on error message formatting and naming consistency. The review agent performed well:

• All 3 inline findings were actionable and led to real code improvements (error-message-consistency, naming-consistency, error-message-accuracy)
• The bot correctly caught new issues introduced by fixes — e.g., after the author standardized error message format, the bot identified that start-character constraints were omitted from the new format
• Final review confirmed all issues resolved, with only a deferred documentation update remaining

Efficiency notes

• 7 pull_request_review events triggered fullsend.yaml but matched no dispatch stage — these are wasted CI runs. Already covered by existing issues: Filter pull_request_review trigger to only fire on changes_requested #1125 (filter pull_request_review trigger), dispatch: filter bot-triggered pull_request_review events to prevent review self-triggering loop #1271 (filter bot-triggered review events), Reduce redundant review dispatches triggered by non-code events #893 (reduce redundant review dispatches)
• The naming-consistency issue (validRole vs validSlugName) existed in the original commit but wasn't caught until the second review pass. This is a minor first-pass thoroughness gap, but hard to address without increasing false positives
• Redundant re-review deduplication (force-push scenarios) is covered by Cancel-and-skip redundant re-reviews on force-pushed PRs with no prior findings #1372, Deduplicate review runs on rapid successive pushes #1418, Deduplicate review runs when PR is rebased multiple times in quick succession #1422, Deduplicate review dispatches for the same HEAD SHA #1452 — though in this case all re-reviews found real issues, so dedup would not have helped

Verdict

No new proposals — the workflow was effective and all identified inefficiencies are already tracked by existing open issues.