Repository navigation
feat(backend): compile the 43 orphaned src-gossamer modules in the lib - #153
Merged
Merged
Conversation
These 43 modules were declared by no crate root, so `cargo fmt` never reached them. Formatting only, no code changes, so the next commit's diff (wiring them into the lib) carries only real edits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SJtjqoZaSQo7uBwvwVMA9G
43 modules under src-gossamer/src/ were declared by no crate root, so they were never compiled, linted or tested. None needs gossamer_rs, so all are now `pub mod` in lib.rs and build without the optional `gossamer` feature. main.rs still adds only system_tray. `cargo test --lib` now runs 285 tests (was 23). Fixes needed once the code compiled (clippy -D warnings, rustdoc): - 221 blank lines between a doc comment and its fn, left where #[tauri::command] attributes were deleted - minter_mint_panel takes a MintRequest instead of 12 positional args (too_many_arguments); generate_panel_manifest drops an unused param - a2ml, cloudguard, farm, script_gist, security: if_same_then_else, bind_instead_of_map, for_kv_map, wildcard_in_or_patterns, useless_format, manual_strip - tests: single_match (a2ml), irrefutable let (valence_shell), always-true len() >= 0 assert (release_manager) - compat::AppHandle derives Default (new_without_default) - bare URL (watcher) and unescaped <id> (identity) in rustdoc Tauri leftovers removed from compat, error (to_tauri_result is now to_command_result), cloudguard::commands and ums_cartridge::commands. The handlers are not yet registered in register_commands (main.rs); that is follow-up work. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SJtjqoZaSQo7uBwvwVMA9G
- Cargo.toml header claimed "All 103 command handlers are registered"; 36 are. It also said libgossamer.so is linked at runtime; gossamer-rs links libgossamer statically. - EXPLAINME-new.adoc claimed "270 migrated commands" and pointed at src-tauri/; it now says 36 registered, with the rest compiled and tested in the lib but not registered. - docs/TECHNICAL_DEBT.adoc: F1 census step done (43 modules, 285 lib tests). - CHANGELOG entry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SJtjqoZaSQo7uBwvwVMA9G
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 50 minutes. View limit details
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Hypatia code scanning flagged the expect() (CWE-754) once rustfmt re-wrapped the line. It could not fire, because an is_empty() check returned first, but `let Some(latest) = recordings.last() else` covers both cases with no panic path. The separate guard and its wrong "line 146" comment are gone. Behaviour is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SJtjqoZaSQo7uBwvwVMA9G
hyperpolymath
marked this pull request as ready for review
October 14, 2026 11:23
|
This was referenced Oct 19, 2026
hyperpolymath
added a commit
that referenced
this pull request
Oct 19, 2026
## Summary This PR changes docs only, in three files. It follows up on #153. **Rust test counts.** README said 12, EXPLAINME-new said 12, and TEST-NEEDS said 267 "all passing". `cargo test --locked --lib` runs **285** since #153. Before #153, only 23 of TEST-NEEDS' 267 tests actually compiled. The rest lived in modules that no crate root declared. TEST-NEEDS keeps its 267 figure as history and adds a dated correction next to it. **JS-suite claims.** README ("97 JS") and EXPLAINME-new ("CI enforces both") described a JS suite that can't run today: - 140 of the 143 files under `tests/` import compiled ReScript output (`src/**/*.res.js`) that #107 removed. - #108 removed `deno.json`. - `build-validation.yml` runs only the Rust test gate. The docs now say the JS suite waits on the AffineScript port (#148). TEST-NEEDS gets the same note under its JS rows. README's `bun task test` became `cargo test --locked --lib`, because there is no JS manifest to define a `test` task. Also: TEST-NEEDS said there are 116 Rust files; there are 112. ## Not changed README's other build commands (`bun task res:buildbures`, "BuildBu", and so on) look damaged too. They belong to the frontend port, so this PR leaves them alone. ## Verification - `cargo test --locked --lib` on `main` (9961333): 285 passed. - `grep -c '#\[test\]\|#\[tokio::test\]'` finds 288 matches. 285 are real tests. The other 3 are doc comments that mention `#[tokio::test]`. --- _Generated by [Claude Code](https://claude.ai/code/session_01SJtjqoZaSQo7uBwvwVMA9G)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
hyperpolymath
added a commit
that referenced
this pull request
Oct 19, 2026
…r) (#157) ## Summary This PR follows up on #153. That PR compiled 43 backend modules into the lib, but their handlers were never registered, so the app can't call them. This PR registers them. It is a **draft**. This first commit adds the bridge and the `coprocessor` pilot (F2 in `docs/TECHNICAL_DEBT.adoc`). The other modules follow in batches on this branch, using the same pattern. ## Design The new GTK-free lib module `ipc` holds a command table, and `main.rs` registers every entry with `App::command_async`. Each call therefore runs on a Gossamer worker thread and never on the GTK main thread. Because the table is in the lib, `cargo test --lib` checks every entry without GTK. The conventions below come from the old frontend's `invoke()` calls, recovered from the ReScript sources deleted in #107. There were 382 distinct names across 75 `*Cmd.res` files. | | Rule | Why | |---|---|---| | Argument keys | Look up the snake_case key first, then its camelCase form. | Tauri renamed `providerId` → `provider_id` automatically; the Gossamer bridge does not. The old frontend used both: 151 backend functions match it exactly, and about 36 differ only in casing. | | Missing argument | It reads as `null`. `Option` parameters get `None`; a required one fails with ``missing argument `x` `` before the handler runs. | Tauri rejected such calls too. Some old frontend calls omitted arguments, for example `valence_shell_input` without `session_id`. | | Struct parameters | Use the named key if present, otherwise the whole payload. | `minter_mint_panel` was called with flattened fields and `ai_send_message` with both shapes. | | Response | `{"ok": true, "result": …}` / `{"ok": false, "error": "…"}`, the envelope `main.rs` already uses. A result that isn't JSON stays a string instead of becoming `null`. | It stays consistent for the AffineScript frontend (#148). | | `async fn` | `ipc::block_on` on one shared multi-thread Tokio runtime. Sync handlers are called directly. | Spawned tasks outlive the call, which AI streaming needs. Several sync handlers use `reqwest::blocking`, which must not run inside a runtime. I checked that no `async` command file calls the blocking modules. | ## Progress - [x] Bridge, conventions and tests (`src-gossamer/src/ipc.rs`) - [x] Pilot: `coprocessor`, 10 commands - [ ] Remaining modules, 223 commands, in batches - [ ] Event pump: drain `compat`'s event bus to the webview through `gossamer_emit`, which is thread-safe. This lands in the same batch as `ai_send_message_streaming` and `watcher_start`, the two handlers that emit events. - [ ] Docs: the `Cargo.toml` header count, EXPLAINME-new, `TECHNICAL_DEBT` F1/F2, CHANGELOG Not registered: `ums_cartridge::validate_level_data`, which is a helper that returns a `ValidationResult`. The 10 `llm_coding` commands were already registered in `main.rs`, and a test keeps the table and `main.rs` from registering the same name twice. ## Verification (local) - `cargo fmt --check`, `cargo clippy --locked --all-targets -- -D warnings`, and `RUSTDOCFLAGS="-D warnings" cargo doc`: all clean. - `cargo test --locked`: 293 passed (285 + 8 new `ipc` tests), plus 2 ignored doctests. - `cargo clippy --locked --features gossamer --bin panll-gossamer -- -D warnings`: clean, type-checked against the real gossamer-rs `command_async`. This used an empty `libgossamer.a` and stub `.pc` files, because the container has no Zig or GTK. The e2e job does the real build. `governance / *`, `scan / *` and Hypatia will fail at job setup with `invalid lockfile pin`, as they do on every PR right now. #156 fixes that. --- _Generated by [Claude Code](https://claude.ai/code/session_01SJtjqoZaSQo7uBwvwVMA9G)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
hyperpolymath
added a commit
that referenced
this pull request
Oct 19, 2026
## Summary `cargo clippy --locked --features gossamer --all-targets -- -D warnings` failed on `main` in the `panll-gossamer` test target with two errors: - `struct SystemTrayState is never constructed` - `associated function new is never used` These are plain rustc dead-code warnings, which `-D warnings` turns into errors. The state was built inside `lazy_static!`'s generated initializer, and dead-code analysis doesn't count that as a use in test builds. The non-test bin and the default-feature targets were clean. No CI job runs this command, so it never showed up in CI. #153 noted it as an issue that predates that PR. **Fix:** `SYSTEM_TRAY_STATE` is now a `static Mutex<SystemTrayState>` built by a `const fn new()`. `Mutex::new` is `const`, so the code needs neither `lazy_static` nor the `Arc`. Behaviour doesn't change: the initial values are the same, `new()` has no side effects, and every caller still uses `SYSTEM_TRAY_STATE.lock()`. `system_tray` was the only user of `lazy_static`, so the dependency is dropped. `Cargo.lock` was regenerated by cargo and loses only the `lazy_static` entry. ## Verification (local) The default-feature gates: - `cargo fmt --all -- --check`: clean - `cargo clippy --locked --all-targets -- -D warnings`: clean - `cargo test --locked`: 285 passed, plus 2 doctests (both `ignore`) - `RUSTDOCFLAGS="-D warnings" cargo doc --no-deps`: clean The `gossamer` feature: this container has no Zig or GTK/WebKit. I pointed `GOSSAMER_LIB_DIR` at an empty `libgossamer.a` and added stub `.pc` files. This setup type-checks without linking, so the e2e job remains the real build and test. - `cargo clippy --locked --features gossamer --all-targets -- -D warnings`: **clean**. On `main` it shows the 2 errors above. - `cargo check --locked --features gossamer --all-targets`: clean --- _Generated by [Claude Code](https://claude.ai/code/session_01SJtjqoZaSQo7uBwvwVMA9G)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
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
3 participants
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.
Summary
43 modules under
src-gossamer/src/were declared by no crate root, so they were never compiled, linted or tested. None of them needsgossamer_rs, so all 43 are nowpub modinlib.rsand build without the optionalgossamerfeature.main.rsstill adds onlysystem_tray.cargo test --lib: 285 passed (was 23).The modules: a2ml, aerie, ai, boj, boj_live, capture, clade_scanner, cloudguard, compat, dlc_workshop, echidna_live, error, farm, feedback, fleet, game_preview, governance, hypatia, k9, level_architect, minter, multiplayer_monitor, network_topology, observability, overlay, plaza, provenance, release_manager, repoloader, script_gist, security, system_update, typell, umoja, ums, ums_cartridge, valence_shell, verisim_live, vm_inspector, voicetag, watcher, wiring_inspector, workspace.
Commits (review them one at a time)
style(backend): rustfmt …: formatting only, about 1,100 lines.cargo fmthad never reached these files. You can skip this one.feat(backend): compile …: the real change, mostly deletions:fn, left behind when#[tauri::command]attributes were deleted (clippy::empty_line_after_doc_comments).minter_mint_panelnow takes aMintRequestinstead of 12 positional arguments (too_many_arguments). That struct already derivesDeserializefor exactly this payload, so IPC registration can doserde_json::from_value::<MintRequest>(payload)likellm_coding_spawn.generate_panel_manifestdrops an unused parameter.a2ml_list's doc.single_match, an irrefutableif let, and an always-truelen() >= 0assert.compat::AppHandlederivesDefault.watcherand an unescaped<id>inidentity. Theidentityone already fails onmain.compat,error(to_tauri_result→to_command_result),cloudguard::commandsandums_cartridge::commands.docs: …:Cargo.tomlheader claimed "All 103 command handlers are registered"; 36 are. It also said libgossamer is linked at runtime as a.so; it is linked statically.EXPLAINME-new.adocclaimed "270 migrated commands" and linkedsrc-tauri/; both now match the code.docs/TECHNICAL_DEBT.adoc: the F1 census step is marked done.Not in this PR
register_commands(main.rs). That is the follow-up.src-tauristrings inminterandrepoloader, which point at other repos' layouts or generated output, and theidaptik-ums/src-tauri/...path inums_cartridge, which names a file in another repo.cargo clippy --features gossamer --all-targets -D warningsfails onSystemTrayStatedead code in the bin's test target. That failure already exists onmain, and no CI job runs that command.Verification (default features, local)
cargo check --locked --all-targets: cleancargo clippy --locked --all-targets -- -D warnings: cleancargo fmt --all -- --check: cleancargo test --locked --all-targets: 285 passed. Five repeat runs and--test-threads=1(as build-validation runs it) were also clean.cargo test --locked: 285 passed, plus 2 doctests (bothignore)RUSTDOCFLAGS="-D warnings" cargo doc --no-deps: cleancargo build --features gossamer --bin panll-gossamer: not verified here. The container has no Zig 0.15.2 and no GTK/WebKit. Instead I rancargo check --features gossamer --bin panll-gossameragainst the real pinned gossamer-rs, withGOSSAMER_LIB_DIRpointing at an emptylibgossamer.aand stub.pcfiles. It type-checks clean. The e2e workflow will do the real build and link.One side effect:
feedback's test writes a report under~/.panll/feedback/and deletes the file, but leaves the empty directory. The other new tests write under/tmp/panll.🤖 Generated with Claude Code
https://claude.ai/code/session_01SJtjqoZaSQo7uBwvwVMA9G
Generated by Claude Code