Skip to content

fix(sdk): fail fast when agent messaging has no encryption #260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sanil-23 merged 2 commits into from
Jul 15, 2026
Merged

fix(sdk): fail fast when agent messaging has no encryption #260

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8db2557
fix(sdk): fail fast when agent messaging has no encryption
sanil-23 Jul 15, 2026
1561dc6
refactor(sdk): extract sendMessage encryption guard into a helper
sanil-23 Jul 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions sdk/typescript/src/agent/messaging.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,30 @@ export interface SendMessageResult {

/**
* Sends a message to `recipient` (a @handle, cryptoId, or base64 key). The body
* is Signal-encrypted by the client before it leaves the process when encryption
* is configured (the recommended setup); otherwise it is sent as plaintext.
* is Signal-encrypted by the client before it leaves the process.
*
* This is the E2E facade: it REQUIRES the client to have encryption configured
* (`encryption: { store }` + a signer). A client without encryption would send
* the body as plaintext, which the relay rejects — a plaintext JSON body trips
* its `looksLikeJSON` guard with `HTTP 400: body must be encrypted ciphertext`,
* surfacing far from the misconfiguration (a client built without a signer/store,
* e.g. an under-provisioned daemon). Fail fast here with a clear cause instead of
* leaking plaintext and getting a cryptic relay rejection. Callers that genuinely
* want plain relay transport use `client.messages.send` directly.
*/
export async function sendMessage(
client: TinyPlaceClient,
signer: AgentSigner,
recipient: string,
text: string,
): Promise<SendMessageResult> {
if (!client.encryptionEnabled) {
throw new Error(
"agent messaging requires encryption: construct the client with " +
"`encryption: { store }` and a signer. Sending a plaintext body over the " +
'E2E channel is rejected by the relay ("body must be encrypted ciphertext").',
);
}

@coderabbitai coderabbitai Bot Jul 15, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Extract the guard and add async error handling.

sendMessage now exceeds the 20-line limit, and its asynchronous directory/transport operations still lack a try/catch. Extract the encryption assertion or envelope construction into helpers, then catch failures while preserving the original error/cause.

As per coding guidelines, async functions must use try-catch blocks and functions are limited to 20 lines.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sdk/typescript/src/agent/messaging.ts` around lines 104 - 116, The
sendMessage function currently exceeds the 20-line limit and lacks asynchronous
error handling. Extract the encryption guard or envelope construction into a
focused helper, then wrap sendMessage’s directory and transport operations in
try/catch while preserving and propagating the original error and cause.

Source: Coding guidelines

@sanil-23 sanil-23 Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed the quick win: extracted the encryption assertion into assertEncryptionEnabled() in 1561dc6, so sendMessage is back to a focused ~14-line body.

Declined the paired try/catch: no function in this module (resolveRecipientKey, publishKeys, readMessages) wraps its async directory/transport calls — the established idiom here is to let errors propagate to the caller with their original stack/cause intact. A re-throw wrapper would be inconsistent noise and could obscure the cause rather than preserve it.

@coderabbitai coderabbitai Bot Jul 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@sanil-23 Thanks for the update — extracting assertEncryptionEnabled() brings sendMessage back under the line limit while keeping the guard clearly named and documented.

Your reasoning on the try/catch is fair: since resolveRecipientKey, publishKeys, and readMessages all let async errors propagate untouched, adding a catch-and-rethrow just in sendMessage would be inconsistent with the module's existing convention and risks flattening the original stack/cause for no real benefit. I won't push further on that point.

✏️ Learnings added 
Learnt from: sanil-23
Repo: tinyhumansai/tiny.place PR: 260
File: sdk/typescript/src/agent/messaging.ts:0-0
Timestamp: 2026-07-15T15:48:16.593Z
Learning: In `sdk/typescript/src/agent/messaging.ts`, async functions (`resolveRecipientKey`, `publishKeys`, `sendMessage`, `readMessages`) intentionally do not wrap their directory/transport calls in try/catch. The established idiom in this module is to let errors from `client.directory.resolve`, `client.messages.send`, `client.messages.list`, etc. propagate to the caller with their original stack/cause intact, rather than catching and re-throwing.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

const to = await resolveRecipientKey(client, recipient);
const envelope: MessageEnvelope = {
id: messageId(),
Expand Down
12 changes: 12 additions & 0 deletions sdk/typescript/tests/agent-messaging.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,16 @@ describe("sendMessage / readMessages round-trip", () => {
// Consumed on read.
expect(await readMessages(bob.client, bob.signer)).toHaveLength(0);
});

it("refuses to send when the client has no encryption configured", async () => {
// A client built without `encryption: { store }` would relay the body as
// plaintext, which the backend rejects with `400: body must be encrypted
// ciphertext`. The facade must fail fast at the misconfiguration instead.
const signer = await LocalSigner.generate();
const plain = new TinyPlaceClient({ baseUrl: "https://relay.test", signer });
expect(plain.encryptionEnabled).toBe(false);
await expect(
sendMessage(plain, signer, signer.agentId, "hi"),
).rejects.toThrow(/requires encryption/);
});
});
Loading