fix: use PATCH /v1/sdk/agents/{id}/identity for DID registration#34
fix: use PATCH /v1/sdk/agents/{id}/identity for DID registration#34
Conversation
- Change HTTP method from PUT to PATCH for identity registration
- Update endpoint path from /v1/sdk/agents/{id} to /v1/sdk/agents/{id}/identity
- Update tests to match new endpoint
This change aligns with the server's new dedicated identity endpoint that
enforces RFC-003 §9.5 immutability rules.
Requires: capiscio/capiscio-server#29
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Updates the SimpleGuard Init (internal RPC) DID registration call to use the new server identity endpoint, preventing agent record overwrites during registration.
Changes:
- Switch DID registration request from
PUT /v1/sdk/agents/{id}toPATCH /v1/sdk/agents/{id}/identity. - Treat HTTP
409 Conflictfrom the identity endpoint as non-fatal (immutability case). - Update unit tests to assert the new method and path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/rpc/simpleguard_service.go | Updates DID registration URL/method and adds special handling for HTTP 409 responses. |
| internal/rpc/simpleguard_service_test.go | Updates expectations for the DID registration request method/path. |
| // 409 Conflict means identity already exists (RFC-003 §9.5 immutability) - not an error | ||
| if resp.StatusCode == http.StatusConflict { | ||
| return nil // Identity already registered, this is expected | ||
| } |
There was a problem hiding this comment.
registerDIDWithServer now treats HTTP 409 Conflict as a non-error, but this new behavior isn't covered by unit tests. Add a test case asserting that a 409 response returns nil (and, if applicable, that callers handle it appropriately) to prevent regressions.
| // 409 Conflict means identity already exists (RFC-003 §9.5 immutability) - not an error | ||
| if resp.StatusCode == http.StatusConflict { | ||
| return nil // Identity already registered, this is expected | ||
| } |
There was a problem hiding this comment.
By returning nil on HTTP 409, callers cannot distinguish between “identity updated” and “identity already existed / update rejected”. In Init, this currently results in registered = true even when the server may not have accepted the new DID. Consider returning an explicit status (e.g., (updated bool, err error) or a sentinel error) so InitResponse.registered can accurately reflect what happened and clients can surface a clear message to the user.
| // Uses PATCH /v1/sdk/agents/{id}/identity to update only the agent's DID (RFC-003). | ||
| func (s *SimpleGuardService) registerDIDWithServer(serverURL, apiKey, agentID, didKey string) error { | ||
| // Normalize URL to prevent double-slash issues | ||
| normalizedURL := strings.TrimRight(serverURL, "/") | ||
| url := fmt.Sprintf("%s/v1/sdk/agents/%s", normalizedURL, agentID) | ||
| url := fmt.Sprintf("%s/v1/sdk/agents/%s/identity", normalizedURL, agentID) | ||
|
|
There was a problem hiding this comment.
The PR updates DID registration in SimpleGuardService, but there is still a code path in the CLI that uses the legacy PUT /v1/sdk/agents/{id} endpoint (cmd/capiscio/init.go registerDID). This looks inconsistent with the PR title/description and may leave the original overwrite/500 issue unresolved for CLI-based registration. Consider updating the CLI registration call to PATCH /v1/sdk/agents/{id}/identity as well (or narrow the PR description/title if this change is intentionally scoped to the Init RPC only).
- Change PUT /v1/sdk/agents/{id} to PATCH /v1/sdk/agents/{id}/identity
- Handle 409 Conflict as success (RFC-003 §9.5 immutability)
- Add test for 409 Conflict handling
Addresses Copilot review comments on PR #34
1 participant
Summary
Updates the DID registration call to use the new dedicated identity endpoint with PATCH method.
Changes
PUTtoPATCH/v1/sdk/agents/{id}to/v1/sdk/agents/{id}/identityWhy
The server's previous
PUT /v1/sdk/agents/{id}endpoint overwrote ALL agent fields, causing:namefield to become empty string (violating NOT NULL constraint)The new dedicated endpoint only updates identity fields (DID, publicKey) and enforces RFC-003 §9.5 immutability rules.
Testing
go test ./...)Dependencies
Related PRs
PR 2/4 in the identity recovery fix series: