Catch main up to origin/main#34
Merged
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| let model = client.completion_model(settings.model.trim()); | ||
| let request = build_request(model.clone(), settings, history, pending_message); | ||
| let response = model.completion(request).await?; | ||
| extract_assistant_message(response.choice.into_iter()).ok_or(ChatError::MissingAssistantMessage) |
| let model = client.completion_model(settings.model.trim()); | ||
| let request = build_request(model.clone(), settings, history, pending_message); | ||
| let response = model.completion(request).await?; | ||
| extract_assistant_message(response.choice.into_iter()).ok_or(ChatError::MissingAssistantMessage) |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR syncs local main to origin/main while retaining the v1.5.0 release commit, bringing in the Windows host/tray work, MCP cleanup, and the workspace version bump to 1.5.0.
Changes:
- Bump workspace + crate versions to
v1.5.0and update lockfile. - Expand Windows host tray state/settings and add a basic Slint host window with a
rig-core-backed chat client. - Tooling/docs updates: Justfile recipes hardened (cog bootstrap) and operational notes added to
AGENTS.md.
Reviewed changes
Copilot reviewed 39 out of 41 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| xtask/src/verify.rs | Formatting-only refactor for manifest lookup/error mapping. |
| xtask/src/server_json.rs | Formatting-only serde attribute reformat. |
| xtask/src/publisher.rs | Formatting-only refactors for constructors and error mapping. |
| xtask/src/manifest.rs | Formatting-only refactor of validation error construction. |
| xtask/src/bundler.rs | Formatting-only refactors; clearer struct literals and error variants. |
| crates/ledgerr-xero/src/client.rs | Formatting-only refactor of error return and request line wrapping. |
| crates/ledgerr-mcp/src/reconciliation.rs | Formatting-only refactor of request parsing and error mapping. |
| crates/ledgerr-mcp/src/plugin_info.rs | Formatting-only refactors around file ops, URL formatting, and logging calls. |
| crates/ledgerr-mcp/src/mcp_adapter.rs | Formatting-only refactors; request construction wrapped for readability. |
| crates/ledgerr-mcp/src/lib.rs | Minor reordering + formatting; document/tag helpers and xero wrapper signatures wrapped. |
| crates/ledgerr-mcp/src/bin/regen-docs.rs | Formatting-only refactor to multiline write(...) calls. |
| crates/ledgerr-mcp/Cargo.toml | Bump internal dependency versions to =1.5.0. |
| crates/ledgerr-llm/src/lib.rs | Formatting-only refactors; multi-line ctor and method signatures. |
| crates/ledgerr-llm/src/extract.rs | Comment alignment/formatting changes. |
| crates/ledgerr-host/tests/tray_state.rs | Update tests for richer tray state + add coverage for deriving tray state from settings. |
| crates/ledgerr-host/tests/settings_roundtrip.rs | Add ChatSettings to roundtrip + legacy JSON test for missing chat block. |
| crates/ledgerr-host/src/tray/state.rs | Introduce richer TrayState derived from AppSettings + new tray commands. |
| crates/ledgerr-host/src/tray/runtime.rs | Expand tray menu + persist additional settings + backend cycling + test handling. |
| crates/ledgerr-host/src/tray/mod.rs | Re-export order tweak for tray labels. |
| crates/ledgerr-host/src/tray/menu.rs | Add backend/last-test/settings labels + truncation helper for status messages. |
| crates/ledgerr-host/src/settings/schema.rs | Add persisted ChatSettings and wire into AppSettings. |
| crates/ledgerr-host/src/settings/path.rs | Formatting-only refactor in tests. |
| crates/ledgerr-host/src/settings/mod.rs | Export ChatSettings. |
| crates/ledgerr-host/src/notify/types.rs | Formatting-only refactor of NotificationEvent::Test. |
| crates/ledgerr-host/src/notify/powershell.rs | Formatting-only refactor of toast script formatting. |
| crates/ledgerr-host/src/lib.rs | Export new chat module. |
| crates/ledgerr-host/src/chat.rs | New chat client wrapper using rig-core + settings validation + unit tests. |
| crates/ledgerr-host/src/bin/host-window.rs | Expand Slint UI into a settings+chat window that reads/writes settings.json. |
| crates/ledgerr-host/src/bin/host-tray.rs | Formatting-only refactor of store initialization. |
| crates/ledgerr-host/Cargo.toml | Add rig-core, tokio, and reqwest to support chat feature. |
| crates/ledger-core/tests/phase2_rustledger_journal.rs | Formatting-only test wrapping. |
| crates/ledger-core/tests/phase1_contracts.rs | Formatting-only test wrapping. |
| crates/ledger-core/src/tags.rs | Formatting-only refactor of error attribute layout. |
| crates/ledger-core/src/fs_meta.rs | Formatting-only refactor of cfg blocks. |
| crates/ledger-core/src/filename.rs | Formatting-only refactor of parsing expressions. |
| crates/ledger-core/src/document.rs | Formatting-only refactor of ctor signature and tests. |
| Justfile | Add WSL/PowerShell host recipes + add ensure-cog guard for cocogitto recipes. |
| Cargo.toml | Workspace version bump to 1.5.0. |
| Cargo.lock | Dependency graph updates for 1.5.0 and new host deps (rig-core, tokio, reqwest versions). |
| CHANGELOG.md | Add v1.5.0 entry and minor formatting normalization. |
| AGENTS.md | Add guidance to treat Justfile as canonical command source + structure notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
329
to
+336
| match command { | ||
| TrayCommand::ToggleToast(enabled) => { | ||
| let mut settings = store.load()?; | ||
| settings.toast_enabled = enabled; | ||
| store.save(&settings)?; | ||
|
|
||
| let mut state = state.lock().expect("tray state poisoned"); | ||
| state.toast_enabled = enabled; | ||
| state.notification_status = if enabled { | ||
| crate::notify::NotificationStatus::Unknown | ||
| } else { | ||
| crate::notify::NotificationStatus::Disabled | ||
| }; | ||
| let status = tray_menu_labels(&state).status; | ||
| let _ = tray.control_tx.send(TrayControl::SetToastEnabled(enabled)); | ||
| let _ = tray.control_tx.send(TrayControl::SetStatus(status)); | ||
| sync_state(state, &settings, tray); | ||
| Ok(false) |
There was a problem hiding this comment.
TrayCommand::ToggleStartMinimizedToTray is generated from menu events (see the event-id dispatch above) but there is no corresponding handle_command match arm. This makes the match command non-exhaustive (won’t compile) and also prevents persisting/updating settings.start_minimized_to_tray when the menu item is clicked. Add a match arm that updates settings.start_minimized_to_tray, saves, and calls sync_state (similar to ToggleWindowVisibleOnStart).
Comment on lines
+33
to
+47
| let notification_status = if settings.toast_enabled { | ||
| settings | ||
| .last_test_result | ||
| .as_ref() | ||
| .map(|result| result.status) | ||
| .unwrap_or(NotificationStatus::Unknown) | ||
| } else { | ||
| NotificationStatus::Disabled | ||
| }; | ||
|
|
||
| Self { | ||
| toast_enabled: settings.toast_enabled, | ||
| notification_backend: settings.toast_backend_preference, | ||
| notification_status, | ||
| last_test_result: settings.last_test_result.clone(), |
There was a problem hiding this comment.
TrayState::from_settings derives notification_status solely from toast_enabled + last_test_result. After cycling the backend to Noop, an old last_test_result (e.g., Ready) can cause the tray to display Backend: Noop but Status: Ready, which is misleading. Consider forcing notification_status to Disabled when toast_backend_preference is Noop, and/or clearing last_test_result when the backend changes.
Suggested change
| let notification_status = if settings.toast_enabled { | |
| settings | |
| .last_test_result | |
| .as_ref() | |
| .map(|result| result.status) | |
| .unwrap_or(NotificationStatus::Unknown) | |
| } else { | |
| NotificationStatus::Disabled | |
| }; | |
| Self { | |
| toast_enabled: settings.toast_enabled, | |
| notification_backend: settings.toast_backend_preference, | |
| notification_status, | |
| last_test_result: settings.last_test_result.clone(), | |
| let notification_backend = settings.toast_backend_preference; | |
| let notifications_disabled = | |
| !settings.toast_enabled || matches!(notification_backend, NotificationBackend::Noop); | |
| let notification_status = if notifications_disabled { | |
| NotificationStatus::Disabled | |
| } else { | |
| settings | |
| .last_test_result | |
| .as_ref() | |
| .map(|result| result.status) | |
| .unwrap_or(NotificationStatus::Unknown) | |
| }; | |
| let last_test_result = if notifications_disabled { | |
| None | |
| } else { | |
| settings.last_test_result.clone() | |
| }; | |
| Self { | |
| toast_enabled: settings.toast_enabled, | |
| notification_backend, | |
| notification_status, | |
| last_test_result, |
Comment on lines
+31
to
+41
| #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] | ||
| pub struct ChatSettings { | ||
| #[serde(default = "default_chat_endpoint")] | ||
| pub endpoint_url: String, | ||
| #[serde(default)] | ||
| pub api_key: String, | ||
| #[serde(default)] | ||
| pub model: String, | ||
| #[serde(default = "default_chat_system_prompt")] | ||
| pub system_prompt: String, | ||
| } |
There was a problem hiding this comment.
ChatSettings adds a persisted api_key field that will be written to settings.json in plaintext. This is a security risk (easy to exfiltrate via backups/log collection, accidental commits, etc.). Prefer storing secrets in the OS credential store (Windows Credential Manager / macOS Keychain / libsecret), or at minimum avoid serializing the key by default and load it from an environment variable / external secret provider.
| LineEdit { text <=> root.model_text; enabled: !root.busy; } | ||
|
|
||
| Text { text: "API Key (persisted in settings.json)"; color: #445566; } | ||
| LineEdit { text <=> root.api_key_text; enabled: !root.busy; } |
There was a problem hiding this comment.
The host window displays the API key in a normal LineEdit, which will show the secret in cleartext and makes shoulder-surfing/screenshots likely. If the API key must be entered here, mask it (password/hidden echo mode) and consider adding a dedicated “reveal” toggle.
Suggested change
| LineEdit { text <=> root.api_key_text; enabled: !root.busy; } | |
| LineEdit { | |
| text <=> root.api_key_text; | |
| enabled: !root.busy; | |
| input-type: password; | |
| } |
Comment on lines
+22
to
+30
| powershell.exe -NoProfile -Command '$env:PATH = "C:\Users\wendy\.cargo\bin;C:\msys64\mingw64\bin;" + $env:PATH; Set-Location "C:\Users\wendy\l3dg3rr"; cargo build -p ledgerr-host --bin host-tray --bin host-window' | ||
|
|
||
| # Rebuild and launch the tray host on Windows. | ||
| wsl2-pwsh-run-tray: | ||
| powershell.exe -NoProfile -Command '$env:PATH = "C:\Users\wendy\.cargo\bin;C:\msys64\mingw64\bin;" + $env:PATH; Set-Location "C:\Users\wendy\l3dg3rr"; cargo build -p ledgerr-host --bin host-tray | Out-Null; Get-Process host-tray -ErrorAction SilentlyContinue | Stop-Process -Force; Start-Sleep -Milliseconds 250; Start-Process -FilePath "C:\Users\wendy\l3dg3rr\target\debug\host-tray.exe" -WorkingDirectory "C:\Users\wendy\l3dg3rr"' | ||
|
|
||
| # Rebuild and launch the separate Slint host window on Windows. | ||
| wsl2-pwsh-run-window: | ||
| powershell.exe -NoProfile -Command '$env:PATH = "C:\Users\wendy\.cargo\bin;C:\msys64\mingw64\bin;" + $env:PATH; Set-Location "C:\Users\wendy\l3dg3rr"; cargo build -p ledgerr-host --bin host-window | Out-Null; Start-Process -FilePath "C:\Users\wendy\l3dg3rr\target\debug\host-window.exe" -WorkingDirectory "C:\Users\wendy\l3dg3rr"' |
There was a problem hiding this comment.
These WSL/PowerShell recipes hardcode a specific Windows username and repo path (C:\Users\wendy\...). That will break for other developers/CI and makes the Justfile non-portable. Use environment variables (e.g., %USERPROFILE%, $env:USERPROFILE) and/or compute the repo path dynamically (e.g., pass it as a just parameter), and avoid embedding user-specific paths in canonical workflows.
Suggested change
| powershell.exe -NoProfile -Command '$env:PATH = "C:\Users\wendy\.cargo\bin;C:\msys64\mingw64\bin;" + $env:PATH; Set-Location "C:\Users\wendy\l3dg3rr"; cargo build -p ledgerr-host --bin host-tray --bin host-window' | |
| # Rebuild and launch the tray host on Windows. | |
| wsl2-pwsh-run-tray: | |
| powershell.exe -NoProfile -Command '$env:PATH = "C:\Users\wendy\.cargo\bin;C:\msys64\mingw64\bin;" + $env:PATH; Set-Location "C:\Users\wendy\l3dg3rr"; cargo build -p ledgerr-host --bin host-tray | Out-Null; Get-Process host-tray -ErrorAction SilentlyContinue | Stop-Process -Force; Start-Sleep -Milliseconds 250; Start-Process -FilePath "C:\Users\wendy\l3dg3rr\target\debug\host-tray.exe" -WorkingDirectory "C:\Users\wendy\l3dg3rr"' | |
| # Rebuild and launch the separate Slint host window on Windows. | |
| wsl2-pwsh-run-window: | |
| powershell.exe -NoProfile -Command '$env:PATH = "C:\Users\wendy\.cargo\bin;C:\msys64\mingw64\bin;" + $env:PATH; Set-Location "C:\Users\wendy\l3dg3rr"; cargo build -p ledgerr-host --bin host-window | Out-Null; Start-Process -FilePath "C:\Users\wendy\l3dg3rr\target\debug\host-window.exe" -WorkingDirectory "C:\Users\wendy\l3dg3rr"' | |
| powershell.exe -NoProfile -Command "\$repo = '$(wslpath -w "$PWD")'; \$env:PATH = \"\$env:USERPROFILE\.cargo\bin;C:\msys64\mingw64\bin;\" + \$env:PATH; Set-Location \$repo; cargo build -p ledgerr-host --bin host-tray --bin host-window" | |
| # Rebuild and launch the tray host on Windows. | |
| wsl2-pwsh-run-tray: | |
| powershell.exe -NoProfile -Command "\$repo = '$(wslpath -w "$PWD")'; \$env:PATH = \"\$env:USERPROFILE\.cargo\bin;C:\msys64\mingw64\bin;\" + \$env:PATH; Set-Location \$repo; cargo build -p ledgerr-host --bin host-tray | Out-Null; Get-Process host-tray -ErrorAction SilentlyContinue | Stop-Process -Force; Start-Sleep -Milliseconds 250; Start-Process -FilePath (Join-Path \$repo 'target\debug\host-tray.exe') -WorkingDirectory \$repo" | |
| # Rebuild and launch the separate Slint host window on Windows. | |
| wsl2-pwsh-run-window: | |
| powershell.exe -NoProfile -Command "\$repo = '$(wslpath -w "$PWD")'; \$env:PATH = \"\$env:USERPROFILE\.cargo\bin;C:\msys64\mingw64\bin;\" + \$env:PATH; Set-Location \$repo; cargo build -p ledgerr-host --bin host-window | Out-Null; Start-Process -FilePath (Join-Path \$repo 'target\debug\host-window.exe') -WorkingDirectory \$repo" |
Comment on lines
7
to
+14
| [dependencies] | ||
| chrono = { workspace = true } | ||
| reqwest = { workspace = true } | ||
| rig-core = "0.35.0" | ||
| serde = { workspace = true } | ||
| serde_json = { workspace = true } | ||
| thiserror = { workspace = true } | ||
| tokio = { workspace = true } |
There was a problem hiding this comment.
ledgerr-host adds a direct dependency on reqwest even though the code only references reqwest::Error in an error enum variant that is never constructed. This also causes multiple reqwest versions in the lockfile (workspace uses 0.12, rig-core pulls 0.13), increasing build time and binary size. Either remove the unused reqwest dependency/ChatError::Http variant, or align the workspace reqwest version with rig-core so only one version is built.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR brings local
mainup to the currentorigin/mainstate while preserving thev1.5.0release commit in history.It includes the release commit plus the merge of
origin/maincontaining the host/tray and MCP cleanup work.Validation:
cargo test -p ledgerr-host --lib --test settings_roundtrip --test settings_atomicity --test tray_statecargo test -p ledgerr-mcp --lib --tests