[codex] desktop host tray#32
Conversation
… filesystem metadata Adds the Financial Document Knowledge Management System foundation across four new/updated crates: document model (DocType, XeroLink, DocumentRecord), #tag newtypes, sidecar+xattr filesystem metadata backends, blocking Xero PKCE OAuth2 client, blocking OpenAI-compatible LLM client (receipt/document OCR + transaction classification), XeroService MCP bridge, and ledgerr_xero as the 8th MCP tool. Regenerates docs and scripts from Rust contract source. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| impl Default for SettingsSchemaVersion { | ||
| fn default() -> Self { | ||
| Self::V1 | ||
| } | ||
| } |
| impl Default for SettingsSchemaVersion { | ||
| fn default() -> Self { | ||
| Self::V1 | ||
| } | ||
| } |
| store.save(&AppSettings::default()).unwrap(); | ||
|
|
||
| let mut updated = AppSettings::default(); | ||
| updated.toast_enabled = false; |
| .to_path_buf() | ||
| } | ||
|
|
||
| fn write(root: &PathBuf, rel: &str, content: &str) { |
| .to_path_buf() | ||
| } | ||
|
|
||
| fn write(root: &PathBuf, rel: &str, content: &str) { |
| request.doc_type.as_deref().map_or(true, |dt| { | ||
| format!("{:?}", r.doc_type).to_ascii_lowercase().contains(dt) | ||
| }) |
| request.doc_type.as_deref().map_or(true, |dt| { | ||
| format!("{:?}", r.doc_type).to_ascii_lowercase().contains(dt) | ||
| }) |
There was a problem hiding this comment.
Pull request overview
This PR introduces a first Windows desktop-host slice (ledgerr-host) with tray + notification + per-user settings, and also expands the MCP surface with new document-management actions plus a new ledgerr_xero tool (backed by a new ledgerr-xero crate) and an optional ledgerr-llm extraction crate.
Changes:
- Add
ledgerr-host(Windows tray runtime, PowerShell/BurntToast notifier, persistent JSON settings, minimal Slint window, tests). - Extend
ledgerr-mcpcontract + adapter to publish 8 tools (addsledgerr_xero) and add newledgerr_documentsactions (image ingest, tagging, fs metadata sync, filename normalization) with document registry persistence. - Add new supporting crates/modules:
ledgerr-xero,ledgerr-llm, plusledger-coredocument/tags/fs-metadata primitives; update generated docs and MCP tests accordingly.
Reviewed changes
Copilot reviewed 50 out of 51 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
| tax-ledger.xlsx.ledgerr-state.json | Adds a sample persisted state sidecar file. |
| docs/mcp-capability-contract.md | Updates generated MCP capability catalog (documents tool actions + adds Xero tool row). |
| docs/agent-mcp-runbook.md | Updates generated runbook tool list to include ledgerr_xero. |
| crates/ledgerr-xero/src/types.rs | Adds serde models for Xero API entities and response envelopes. |
| crates/ledgerr-xero/src/lib.rs | Exposes ledgerr-xero public modules and re-exports. |
| crates/ledgerr-xero/src/error.rs | Defines XeroError / XeroResult. |
| crates/ledgerr-xero/src/client.rs | Implements blocking Xero HTTP client with token refresh. |
| crates/ledgerr-xero/src/auth.rs | Implements blocking OAuth2 PKCE auth + token persistence. |
| crates/ledgerr-xero/Cargo.toml | Introduces ledgerr-xero crate dependencies/features. |
| crates/ledgerr-mcp/tests/mcp_stdio_e2e.rs | Updates tool-catalog assertions from 7 → 8. |
| crates/ledgerr-mcp/tests/mcp_adapter_contract.rs | Updates tool-catalog contract tests and asserts ledgerr_xero presence. |
| crates/ledgerr-mcp/src/xero_service.rs | Adds MCP-facing Xero service wrapper (feature-gated). |
| crates/ledgerr-mcp/src/ontology.rs | Adds ontology entity kinds for Xero-linked entities + workflow tags. |
| crates/ledgerr-mcp/src/mcp_adapter.rs | Wires in ledgerr_xero tool dispatch + adds new documents actions dispatch. |
| crates/ledgerr-mcp/src/lib.rs | Adds document registry persistence + new document tools + Xero/LLM wiring in TurboLedgerService. |
| crates/ledgerr-mcp/src/contract.rs | Expands published tool catalog to 8 and adds schemas/args for new documents actions + Xero tool. |
| crates/ledgerr-mcp/src/bin/regen-docs.rs | Adds helper binary to regenerate docs/scripts from the Rust contract. |
| crates/ledgerr-mcp/src/bin/ledgerr-mcp-server.rs | Routes ledgerr_xero tool calls to the adapter handler. |
| crates/ledgerr-mcp/Cargo.toml | Adds optional deps/features for xero + llm and enables them by default. |
| crates/ledgerr-llm/src/lib.rs | Adds blocking OpenAI-compatible client for vision + classification. |
| crates/ledgerr-llm/src/extract.rs | Defines extraction structs and prompt templates. |
| crates/ledgerr-llm/src/error.rs | Defines LlmError / LlmResult. |
| crates/ledgerr-llm/Cargo.toml | Introduces ledgerr-llm crate dependencies. |
| crates/ledgerr-host/tests/tray_state.rs | Adds unit tests for tray state defaults and menu labels. |
| crates/ledgerr-host/tests/settings_roundtrip.rs | Adds settings load/save roundtrip and fallback tests. |
| crates/ledgerr-host/tests/settings_atomicity.rs | Adds tests for parent-dir creation + “atomic” save behavior. |
| crates/ledgerr-host/src/tray/state.rs | Defines tray state + command model. |
| crates/ledgerr-host/src/tray/runtime.rs | Implements Windows tray runtime and command handling. |
| crates/ledgerr-host/src/tray/mod.rs | Exposes tray modules and re-exports. |
| crates/ledgerr-host/src/tray/menu.rs | Implements deterministic tray menu labels + tests. |
| crates/ledgerr-host/src/settings/store.rs | Implements JSON settings store with temp-file rename. |
| crates/ledgerr-host/src/settings/schema.rs | Defines settings schema and defaults. |
| crates/ledgerr-host/src/settings/path.rs | Provides default per-user settings path selection. |
| crates/ledgerr-host/src/settings/mod.rs | Exposes settings modules and re-exports. |
| crates/ledgerr-host/src/notify/types.rs | Defines notifier types, events, and trait. |
| crates/ledgerr-host/src/notify/powershell.rs | Adds PowerShell/BurntToast notifier implementation + tests. |
| crates/ledgerr-host/src/notify/mod.rs | Exposes notify modules and re-exports. |
| crates/ledgerr-host/src/lib.rs | Exposes notify, settings, and tray modules. |
| crates/ledgerr-host/src/bin/notify-test.rs | Adds CLI tool to test notifications and persist results. |
| crates/ledgerr-host/src/bin/host-window.rs | Adds minimal Windows-only Slint host window. |
| crates/ledgerr-host/src/bin/host-tray.rs | Adds Windows-only tray host binary entrypoint. |
| crates/ledgerr-host/Cargo.toml | Introduces ledgerr-host crate + Windows-only UI/tray deps. |
| crates/ledger-core/src/tags.rs | Adds validated workflow tag model + well-known tags. |
| crates/ledger-core/src/lib.rs | Exposes new document, fs_meta, and tags modules. |
| crates/ledger-core/src/fs_meta.rs | Adds filesystem sidecar (+ Linux xattr) metadata read/write/scan support. |
| crates/ledger-core/src/document.rs | Adds document model, doc types, Xero link model, and content-addressed doc IDs. |
| crates/ledger-core/Cargo.toml | Adds serde_json and Linux-only xattr dep for fs metadata. |
| TODO.md | Adds phased TODO plan for notifier/settings/tray/window work. |
| Cargo.toml | Adds new workspace members and shared workspace dependencies. |
| AGENTS.md | Adds updated operator direction and execution loop guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| std::fs::read_to_string(&path) | ||
| .ok() | ||
| .and_then(|raw| serde_json::from_str(&raw).ok()) | ||
| .unwrap_or_default() | ||
| } |
There was a problem hiding this comment.
load_document_registry silently falls back to an empty registry on read/parse errors. That can cause quiet state loss if the file is corrupted or partially written. Consider returning an error (or at least logging + preserving the unreadable file) so operators can recover instead of starting from an empty registry.
| if request.apply && path.exists() { | ||
| let new_path = path.with_file_name(&proposed_name); | ||
| std::fs::rename(path, &new_path) | ||
| .map_err(|e| ToolError::Internal(e.to_string()))?; |
There was a problem hiding this comment.
When apply is true, this will rename whatever file_path points at, with no allowed-base restriction (unlike ingest_pdf). Consider constraining renames to a safe base directory (e.g. self.workbook_path().parent()) and rejecting paths outside it.
|
|
||
| [features] | ||
| default = ["core", "events", "reconciliation", "hsm", "ontology", "classification", "audit", "tax"] | ||
| default = ["core", "events", "reconciliation", "hsm", "ontology", "classification", "audit", "tax", "xero", "llm"] |
There was a problem hiding this comment.
The PR is titled/described as a desktop host tray slice, but this change also expands/enables new MCP capabilities by default (notably xero + llm). If that scope increase is intended, please update the PR description to call it out explicitly (or split into separate PRs) so reviewers can evaluate risk appropriately.
| /// Infallible parse that prefixes '#' if missing and normalizes. | ||
| pub fn normalize(raw: &str) -> Self { | ||
| let s = if raw.starts_with('#') { | ||
| raw.to_string() | ||
| } else { | ||
| format!("#{raw}") | ||
| }; | ||
| Self::new(&s).unwrap_or_else(|_| Self(s.to_ascii_lowercase())) |
There was a problem hiding this comment.
Tag::normalize falls back to Self(s.to_ascii_lowercase()) on validation failure, which can create Tag values containing invalid characters (spaces/symbols) despite Tag being documented as validated. Consider sanitizing into a valid tag (e.g., replace invalid chars) or make normalize return Result<Tag, TagError> to preserve the invariant.
| /// Infallible parse that prefixes '#' if missing and normalizes. | |
| pub fn normalize(raw: &str) -> Self { | |
| let s = if raw.starts_with('#') { | |
| raw.to_string() | |
| } else { | |
| format!("#{raw}") | |
| }; | |
| Self::new(&s).unwrap_or_else(|_| Self(s.to_ascii_lowercase())) | |
| fn sanitize_body(body: &str) -> String { | |
| let mut sanitized = String::new(); | |
| let mut last_was_hyphen = false; | |
| for c in body.trim().chars() { | |
| let mapped = if c.is_ascii_alphanumeric() { | |
| Some(c.to_ascii_lowercase()) | |
| } else if c == '-' || c.is_ascii_whitespace() { | |
| Some('-') | |
| } else { | |
| Some('-') | |
| }; | |
| if let Some(ch) = mapped { | |
| if ch == '-' { | |
| if !sanitized.is_empty() && !last_was_hyphen { | |
| sanitized.push(ch); | |
| last_was_hyphen = true; | |
| } | |
| } else { | |
| sanitized.push(ch); | |
| last_was_hyphen = false; | |
| } | |
| } | |
| } | |
| let sanitized = sanitized.trim_matches('-').to_string(); | |
| if sanitized.is_empty() { | |
| "tag".to_string() | |
| } else { | |
| sanitized | |
| } | |
| } | |
| /// Infallible parse that prefixes '#' if missing and normalizes. | |
| pub fn normalize(raw: &str) -> Self { | |
| let s = raw.trim(); | |
| let s = if s.starts_with('#') { | |
| s.to_string() | |
| } else { | |
| format!("#{s}") | |
| }; | |
| Self::new(&s).unwrap_or_else(|_| { | |
| let body = s.strip_prefix('#').unwrap_or(&s); | |
| Self(format!("#{}", Self::sanitize_body(body))) | |
| }) |
| let proposed_name = format!( | ||
| "{}--{}--{}--{}.{}", | ||
| vendor, account, request.year_month, request.doc_type, ext | ||
| ); |
There was a problem hiding this comment.
proposed_name is constructed from user-supplied strings without sanitizing path separators / traversal sequences. Reject or sanitize characters like /, \, and .. in vendor/account/year_month/doc_type so the output is always a safe filename and can’t escape the intended directory.
| "source_ref": "/tmp/l3dg3rr-outcome-flow-161024-1776478923653/WF--BH-CHK--2023-01--statement.rkyv" | ||
| } |
There was a problem hiding this comment.
This persisted state sidecar appears to be a generated local artifact and includes machine-specific absolute paths under /tmp. It shouldn’t be committed; please remove it from the PR and add an ignore rule (e.g. *.ledgerr-state.json) so local runs don’t dirty the repo or leak local paths.
| pub struct XeroContact { | ||
| pub contact_i_d: String, | ||
| pub name: String, | ||
| #[serde(default)] | ||
| pub email_address: Option<String>, |
There was a problem hiding this comment.
Public Xero types use *_i_d field names to hit PascalCase → *ID serde mapping. This leaks an awkward naming convention into the public API; consider using idiomatic Rust names (e.g. contact_id) with explicit #[serde(rename = "ContactID")] instead.
| let filter_tags: Vec<String> = request.tags.iter() | ||
| .map(|t| Tag::normalize(t).as_str().to_string()) | ||
| .collect(); |
There was a problem hiding this comment.
list_tagged_tool never uses request.directory, even though the contract includes it. Either implement directory scoping (e.g. only include records whose file_path is under the provided directory) or remove the parameter from the API to avoid misleading callers.
| tokio = { workspace = true, optional = true } | ||
| tracing = { workspace = true } |
There was a problem hiding this comment.
tokio is added as an optional dependency (and pulled in by the xero/llm features), but there are no tokio:: usages in this crate (or in the new ledgerr-xero/ledgerr-llm crates). Unless there’s a near-term async follow-up, consider removing this to avoid unnecessary compile time and transitive deps.
| let settings = store.load()?; | ||
| let notify_settings = NotificationSettings { | ||
| enabled: settings.toast_enabled, | ||
| backend: settings.toast_backend_preference, | ||
| last_test_result: settings.last_test_result.clone(), | ||
| }; | ||
| let notifier = PowerShellBurntToastNotifier::new(notify_settings); | ||
| let result = notifier.test("l3dg3rr", "tray test toast"); | ||
|
|
||
| let mut settings = settings; | ||
| let mut state = state.lock().expect("tray state poisoned"); | ||
| match result { | ||
| Ok(test_result) => { | ||
| settings.last_test_result = Some(test_result); | ||
| settings.toast_backend_preference = NotificationBackend::PowerShell; | ||
| store.save(&settings)?; | ||
| state.notification_status = if state.toast_enabled { | ||
| crate::notify::NotificationStatus::Ready | ||
| } else { | ||
| crate::notify::NotificationStatus::Disabled | ||
| }; | ||
| } | ||
| Err(_) => { | ||
| state.notification_status = crate::notify::NotificationStatus::Failed; |
There was a problem hiding this comment.
Even when settings request NotificationBackend::Noop or Auto, the tray runtime always constructs a PowerShellBurntToastNotifier and attempts to send a PowerShell toast. Consider routing through a backend-selection layer that respects toast_backend_preference (and uses a real noop backend when configured).
| let settings = store.load()?; | |
| let notify_settings = NotificationSettings { | |
| enabled: settings.toast_enabled, | |
| backend: settings.toast_backend_preference, | |
| last_test_result: settings.last_test_result.clone(), | |
| }; | |
| let notifier = PowerShellBurntToastNotifier::new(notify_settings); | |
| let result = notifier.test("l3dg3rr", "tray test toast"); | |
| let mut settings = settings; | |
| let mut state = state.lock().expect("tray state poisoned"); | |
| match result { | |
| Ok(test_result) => { | |
| settings.last_test_result = Some(test_result); | |
| settings.toast_backend_preference = NotificationBackend::PowerShell; | |
| store.save(&settings)?; | |
| state.notification_status = if state.toast_enabled { | |
| crate::notify::NotificationStatus::Ready | |
| } else { | |
| crate::notify::NotificationStatus::Disabled | |
| }; | |
| } | |
| Err(_) => { | |
| state.notification_status = crate::notify::NotificationStatus::Failed; | |
| let mut settings = store.load()?; | |
| let backend = settings.toast_backend_preference; | |
| let notify_settings = NotificationSettings { | |
| enabled: settings.toast_enabled, | |
| backend, | |
| last_test_result: settings.last_test_result.clone(), | |
| }; | |
| let mut state = state.lock().expect("tray state poisoned"); | |
| match backend { | |
| NotificationBackend::Noop => { | |
| state.notification_status = crate::notify::NotificationStatus::Disabled; | |
| } | |
| NotificationBackend::PowerShell | NotificationBackend::Auto => { | |
| let notifier = PowerShellBurntToastNotifier::new(notify_settings); | |
| match notifier.test("l3dg3rr", "tray test toast") { | |
| Ok(test_result) => { | |
| settings.last_test_result = Some(test_result); | |
| store.save(&settings)?; | |
| state.notification_status = if state.toast_enabled { | |
| crate::notify::NotificationStatus::Ready | |
| } else { | |
| crate::notify::NotificationStatus::Disabled | |
| }; | |
| } | |
| Err(_) => { | |
| state.notification_status = | |
| crate::notify::NotificationStatus::Failed; | |
| } | |
| } |
|
@copilot apply changes based on the comments in this thread |
…eview comments Agent-Logs-Url: https://github.com/PromptExecution/l3dg3rr/sessions/71b99866-787b-4851-9f7d-7389a02da586 Co-authored-by: elasticdotventures <35611074+elasticdotventures@users.noreply.github.com>
… variable names in sanitize_body Agent-Logs-Url: https://github.com/PromptExecution/l3dg3rr/sessions/71b99866-787b-4851-9f7d-7389a02da586 Co-authored-by: elasticdotventures <35611074+elasticdotventures@users.noreply.github.com>
Applied all changes from the PR review thread in commits Clippy fixes:
Security / path traversal:
Error handling:
Xero OAuth:
Xero types: Idiomatic Rust field names ( Tray:
Other:
|
This PR adds the first desktop-host slice for
l3dg3rr:ledgerr-hostcrate with notifier, settings, and tray modulesValidation:
cargo test -p ledgerr-hosthost-tray.exehost-window.exeNotes: