feat: add PENDING status for invitation-style principals - #1104
Conversation
Add RESOURCE_STATUS_PENDING and STATUS_PENDING as value 4 on the resource and user-trait status enums so connectors can express a principal whose account creation was initiated but is not yet usable, such as an unaccepted invitation. Mirror the value onto the v3 StatusRecord enum so pebble translation stays in lockstep. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| RESOURCE_STATUS_ENABLED = 1; | ||
| RESOURCE_STATUS_DISABLED = 2; | ||
| RESOURCE_STATUS_DELETED = 3; | ||
| RESOURCE_STATUS_PENDING = 4; |
There was a problem hiding this comment.
🟡 Suggestion (medium confidence): the v3 mirror is maintained by hand — the comment above StatusRecord says "changes to v2 require an explicit mirror update here (no automatic propagation)" — and translate_v2.go casts numerically between v2.Status_ResourceStatus and v3.StatusRecord_ResourceStatus. The new test only proves PENDING specifically round-trips; the next enum value added to one side and not the other silently mistranslates into durable c1z data. Consider a value-agnostic invariant test that asserts v2.Status_ResourceStatus_name, v2.UserTrait_Status_Status_name, and v3.StatusRecord_ResourceStatus_name agree on every shared number, so drift fails CI instead of shipping.
There was a problem hiding this comment.
Addressed in 8a66dfb: added TestStatusEnumMirrorsStayAligned (pkg/dotc1z/engine/pebble/translate_v2_test.go), which asserts all three name maps agree on every value — set-equality between v2 Status.ResourceStatus, v3 StatusRecord.ResourceStatus, and (prefix-normalized) UserTrait.Status — so the next one-sided enum addition fails CI rather than mistranslating stored data.
| RESOURCE_STATUS_DELETED = 3; | ||
| // Account creation was initiated but the account is not yet usable, such | ||
| // as an invitation that has not been accepted. | ||
| RESOURCE_STATUS_PENDING = 4; |
There was a problem hiding this comment.
🟡 Suggestion (high confidence): ResourceStatus and AgentTrait.AgentStatus are no longer value-identical after this — AgentStatus stops at AGENT_STATUS_DELETED = 3. Two comments now overstate the invariant: pkg/types/resource/resource.go:174-175 and pkg/types/resource/resource_attrs.go:90-91 both say "AgentTrait_AgentStatus and Status_ResourceStatus enum values are identical" right above a numeric cast. The cast direction used is still safe, but if anyone later adds AGENT_STATUS_* = 4 it will silently surface as RESOURCE_STATUS_PENDING. Worth reworking those comments to state the actual contract (AgentStatus is a prefix of ResourceStatus; new AgentStatus values must not collide) and adding a note here.
There was a problem hiding this comment.
Addressed in 8a66dfb: reworded both comments (resource.go, resource_attrs.go) to state the actual contract — AgentTrait_AgentStatus is a numeric prefix of Status_ResourceStatus, and new AgentStatus values must not reuse ResourceStatus numbers with different meanings. The new alignment test also asserts every AgentStatus number exists in ResourceStatus, so an AGENT_STATUS_* = 4 addition would fail CI and force the collision decision explicitly.
General PR Review: feat: add PENDING status for invitation-style principalsBlocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0 Review SummaryThe full PR diff was re-scanned for security and correctness; it remains an additive proto3 enum value Risk triage (per Security IssuesNone found. Correctness IssuesNone found. Suggestions
Prompt for AI agents |
…ents The v2 ResourceStatus, v2 UserTrait status, and v3 StatusRecord status enums are hand-mirrored and cast numerically; assert every shared value agrees by name so drift fails CI instead of mistranslating stored data. AgentTrait_AgentStatus is a numeric prefix of ResourceStatus, not a mirror - reword the two comments that overstated it as identical and assert the subset property. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| // AgentTrait_AgentStatus is cast numerically into Status_ResourceStatus. | ||
| // It is a prefix, not a mirror (READY maps to ENABLED); every AgentStatus | ||
| // number must exist in ResourceStatus so a new AgentStatus value cannot | ||
| // silently surface as an unrelated resource status. | ||
| for num, name := range v2.AgentTrait_AgentStatus_name { | ||
| _, ok := v2.Status_ResourceStatus_name[num] | ||
| require.Truef(t, ok, "AgentTrait_AgentStatus value %d (%s) has no Status_ResourceStatus counterpart", num, name) | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: this guard doesn't catch the failure mode its comment describes. AgentStatus currently stops at 3, and ResourceStatus now goes to 4 — so the next AgentStatus value added (4) will find a counterpart in Status_ResourceStatus_name and pass, while syncAgentTraitToResource's numeric cast silently renders it as RESOURCE_STATUS_PENDING. Existence-only checking is exactly what the added PENDING value made insufficient. Pinning the mapping by name, so a new AgentStatus value forces the table to be extended deliberately, would close it:
| // AgentTrait_AgentStatus is cast numerically into Status_ResourceStatus. | |
| // It is a prefix, not a mirror (READY maps to ENABLED); every AgentStatus | |
| // number must exist in ResourceStatus so a new AgentStatus value cannot | |
| // silently surface as an unrelated resource status. | |
| for num, name := range v2.AgentTrait_AgentStatus_name { | |
| _, ok := v2.Status_ResourceStatus_name[num] | |
| require.Truef(t, ok, "AgentTrait_AgentStatus value %d (%s) has no Status_ResourceStatus counterpart", num, name) | |
| } | |
| // AgentTrait_AgentStatus is cast numerically into Status_ResourceStatus. | |
| // It is a prefix, not a mirror (READY maps to ENABLED). Pin the mapping by | |
| // name so a new AgentStatus value cannot silently inherit an unrelated | |
| // ResourceStatus meaning — extend this table deliberately when adding one. | |
| expectedAgentMirror := map[int32]string{ | |
| 0: "RESOURCE_STATUS_UNSPECIFIED", | |
| 1: "RESOURCE_STATUS_ENABLED", | |
| 2: "RESOURCE_STATUS_DISABLED", | |
| 3: "RESOURCE_STATUS_DELETED", | |
| } | |
| require.Len(t, v2.AgentTrait_AgentStatus_name, len(expectedAgentMirror), | |
| "new AgentTrait_AgentStatus value: confirm its numeric cast into Status_ResourceStatus is still meaningful, then extend expectedAgentMirror") | |
| for num, name := range v2.AgentTrait_AgentStatus_name { | |
| want, ok := expectedAgentMirror[num] | |
| require.Truef(t, ok, "AgentTrait_AgentStatus value %d (%s) has no reviewed Status_ResourceStatus counterpart", num, name) | |
| require.Equalf(t, want, v2.Status_ResourceStatus_name[num], | |
| "AgentTrait_AgentStatus value %d (%s) casts to an unexpected Status_ResourceStatus", num, name) | |
| } |
1 participant
Problem
Connectors have no way to say "this principal is a pending invitation, not a usable account." Both status enums stop at
DELETED = 3, so an invitation-style principal can only be expressed asSTATUS_UNSPECIFIED— and even that requires the deprecatedWithStatusoption, becauseNewUserTraitforce-defaults an unset status toENABLED. Consumers are left inferring "pending" from an absent value, which is indistinguishable from a connector that simply never set a status.Change
Additive enum value
4on both status enums:c1.connector.v2.Status.ResourceStatus→RESOURCE_STATUS_PENDING = 4c1.connector.v2.UserTrait.Status.Status→STATUS_PENDING = 4c1.storage.v3.StatusRecord.ResourceStatusis mirrored as well — that enum documents itself as an explicit mirror of the v2 one, and the pebble translation layer casts between them numerically, so it has to move in lockstep.Trait ↔ resource status mirroring (
GetStatus,syncUserTraitToResource,WithResourceStatus/WithDetailedStatus/WithStatus) already round-trips by numeric cast, soPENDINGflows through unchanged with no new branches. The one enumerating switch,getUserStatusin the CSV exporter, gains aPendingcase.NewUserTrait's enabled-by-default behavior for an unset status is unchanged; an explicitly-setPENDINGis not overwritten, and there's a test proving it.Wire compatibility
Proto3 enums are open, so this is additive and existing values are untouched — nothing is renumbered or reused. The one caveat: the status fields carry
(validate.rules).enum = {defined_only: true}, which is enforced against the generated_namemap. A consumer running an older generated.pb.validate.gowill reject value4until it regenerates. Consumers should upgrade before connectors start emittingPENDING.Testing
go test -tags=baton_lambda_support ./...andgolangci-lint runboth clean. Protos regenerated withbuf generate(no hand edits);buf lintandbuf breaking --against origin/mainpass.Refs IGA-1212.
🤖 Generated with Claude Code