Move duplicate trait attributes to resource.

[ggreer]

Add WithResourceProfile, WithResourceIcon, and WithResourceStatus to ResourceOption.

Deprecate old trait options, but have them still work by syncing the trait attributes to the resource.

Limit NHI details to 1024 bytes.

Protofmt.

[github-actions]

🟠 Bug: Deprecating these trait proto fields (profile/icon/status/created_at) makes staticcheck SA1019 fire on every non-test caller that reads or writes them. This SA1019 exclusion only covers pkg/types/resource/*_test.go, but there are unguarded production callers — pkg/c1zsanitize/handlers.go (reads via getters and writes via UserTrait_builder{Profile,Icon}/SetStatus/SetCreatedAt), pkg/sync/syncer.go:1382,1391,1400,2361,2374, and pkg/baton/explorer/baton_service.go:23-24. make lint (staticcheck is enabled) will fail. Either broaden this exclusion / add //nolint:staticcheck at those sites, or migrate those callers to the resource-level fields. (confidence: high)

[github-actions]

🟡 Suggestion: This adds a new min_bytes:1/max_bytes:1024 constraint to nhi_detail (a validation tightening) that's unrelated to the PR's stated purpose of moving trait attributes to the resource. ignore_empty keeps empty values valid, but any connector currently emitting an nhi_detail longer than 1024 bytes would newly fail validation. Confirm this tightening is intended and safe for existing callers. (confidence: low)

[github-actions]

🟡 Suggestion: This tightens nhi_detail validation (new max_bytes: 1024) where none existed before. It's intentional per the PR description, but any existing connector emitting a longer detail will now fail message validation at sync time — worth confirming no downstream connector relies on longer values before this ships. (confidence: medium)

[github-actions]

General PR Review: Move duplicate trait attributes to resource.

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 84d755cd854f.
Review mode: full
View review run

Review Summary

Full PR diff scanned for security and correctness. This PR adds resource-level profile/icon/status/created_at fields (proto field numbers 9-12, plus a new Status message), deprecates the equivalent trait-level fields, and keeps the deprecated WithXTrait options working by mirroring their values onto the resource. The proto changes are additive and wire-safe (no renumbered/reused/removed active fields), generated pb/*.pb.go output matches the proto sources, and the deprecated-field fallback getters in resource_attrs.go are covered by new tests. The enum-identity assumptions (UserTrait_Status, AgentTrait_AgentStatus, Status_ResourceStatus all 0-3) hold, and no unused-variable or shadowing regressions were introduced in cmd/baton/csv.go, cmd/baton/xlsx.go, or pkg/sync/syncer.go. No new blocking issues found.

Two items from prior reviews still apply and are not re-flagged inline: proto/c1/connector/v2/annotation_trait.proto nhi_detail now caps at max_bytes: 1024 (can reject previously-valid longer values at sync time), and pkg/sdk/version.go remains v0.16.0 despite the deprecations/validation tightening — consider a minor bump as the pre-1.0 compatibility signal.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• .golangci.yml:105-111 — the new blanket SA1019 exclusion suppresses all staticcheck deprecation warnings in every _test.go; consider narrowing the text matcher to the specific deprecated symbols.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In .golangci.yml:
- Around line 105-111: The added exclusion rule uses a text matcher of SA1019 scoped to path _test.go, which suppresses ALL staticcheck deprecation warnings in every test file, not just the intended trait-level profile/icon/status/created_at fields. Narrow it by matching the specific deprecated symbols in the text regex (mirroring the existing dotc1z.NewC1ZFile rule just above) so unrelated future deprecations in tests are not silently masked.

[github-actions]

No blocking issues found.

[github-actions]

No blocking issues found.

[github-actions]

🟡 Suggestion: The zero value RESOURCE_STATUS_STATUS_UNSPECIFIED has a doubled STATUS_STATUS segment; every other zero value in this codebase follows <PREFIX>_UNSPECIFIED (e.g. AGENT_STATUS_UNSPECIFIED). Since this is a brand-new wire enum, the name is free to fix now but permanent once shipped — consider RESOURCE_STATUS_UNSPECIFIED. (medium confidence)

[github-actions]

🟡 Suggestion: This blanket SA1019 exclusion suppresses all staticcheck deprecation warnings across every _test.go file, not just the trait-level profile/icon/status/created_at fields it's intended for. It could mask unrelated future deprecations in tests. Consider narrowing the text: to the specific deprecated symbols (as the existing dotc1z.NewC1ZFile rule above does) to keep coverage tight. (confidence: medium, non-blocking)

[github-actions]

No blocking issues found.