feat: add Letta Code detection and restore - #3107
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (21)
🚧 Files skipped from review as they are similar to previous changes (6)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughHerdr adds Letta Code detection, session hooks, integration installation, native conversation restore, configuration entries, API and CLI support, localized documentation, and changelog entries. ChangesLetta Code support
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔵 Low · up to The PR adds Letta session detection, conversation restore, and a user-level SessionStart hook. It is mergeable with explicit owner awareness because status may remain stale after approval, session identity reporting may fail if hook settings are rejected, and interrupted or concurrent setup changes could leave configuration inconsistent. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 36.07% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 61 functions across 17 files. (14 skipped: 14 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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. Comment |
kangal-bot
added
the
ai-review
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/config/sound.rs (1)
47-47: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd an explicit regression assertion for omitted
letta.
#[serde(default)]already preservesAgentSoundOverrides::default(). Add an assertion that an absentlettafield producesAgentSoundSetting::Default.src/detect/mod.rs (1)
629-640: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueThe bare
letta-code/lettapath match is broad.The
windows(2)check matches any path that contains aletta-codedirectory followed by alettacomponent, outsidenode_modules. A local checkout path such as/home/user/src/letta-code/letta/build.jswould be classified as Letta. The scopedwindows(4)match above already covers the published package layout. Theis_interactive_letta_processgate limits the damage, so this is a small risk only.Consider anchoring the fallback to a
node_modulesor.bincontext, or to a trailing component.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: aa59483c-24ca-4cc4-8592-87c045048df4
📒 Files selected for processing (39)
docs/next/CHANGELOG.mddocs/next/api/herdr-api.schema.jsondocs/next/website/src/content/docs/agent-automation.mdxdocs/next/website/src/content/docs/agents.mdxdocs/next/website/src/content/docs/cli-reference.mdxdocs/next/website/src/content/docs/integrations.mdxdocs/next/website/src/content/docs/ja/agent-automation.mdxdocs/next/website/src/content/docs/ja/agents.mdxdocs/next/website/src/content/docs/ja/cli-reference.mdxdocs/next/website/src/content/docs/ja/integrations.mdxdocs/next/website/src/content/docs/ja/session-state.mdxdocs/next/website/src/content/docs/session-state.mdxdocs/next/website/src/content/docs/zh-cn/agent-automation.mdxdocs/next/website/src/content/docs/zh-cn/agents.mdxdocs/next/website/src/content/docs/zh-cn/cli-reference.mdxdocs/next/website/src/content/docs/zh-cn/integrations.mdxdocs/next/website/src/content/docs/zh-cn/session-state.mdxdocs/next/website/src/data/config-reference.jsonsrc/agent_resume.rssrc/api/schema/integrations.rssrc/cli/integration.rssrc/config/model.rssrc/config/sidebar.rssrc/config/sound.rssrc/detect/manifest.rssrc/detect/manifests/letta.tomlsrc/detect/mod.rssrc/integration/actions.rssrc/integration/assets/letta/herdr-agent-session.ps1src/integration/assets/letta/herdr-agent-session.shsrc/integration/env.rssrc/integration/mod.rssrc/integration/registry.rssrc/integration/targets.rssrc/integration/tests.rssrc/integration/types.rssrc/main.rswebsite/agent-detection/index.tomlwebsite/agent-detection/letta.toml
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
04c821e to
cfeed00
Compare
Greptile SummaryThis PR adds Letta Code detection, managed startup, session reporting and native conversation restoration, plus installation, configuration, API, and documentation support.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains within the eligible follow-up-review scope. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/detect/mod.rs | Adds the Letta agent identity, aliases, managed executable mapping, and interactive-process filtering. |
| src/detect/manifests/letta.toml | Defines prioritized OSC and screen rules for Letta idle, working, blocked, and unknown states. |
| src/agent_resume.rs | Converts ordinary and default Letta session references into native conversation restore commands. |
| src/integration/targets.rs | Adds Letta hook installation, settings registration, staged publication, rollback, and uninstallation. |
| src/integration/assets/letta/herdr-agent-session.sh | Reports Unix Letta SessionStart conversation identity to the owning Herdr pane. |
| src/integration/assets/letta/herdr-agent-session.ps1 | Implements equivalent session reporting for Windows. |
| src/integration/registry.rs | Registers Letta command availability, platform support, installed asset path, and integration version. |
| src/api/schema/integrations.rs | Adds Letta to the typed integration target API contract. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
P[Letta foreground process] --> D[Process detection]
D --> M[OSC and screen manifest]
M --> S[Agent lifecycle state]
H[Optional SessionStart hook] --> R[Native session reference]
R --> X[Saved session snapshot]
X --> C[Letta resume command]
Reviews (6): Last reviewed commit: "feat: add Letta Code detection and resto..." | Re-trigger Greptile
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/config/sound.rs (1)
226-226: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the Letta-specific lookup with a non-default test.
The current assertion verifies only the default value. It does not verify that a configured value reaches
AgentSoundOverrides::for_agentforAgent::Letta. Addletta = "on"to the fixture and assert both the parsed field and the lookup result.Suggested test extension
[ui.sound.agents] droid = "off" claude = "on" +letta = "on" ... - assert_eq!(config.ui.sound.agents.letta, AgentSoundSetting::Default); + assert_eq!(config.ui.sound.agents.letta, AgentSoundSetting::On); + assert_eq!( + config + .ui + .sound + .agents + .for_agent(Some(crate::detect::Agent::Letta)), + AgentSoundSetting::On + );src/detect/mod.rs (1)
692-735: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider normalizing the
--flag=valueform once.The list repeats each option twice: once as an exact match and once as a
--name=prefix. Splitting the argument at the first=and matching the name once removes the duplication and keeps both forms in sync when the option list changes.♻️ Possible simplification
- if cli_args.iter().any(|arg| { - matches!( - arg.as_str(), + if cli_args.iter().any(|arg| { + let name = arg.split_once('=').map_or(arg.as_str(), |(name, _)| name); + matches!( + name, "-p" | "--print"Then drop the
starts_with("--…=")chain.src/integration/targets.rs (1)
1209-1211: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse a single
letta_dir()result.
letta_dir()is called twice. Bind it once and derive both paths from it.♻️ Proposed change
- let hook_path = letta_dir()?.join("hooks").join(LETTA_HOOK_INSTALL_NAME); - let settings_path = letta_dir()?.join("settings.json"); + let dir = letta_dir()?; + let hook_path = dir.join("hooks").join(LETTA_HOOK_INSTALL_NAME); + let settings_path = dir.join("settings.json");
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a6b68c80-a89f-46f3-907b-3c80fcdc37c8
📒 Files selected for processing (6)
docs/next/website/src/data/config-reference.jsonsrc/config/sound.rssrc/detect/mod.rssrc/integration/assets/letta/herdr-agent-session.ps1src/integration/targets.rssrc/integration/tests.rs
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
0e6f70e to
6c0ff7b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/integration/targets.rs (1)
1011-1018: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse a named millisecond timeout constant for the Letta hook. Letta Code v0.30.28 supports both
timeoutin milliseconds andquietas a boolean. Replace the unexplained10_000literal withLETTA_HOOK_TIMEOUT_MS.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0a396887-ed40-43ca-91ce-80b4fa6492ca
📒 Files selected for processing (3)
src/config/sound.rssrc/detect/mod.rssrc/integration/targets.rs
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.
6c0ff7b to
e03e033
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 25b3509d-f279-4a69-94a2-22d181088986
📒 Files selected for processing (3)
src/integration/mod.rssrc/integration/targets.rssrc/integration/tests.rs
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
e03e033 to
f96c294
Compare
|
hey @just-cameron could you verify if this branch works great with letta? |
|
Yeah, taking a look now. Thanks! |
|
Okay -- battletested and found one blocking bug. You can see my agent's fix in this commit. I have included my agent's report for your perusal. As a side note, one of our users added a lot of bells and whistles to improve the herdr/letta experience, but I think it is out of scope for this PR. Would you like me to open issues for some of the features I liked in his work? https://github.com/klittle32/letta-herdr-mod Agent reportTested this thoroughly against exact PR head What worked:
The 11 Letta-focused tests pass, as do strict Clippy, manifest validation, and the documentation builds. I found one startup edge case:
The PR currently conflicts with master. I rebased it locally onto One note on validation: the full Overall, the integration works well in real use. The false-ready profile-selector case is the only Letta-specific issue I found. |
refs #3106 Co-authored-by: Cameron <cameron@pfiffer.org>
f96c294 to
daf9d95
Compare
|
Agreed—the known-agent idle fallback could make startup succeed before Letta showed live composer evidence. I applied your manifest fix and regression coverage, then rebased the PR onto current master. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@just-cameron could you re-review please and verify if it works with letta? |
|
FWIW, tested |
4 participants
Summary
Builds on the initial integration work from @just-cameron in https://github.com/just-cameron/herdr/tree/letta-code-integration.
Validation
just checkRefs #3106