feat(provider/terminal): add richlink content type support#42
feat(provider/terminal): add richlink content type support#42Zane Wang (zelinewang) wants to merge 2 commits into
Conversation
Encode + decode for `richlink` in the terminal provider — mirrors the existing `attachment`/`voice`/`contact` lazy-accessor wrapping pattern. Decoder normalizes empty strings to undefined and guards missing cover bytes so peer-side malformed shapes don't defer ZodErrors or crash Buffer.from. Closes the UnsupportedError gap that surfaced after photon-hq#29 introduced richlink as a first-class content type.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds bidirectional richlink support to the terminal provider: outbound converts Spectrum Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/spectrum-ts/src/providers/terminal/index.ts`:
- Around line 608-614: richlinkSchema.parse can throw on malformed input and
halt the event stream; replace the direct parse with a non-throwing path (either
richlinkSchema.safeParse(...) or wrap richlinkSchema.parse(...) in a try/catch)
inside the function returning SpectrumContent and, on validation failure, return
a soft-fail fallback object with type: "custom" (preserving the original
url/title/summary/cover accessors or raw payload as appropriate) so bad richlink
payloads don't break processing; update the return site that currently
references richlinkSchema.parse(...) and ensure the fallback still satisfies
SpectrumContent shape.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d1f876d6-2eaa-4ee7-ba0e-07cf3a2eaf58
📒 Files selected for processing (2)
packages/spectrum-ts/src/providers/terminal/index.tspackages/spectrum-ts/src/providers/terminal/protocol.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity in TypeScript
Preferunknownoveranywhen the type is genuinely unknown in TypeScript
Use const assertions (as const) for immutable values and literal types in TypeScript
Leverage TypeScript's type narrowing instead of type assertions
Files:
packages/spectrum-ts/src/providers/terminal/protocol.tspackages/spectrum-ts/src/providers/terminal/index.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions in JavaScript/TypeScript
Preferfor...ofloops over.forEach()and indexedforloops in JavaScript/TypeScript
Use optional chaining (?.) and nullish coalescing (??) for safer property access in JavaScript/TypeScript
Prefer template literals over string concatenation in JavaScript/TypeScript
Use destructuring for object and array assignments in JavaScript/TypeScript
Useconstby default,letonly when reassignment is needed, nevervarin JavaScript/TypeScript
Alwaysawaitpromises in async functions - don't forget to use the return value in JavaScript/TypeScript
Useasync/awaitsyntax instead of promise chains for better readability in JavaScript/TypeScript
Handle errors appropriately in async code with try-catch blocks in JavaScript/TypeScript
Don't use async functions as Promise executors in JavaScript/TypeScript
Removeconsole.log,debugger, andalertstatements from production code in JavaScript/TypeScript
ThrowErrorobjects with descriptive messages, not strings or other values in JavaScript/TypeScript
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them in JavaScript/TypeScript
Prefer early returns over nested conditionals for error cases in JavaScript/TypeScript
Keep functions focused and under reasonable cognitive complexity limits
Extract complex conditions into well-named boolean variables in JavaScript/TypeScript
Use early returns to reduce nesting in JavaScript/TypeScript
Prefer simple conditionals over nested ternary operators in JavaScript/TypeScript
Group related code together and separate concerns in JavaScript/TypeScript
Don't useeval()or assign directly todocument.cookiein JavaScript/TypeScript
Validate and sanitize user input in JavaScript/TypeScript
Avoid spread syntax in accumulators within loops in JavaScript/Ty...
Files:
packages/spectrum-ts/src/providers/terminal/protocol.tspackages/spectrum-ts/src/providers/terminal/index.ts
🔇 Additional comments (2)
packages/spectrum-ts/src/providers/terminal/protocol.ts (1)
38-44: Richlink protocol variant is well-scoped and consistent.The new discriminated union member cleanly matches the provider conversion contract.
packages/spectrum-ts/src/providers/terminal/index.ts (1)
453-479: Richlink outbound serialization looks solid.Eager accessor resolution and empty-cover elision are implemented correctly for the wire format.
Wrap `richlinkSchema.parse` in try/catch and let the catch fall through
to the existing unknown-shape fallback (`{ type: "custom", raw: p }`)
at the function tail. A non-conforming peer sending an invalid `url`
(or omitting it entirely) would otherwise raise a `ZodError` from
`protocolToSpectrum`, halting the inbound message stream.
Mirrors the contact-decode try/catch at lines 526-532, which already
swallows malformed-vCard zod errors for the same reason.
Addresses CodeRabbit review feedback on photon-hq#42.
|
Zane Wang (@zelinewang) does this require any changes in tuichat? if doesn't, how can the tuichat render the richlink |
|
Ryan Zhu (@underthestars-zhy) From what I could test against tuichat v0.1.4, the wire round-trips fine. For rendering, yes i opened tuichat#1 for a separate followup on rendering. This PR only adds the terminal adapter side, tested with tuichat v0.1.4, the |
|
Zane Wang (@zelinewang) do u mind also submitting a pr to tuichat so it does support the rendering |
|
Opened draft PR photon-hq/tuichat#2 for the rendering side. Kept it text-only first and left Pattern A vs Raw passthrough / cover rendering as explicit design questions in the PR body. |
|
can we also render the image or have a little button that we hover and display and image? like what we have with the current attachmemt |
|
Yes. I updated photon-hq/tuichat#2 in commit Verified with |
Summary
Adds
richlinkcontent type support to theterminalprovider — encode inspectrumToProtocol, decode inprotocolToSpectrum. Closes theUnsupportedErrorgap that surfaced after #29 introducedrichlinkas a first-class content type.Background
#29 added
richlinkto spectrum-ts as a content type with lazy async accessors fortitle/summary/cover. None of the existing providers (iMessage, WhatsApp Business, terminal) handle it yet — they all throwUnsupportedError. This PR closes the gap forterminal.Usage
Changes
packages/spectrum-ts/src/providers/terminal/protocol.tsrichlinkvariant toProtocolContent({ type: "richlink"; url; title?; summary?; cover?: { mimeType?; bytes? } }).cover.bytes?mirrors theattachment/voicebytes?precedent.packages/spectrum-ts/src/providers/terminal/index.tsattachment/voice/contactlazy-accessor wrapping pattern. ImportsrichlinkSchema+RichlinkCover+bufferToStream. Updates theUnsupportedErrordoc-comment example toeffect(sincerichlinkis now supported here).Design notes
Encode (
spectrumToProtocol) — Resolves the lazy accessors eagerly (Promise.all([title(), summary(), cover()])), inlines cover bytes as base64. An empty cover buffer (e.g.buildCover's catch-all returnsBuffer.alloc(0)after a failed image fetch) is dropped to save wire space.Decode (
protocolToSpectrum) — Wraps the static wire values back into the lazy-accessor shaperichlinkSchemarequires. The accessors are trivial closures — no network call on the receiver, mirroring howattachmentdecode keepsread()lazy.Wire-boundary normalization — three runtime safety guards at the decode boundary:
title/summary: normalize""→undefined. The protocol type permits""(JSON has no min-length) butrichlinkSchemaenforcesz.string().min(1).optional()and Zod v4 validates the accessor return value at call time. Without normalization, a peer sending an empty string would defer aZodErrorinto the agent'sawait decoded.title().cover.mimeType: same""→undefined(same Zod constraint onrichlinkCoverSchema).cover.bytes: optional in the protocol type. Decoder guardscoverWire?.bytestruthy before constructing the cover; otherwise treats as no-cover (avoidsBuffer.from(undefined, "base64")crashing on a malformedcover: { mimeType: "..." }payload).Verification
Local environment: macOS arm64, Bun 1.3.11.
bun run check(Ultracite / Biome)bun run build(tsup ESM + DTS)tuichat v0.1.4(real spawn + real LSP-framed JSON-RPC + realsendwith richlink content)SendResult{id, timestamp}, no-32099, exit code 0The wire-boundary normalization fixes were caught by a multi-engine review pass before push (Codex + Claude reviewers + cross-scorer): three independent reviewers flagged the empty-string Zod deferral pattern; one additionally flagged the missing
cover.bytesguard with aBuffer.from(undefined)reproduction. All three fixes were verified by adding regression scenarios to the contract test.Verification scripts kept locally as artifacts; not part of this PR since the project doesn't have test infrastructure yet — happy to add a
bun:testsetup as a follow-up if welcome.Limitations / follow-ups
tuichat v0.1.4(the pinned default binary) accepts the wire shape (verified empirically — no-32099) but doesn't yet renderrichlinkcontent visually.internal/protocol/types.goContentstruct has noUrl/Title/Summary/Coverfields, so unknown wire fields are silently dropped on Go'sUnmarshal. Filed as feat(rendering): tuichat does not render richlink content type tuichat#1 for tracking. The wire-level integration in this PR is forward-compatible — no spectrum-ts-side change needed when tuichat ships rendering.as SpectrumContentcast — My new decode branch ends withrichlinkSchema.parse(...) as SpectrumContent, consistent with the five existing branches inprotocolToSpectrum. The project's.claude/CLAUDE.mdrecommends "leverage TypeScript's type narrowing instead of type assertions" — happy to follow up with a separate refactor PR converting all five branches to schema-inferred types, if that direction is welcome.Test plan
bun run checkbun run buildtuichat v0.1.4Need help on this PR? Tag
@codesmithwith what you need.Summary by CodeRabbit
New Features
Bug Fixes / Resilience