Repository navigation
fix(core): make file links in emails open the instance dashboard - #1136
Conversation
Agents link memory and project files with dashboard paths such as `/memory/topics/file.md`. An email has no web address to complete those paths, so the links went nowhere when opened from a mail client. Emails now add the instance's public web address to these links, in both the HTML and the plain-text version. The footer's Settings link opens the instance's own settings page too. An instance with no public web address keeps its links as they are. Fixes rome-os#1076
Jessie-QingYu
left a comment
There was a problem hiding this comment.
Code Review: ✅ APPROVE
This PR adds the instance's public web address to links that start with a single /, in both the HTML and the plain-text part of every outgoing email. It does this in EmailAdapter.sendMail, so agent markdown and app HTML both get it. The footer's Settings link now goes to <origin>/settings when an address is set. The address comes from getConfiguredInstanceOrigin(), the same function that supplies the address named in the agent prompt (index.ts) and used by Favors. It returns null for local or desktop installs, so Rome never makes up a localhost address. I checked that the web app serves its routes from the root of the address (/settings is a route in App.tsx; there is no base path). I also checked that sendMail is the only place outgoing email is built.
I read both changed files in full and ran resolveMarkdownLinks against marked@17 on about 20 edge cases. All behaved correctly: links in <…>, titles, reference-style definitions (including ones that span lines), #////web links, tables, quotes, query and fragment, links whose text also appears in inline or fenced code (left as written, as the PR says), and links written with backslash escapes (left as written). The HTML step re-parses only output that has already been through DOMPurify or marked, and it returns the input unchanged when it has no matching links. The /settings address is escaped, though URL.origin is already safe. The new tests cover both email versions, the case with no address, app HTML, and the adapter reading the environment variable. I found no correctness or security problems. My one note is a P3 about the footer showing the instance address to every recipient.
Verdict: APPROVE — The fix is in the right place, reuses the existing source of truth for the instance's web address, holds up against the edge cases I tested, and comes with good tests; I only have one optional note.
1 finding(s) posted as inline comments below.
| Severity | Category | File | Title |
|---|---|---|---|
| P3 | design | packages/core/src/lib/email-markdown.ts |
Footer Settings link shows the instance address to every recipient |
| // row; appended to app-supplied HTML it rides in `standaloneFooter` below. | ||
| function footerContent(): string { | ||
| function footerContent(instanceOrigin: string | null): string { | ||
| const settingsUrl = instanceOrigin ? `${instanceOrigin}/settings` : ROME_SETTINGS_URL; |
There was a problem hiding this comment.
[P3] design — Footer Settings link shows the instance address to every recipient
Every recipient now sees PANTHEON_INSTANCE_ORIGIN, including third parties the agent writes to, not just the guardian. That value can be a custom domain or a dev tunnel host, not just <slug>.romeos.cc. A non-guardian recipient also can't sign in, so for them the Settings link only reaches a sign-in wall. The PR description accepts this trade-off. If it matters, keep romeos.cc in the footer for recipients other than the guardian, or note the exposure where PANTHEON_INSTANCE_ORIGIN is documented.
There was a problem hiding this comment.
I am keeping the footer link as it is.
- The issue asks for this link to open the owner's own Rome instead of
romeos.cc. - This web address is already meant to be shared. Rome tells its agent to use it in links it sends out.
- Showing a different link to different people would mean tracking who each reply goes to. That is a bigger change than this fix.
- Anyone else who clicks it only sees a sign-in page.
I also fixed one sentence in the description about the sender address, which I had not checked.
yunfanye
left a comment
There was a problem hiding this comment.
Code Review: ✅ APPROVE
Reviewed all changed files and traced callers, origin resolution, and dashboard routes. The implementation preserves other channels, authentication, and the no-origin fallback.
All 23 email-markdown tests passed in an isolated harness with matching direct dependency versions. Adapter tests and the full repository suite were not run.
Verdict: APPROVE — The fix uses the appropriate email-rendering boundary and introduces no actionable correctness or security issues.
No issues found. Code looks good!
What this PR does
When an agent answers by email with a link to a memory or project file, the link is a dashboard path such as
/memory/topics/file.md. That works inside the dashboard, but an email has no web address to complete it. From a mail client, the link opens the mail service's own site or a missing local file. The footer's Settings link also went toromeos.cc, not to the instance.Outgoing emails now add the instance's public web address to these links, in both the HTML and the plain-text version. A reply with
[file.md](</memory/topics/file.md>)arrives ashttps://<instance>/memory/topics/file.md. Settings opens the instance's own settings page. HTML emails that apps write get the same fix.Fixes #1076
Design & Invariants
/change. Web links,//other-hostlinks and#sectionlinks stay as written, and so does the rest of each path, query and fragment.romeos.ccSettings link as they are. Rome never makes up alocalhostaddress.Test plan
email-markdown.test.tsandemail.test.ts. Memory, project and/desktoplinks get the instance address in both versions, including a link with a title and a reference-style link. Web links, other hosts,#links and code samples stay as written. Without an address, nothing changes. App HTML, the text made from it, and the agent's own text get the fix too. The adapter test reads the address fromPANTHEON_INSTANCE_ORIGIN, as production does. All 5 new tests fail without the fix.pnpm typecheck, and Biome on the changed files.pnpm test:unit:core: 6048 passed, 2 failed. Neither one touches email. The WeChat test fails on macOS onmaintoo, andpack.test.tspasses when run alone.file:///memory/...(not found). After the fix, both went tohttps://acme.romeos.cc/memory/topics/meeting-notes.md./memory/topics/health.mdas a fresh page in the mock dashboard shows that file, so the new links land on the right file.PANTHEON_INSTANCE_ORIGINset. It reachedRome startedwith all 13 first-party apps and no failed actions, and/api/healthansweredok. This runssrc/index.tsdirectly, the same entry pointpnpm dev:allruns in its container.Not in this PR