feat(add-agent): opt-in --custom flag for non-catalog role names#28
Conversation
The roles package documents itself as an open set ("unknown role names
are valid and receive sensible defaults") and LookupRole correctly
returns defaults for arbitrary names. But IsValidRoleName at the CLI
gated add-agent to the catalog + numbered families, and its own doc
comment scoped the fix out: "Operators wanting truly custom roles must
add them to the Catalog or design a separate opt-in (out of scope here)."
This adds that opt-in. add-agent now accepts a --custom flag. When set,
names outside the catalog go through IsValidCustomRoleName, a pure
pattern check requiring ^[a-z][a-z0-9-]{0,31}$. The default closed-set
gate stays intact for typo protection. Custom roles get the bare default
RoleDef (Autonomous, no src, no playbooks) via LookupRole's existing
fallback, so no scaffold or runtime changes are needed.
Motivated by adding marketing-shaped roles (content, demandgen, events,
community) to a non-software project. The same opt-in lets future users
register designer, dba, or any other vocabulary their team needs.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR adds opt-in support for custom role names in the ChangesCustom Role Name Support via --custom Flag
Sequence Diagram(s)N/A — The changes are primarily validation logic and flag integration without multi-component request/response flows warranting a sequence diagram. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
The roles package documents itself as an open set:
unknown role names are valid and receive sensible defaults.LookupRole("designer")correctly returns a bare defaultRoleDef, andTestLookupRole_Unknownlocks that contract in. But the CLI gate inadd-agentrejects every non-catalog name, contradicting the open-set claim. The current validator (IsValidRoleName) acknowledges the gap in its own doc comment and explicitly scopes the fix out:This PR adds that opt-in.
What changed
--customoninitech add-agent. When set, names outside the catalog and numbered families go through a new pattern check rather than the closed-set check.roles.IsValidCustomRoleName— pure pattern validator:^[a-z][a-z0-9-]{0,31}$. Lowercase letter start, then up to 31 lowercase letters / digits / hyphens. Hyphens not underscores, consistent with the catalog's own naming. URL- and filesystem-friendly.IsValidRoleNameis unchanged. Default behavior is unchanged. Typo protection is preserved for callers who do not opt in.--custom.Custom roles flow through
LookupRole's existing bare-default branch (Autonomous, no src, no playbooks), so no scaffold or runtime changes are needed.Why
Initech is being used outside pure software-engineering org shapes. The motivating use case is a marketing fleet (content, demandgen, events, community) but the same gap blocks any non-software vocabulary: design teams, DBAs, sec-ops, customer success, etc. The previous workaround was either to misuse
eng4/qa10slots or to fork initech and edit the catalog. Both are bad.Backward compatibility
initech add-agent designerstill rejects withunknown agent. The new error message adds a hint about--custom.IsValidRoleNamesemantics unchanged. New callers can opt in toIsValidCustomRoleNameas a sibling.Regression tests
Three new tests in
cmd/add_agent_test.go:TestRunAddAgent_CustomFlag_AllowsArbitraryName—marketingis accepted with--customand the role lands ininitech.yamland on disk.TestRunAddAgent_CustomFlag_RejectsBadIdentifier— uppercase, leading digits, underscores, dots, and whitespace are rejected with a clear pattern error.TestRunAddAgent_WithoutCustomFlag_StillRejectsUnknown— locks in that--customis strictly opt-in. Also extendsTestRunAddAgent_UnknownRoleto require the--customhint in the error message.One new table test in
internal/roles/catalog_test.go:TestIsValidCustomRoleName_AcceptsAndRejects— pure unit coverage on the pattern (marketing, designer, dba, data-eng accepted; uppercase, leading digits, underscores, whitespace, 33-char names rejected; 32-char boundary accepted).Note on the "fail on main" requirement: this is a feature-add PR, not a fix-the-broken-code PR. The new tests reference symbols that do not exist on main (
addAgentCustomflag,IsValidCustomRoleNamefunction), so checking out main and running them produces a compile failure rather than a runtime failure. The semantic regression they protect against —add-agentrejecting valid custom names thatLookupRolealready supports — is locked in by the existence of the tests at all.Test plan
make checkpasses clean (vet + lint-test-names + test, 14 packages).go test ./cmd/ -run 'TestRunAddAgent_(Custom|Unknown|WithoutCustom)' -vpasses (5 new subtests + the existing TestRunAddAgent_UnknownRole).go test ./internal/roles/ -run TestIsValidCustomRoleName -vpasses.How to use
Summary by CodeRabbit
New Features
--customflag to theadd-agentcommand, enabling users to add custom roles outside the predefined catalog. Custom role names must follow a naming pattern: start with a lowercase letter and contain only lowercase letters, digits, or hyphens (maximum 32 characters).