Skip to content

refactor: strip speculative RPCs — session lifecycle + hooks only#4

Merged
tumberger merged 1 commit intomainfrom
fix/strip-speculative-rpcs
Apr 6, 2026
Merged

refactor: strip speculative RPCs — session lifecycle + hooks only#4
tumberger merged 1 commit intomainfrom
fix/strip-speculative-rpcs

Conversation

@tumberger
Copy link
Copy Markdown
Contributor

@tumberger tumberger commented Apr 6, 2026

Summary

Remove speculative RPCs and message types that don't need to exist:

Removed:

  • ExchangeCredential — credential exchange uses the existing OAuth token exchange endpoint (POST /oauth2/token, RFC 8693). The SDK already does this with public clients. No new RPC needed.
  • SyncPolicy — future feature. Add when we build it.
  • CredentialInjection, CredentialKind, ExchangeCredentialRequest/Response, SyncPolicyRequest/Response, PolicyTuple — all associated message types.

What remains (4 RPCs):

  • ProcessHookEvent (bidi stream) — hook telemetry
  • CreateSession — start a session
  • Heartbeat — keep alive
  • EndSession — terminate

Also clarified: auth uses the user's OIDC bearer token from kontext login, not application client_credentials.

This is a breaking change against the previous proto. No consumers are deployed yet — the CLI PR (#3) will regenerate after this merges.

🤖 Generated with Claude Code


Open with Devin

Remove ExchangeCredential, SyncPolicy, CredentialInjection, and all
credential/policy message types. These were speculative:

- Credential exchange uses the existing OAuth token exchange endpoint
  (POST /oauth2/token, RFC 8693). No new RPC needed.
- Policy sync is a future feature. Add it when we build it.

What remains: ProcessHookEvent (bidi stream), CreateSession, Heartbeat,
EndSession. Four RPCs. Session lifecycle + hook telemetry.

Also document that auth uses the user's OIDC bearer token, not
application client_credentials.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@tumberger tumberger merged commit 0b660bd into main Apr 6, 2026
0 of 2 checks passed
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 3 additional findings in Devin Review.

Open in Devin Review

Comment on lines 45 to 48
Decision decision = 1;
string reason = 2;
string event_id = 3;
// Credential to inject for this tool call (if applicable)
CredentialInjection credential = 4;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Removed field number 4 from ProcessHookEventResponse not marked as reserved, risking future wire-format conflicts

The credential field (number 4) was removed from ProcessHookEventResponse without adding a reserved 4; declaration. Per the protobuf documentation, if a future contributor reuses field number 4 for a different type or semantic, older clients/servers that still have the old schema will silently misinterpret the wire data, causing data corruption. This is especially important in this repo since it's a shared contract between the CLI and API (README.md:2-3), and buf breaking is configured to enforce backward compatibility (buf.yaml).

(Refers to lines 44-48)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant