fix(daemon,viewer,release): bound ingest memory; wire real bundles; install + AppImage/sigstore fixes - #395
fix(daemon,viewer,release): bound ingest memory; wire real bundles; install + AppImage/sigstore fixes#395KooshaPari wants to merge 1 commit into
Conversation
…nstall + AppImage/sigstore fixes
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
📝 WalkthroughSummaryThis PR improves ingest memory safety and wires real session bundles into the viewer. It also updates installer, AppImage, and release workflow handling. The stated daemon and viewer tests pass, and Clippy passes. Must Fix
Should Fix
Consider
Approve / Request ChangesRequest changes because the PowerShell installer uses WalkthroughThe release workflow and installers now support normalized archive names. Daemon ingestion, decompression, and idempotency caches have bounded resource usage. Viewer bundle and wiki data now react to shared session context. ChangesRelease artifact handling
Daemon input and cache limits
Session-driven viewer state
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 2⚔️ Resolve merge conflicts 💡
🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
codeant-ai
Bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| ICON_SRC="$ROOT/assets/icons/sessionledger.iconset/icon_256x256.png" | ||
| if [ -f "$ICON_SRC" ]; then | ||
| install -m 0644 "$ICON_SRC" "$APPDIR/sessionledger.png" | ||
| else | ||
| echo "warning: brand icon not found at $ICON_SRC; AppImage will be built without an icon." >&2 | ||
| fi | ||
|
|
||
| cat >"$APPDIR/sessionledger.desktop" <<EOF | ||
| [Desktop Entry] | ||
| Type=Application | ||
| Name=SessionLedger | ||
| Comment=View SessionLedger session bundles | ||
| Exec=sl-viewer | ||
| Icon=sessionledger |
There was a problem hiding this comment.
Suggestion: The referenced icon file is absent from the repository, so the fallback still writes Icon=sessionledger while no sessionledger.png exists in the AppDir. Since appimagetool rejects desktop entries with missing icons, this warning does not provide a working fallback and the AppImage build still fails. Either provide the asset or omit the icon reference when it is unavailable. [api mismatch]
Severity Level: Major ⚠️
- ❌ Release AppImage packaging fails on the Ubuntu workflow.
- ❌ `dist-out` lacks the expected AppImage artifact.
- ⚠️ Release artifact upload can fail when no AppImage exists.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packaging/linux/package-appimage.sh
**Line:** 30:43
**Comment:**
*Api Mismatch: The referenced icon file is absent from the repository, so the fallback still writes `Icon=sessionledger` while no `sessionledger.png` exists in the AppDir. Since `appimagetool` rejects desktop entries with missing icons, this warning does not provide a working fallback and the AppImage build still fails. Either provide the asset or omit the icon reference when it is unavailable.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| $assetVersion = $Version.TrimStart("v") | ||
| $archive = $null | ||
| foreach ($candidate in @("sl-viewer-$Version-$Target.zip", "sl-viewer-$assetVersion-$Target.zip")) { | ||
| try { | ||
| Invoke-WebRequest -Uri "$baseUrl/$candidate" -Method Head -UseBasicParsing -ErrorAction Stop | Out-Null | ||
| $archive = $candidate |
There was a problem hiding this comment.
Suggestion: The archive probes use $baseUrl before it is assigned. Because Invoke-WebRequest receives an invalid relative URI, both attempts are caught as failures, $archive remains unset, and every Windows installation throws the “No sl-viewer archive found” error. Assign $baseUrl before entering the probe loop. [api mismatch]
Severity Level: Critical 🚨
- ❌ Windows `irm` installer cannot locate any release archive.
- ❌ Pinned Windows installations fail before downloading.
- ⚠️ Checksum verification and extraction never execute.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/install.ps1
**Line:** 84:89
**Comment:**
*Api Mismatch: The archive probes use `$baseUrl` before it is assigned. Because `Invoke-WebRequest` receives an invalid relative URI, both attempts are caught as failures, `$archive` remains unset, and every Windows installation throws the “No sl-viewer archive found” error. Assign `$baseUrl` before entering the probe loop.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| fn gunzip_bounded(data: &[u8], path: &Path, max_bytes: usize) -> Result<Vec<u8>, ArchiveError> { | ||
| let mut decoder = GzDecoder::new(data); | ||
| let mut out = Vec::new(); |
There was a problem hiding this comment.
Suggestion: The new decompression cap does not limit the memory allocated for the compressed input itself: restore_bundle reads the entire archive into a Vec before calling this bounded decompressor. A very large compressed archive can therefore exhaust memory even when its decompressed payload is within the 256 MiB cap. Stream the archive from the file and apply limits during reading instead of passing a fully buffered input slice. [security]
Severity Level: Major ⚠️
- ❌ Restore can exhaust process memory before decompression limits apply.
- ⚠️ Large archive restores can terminate the daemon process.
- ⚠️ Disk-backed archive size remains unbounded in memory.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-daemon/src/archive.rs
**Line:** 243:245
**Comment:**
*Security: The new decompression cap does not limit the memory allocated for the compressed input itself: `restore_bundle` reads the entire archive into a `Vec` before calling this bounded decompressor. A very large compressed archive can therefore exhaust memory even when its decompressed payload is within the 256 MiB cap. Stream the archive from the file and apply limits during reading instead of passing a fully buffered input slice.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| out_dir: &Path, | ||
| memory_store: Option<&dyn MemoryStore>, | ||
| ) -> Result<Vec<PathBuf>, EtlError> { | ||
| enforce_size_cap(jsonl_path)?; |
There was a problem hiding this comment.
Suggestion: The cap checks only the compressed file's metadata length. For .jsonl.zst inputs, read_sessions subsequently decompresses the file through CodexDir and accumulates the resulting session in memory, so a highly compressible file below the 512 MiB on-disk limit can still expand to many gigabytes and bypass the intended memory bound. Enforce a decompressed-byte limit while reading the zstd stream, not only a metadata-size limit. [security]
Severity Level: Critical 🚨
- ❌ Compressed transcript expansion can exhaust daemon memory.
- ❌ ETL processing can fail before OKF output completes.
- ⚠️ Watched transcript ingestion remains vulnerable to expansion bombs.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-daemon/src/etl.rs
**Line:** 86:86
**Comment:**
*Security: The cap checks only the compressed file's metadata length. For `.jsonl.zst` inputs, `read_sessions` subsequently decompresses the file through `CodexDir` and accumulates the resulting session in memory, so a highly compressible file below the 512 MiB on-disk limit can still expand to many gigabytes and bypass the intended memory bound. Enforce a decompressed-byte limit while reading the zstd stream, not only a metadata-size limit.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| fn compile_bundles_from_sessions(sessions: &[Session]) -> Vec<ContinuationBundle> { | ||
| if sessions.is_empty() { | ||
| return sample_bundles(); | ||
| } | ||
| sessions.iter().map(session_ledger::distill::compile).collect() |
There was a problem hiding this comment.
Suggestion: Compiling real sessions with session_ledger::distill::compile produces Context data containing fields such as cwd, title, and file references, but no created_at or model fields. TimelineEntry::from_bundle reads those two fields from the Context slice, so every real-corpus timeline entry will have an empty date and an unknown model, causing incorrect grouping and metadata display. Preserve the source timestamp/model in the normalized session or emit the fields expected by the timeline. [api mismatch]
Severity Level: Major ⚠️
- ❌ Real Timeline entries lose their source dates.
- ❌ Real Timeline entries display model as `unknown`.
- ⚠️ Date grouping and model colorization become inaccurate.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/src/app.rs
**Line:** 915:919
**Comment:**
*Api Mismatch: Compiling real sessions with `session_ledger::distill::compile` produces Context data containing fields such as `cwd`, `title`, and file references, but no `created_at` or `model` fields. `TimelineEntry::from_bundle` reads those two fields from the Context slice, so every real-corpus timeline entry will have an empty date and an `unknown` model, causing incorrect grouping and metadata display. Preserve the source timestamp/model in the normalized session or emit the fields expected by the timeline.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 122-125: Pass workflow values through step-level environment
variables before Bash uses them: in .github/workflows/release.yml lines 122-125
bind the normalized version in env and use quoted shell expansion for VER; at
lines 490-497 bind github.ref_name as TAG and derive the version from it; at
lines 563-566 bind the release tag as RELEASE_TAG for gh release download; and
at line 586 reuse the same environment-bound RELEASE_TAG for gh release upload.
In `@crates/sl-daemon/src/archive.rs`:
- Around line 239-258: The restore flow in restore_bundle must stop loading the
entire .json.gz input before gunzip_bounded runs. Open the archive as a File and
stream it through a gzip decoder, preserving the MAX_RESTORE_BYTES
decompressed-output cap; if the input itself requires bounding, enforce that
while reading rather than after buffering. Add a regression test covering a
large compressed archive and verifying restoration does not allocate the
complete compressed input.
In `@crates/sl-daemon/src/etl.rs`:
- Around line 263-274: Refactor the size-cap parsing flow around
max_etl_file_bytes into an Option<&str>-accepting helper, keeping the existing
valid, invalid, sub-minimum, and default behaviors. Update
size_cap_env_parsing_is_single_sequenced_check to call the helper with literal
Some values and None instead of mutating SL_ETL_MAX_FILE_BYTES, and remove the
environment-variable setup and cleanup from the test.
In `@crates/sl-daemon/src/http.rs`:
- Around line 275-279: Update the idempotency cache capacity handling in the
request ingestion logic to evict only one oldest or least-recently-used entry
when full, then retain the new key instead of clearing the cache. Revise
idempotency_cache_remains_bounded to assert that the expected retained keys
remain and the evicted key is absent, rather than checking only entries.len().
In `@crates/sl-viewer/src/app.rs`:
- Around line 915-919: Update compile_bundles_from_sessions so an empty sessions
slice is not treated as a successful empty corpus: preserve and propagate the
load outcome from load_sessions, or pass an explicit demo-mode/success flag, and
call sample_bundles only when loading succeeded with no sessions. Ensure Bundles
and Timeline views retain the load failure instead of displaying fabricated
bundles.
- Around line 922-925: Update the sl-viewer crate’s Dioxus dependency in
Cargo.toml from version 0.7 to 0.6, preserving the existing dependency
configuration and leaving the BundlesTab implementation unchanged.
In `@crates/sl-viewer/src/memory_tab.rs`:
- Around line 62-69: Update the selection state around selected_idx to preserve
the selected page by session_id rather than list index when use_effect
regenerates pages. After all_wiki_pages_from_sessions updates the list, resolve
the selected page using its stored session_id and update the index accordingly;
clear the selection only when that session_id is no longer present.
In `@packaging/linux/package-appimage.sh`:
- Around line 27-43: Update the icon handling around ICON_SRC and the generated
sessionledger.desktop entry so Icon=sessionledger is emitted only when the icon
installation succeeds. If ICON_SRC is missing, either omit the Icon entry from
the desktop file or fail packaging immediately; do not continue with a desktop
reference to an uninstalled icon.
In `@scripts/install.ps1`:
- Around line 81-98: Move the `$baseUrl` assignment above the archive-probing
`foreach` loop so the HEAD requests in the `$archive` resolution flow use the
release download URL. Keep the candidate probing and missing-archive error
behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 643ebc05-00ee-4a7e-8479-9e8bc18061db
📒 Files selected for processing (9)
.github/workflows/release.ymlcrates/sl-daemon/src/archive.rscrates/sl-daemon/src/etl.rscrates/sl-daemon/src/http.rscrates/sl-viewer/src/app.rscrates/sl-viewer/src/memory_tab.rspackaging/linux/package-appimage.shscripts/install.ps1scripts/install.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Kilo Code Review
- GitHub Check: Summary
⚠️ CI failures not shown inline (5)
GitHub Actions: qgate / browser e2e · axe · responsive · visual: fix(daemon,viewer,release): bound ingest memory; wire real bundles; i…
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: qgate / browser e2e · axe · responsive · visual: fix(daemon,viewer,release): bound ingest memory; wire real bundles; i…
Conclusion: failure
##[group]Run timeout 15m npm run test:a11y && timeout 15m npm run test:responsive && timeout 15m npm run test:visual
�[36;1mtimeout 15m npm run test:a11y && timeout 15m npm run test:responsive && timeout 15m npm run test:visual�[0m
shell: /usr/bin/bash -e {0}
env:
CARGO_HOME: /home/runner/.cargo
CARGO_INCREMENTAL: 0
CARGO_TERM_COLOR: always
##[endgroup]
> test:a11y
> playwright test a11y.spec.js
Running 46 tests using 1 worker
··············································
##[notice] 46 passed (1.1m)
46 passed (1.1m)
> test:responsive
> playwright test responsive.spec.js
Running 6 tests using 1 worker
······
##[notice] 6 passed (11.3s)
6 passed (11.3s)
> test:visual
> playwright test visual.spec.js
Running 15 tests using 1 worker
·······××F::error file=tests/visual/harness/visual.spec.js,title=visual.spec.js:78:1 › viewer exposes type tokens and persists theme preference,line=94,col=26:: 1) visual.spec.js:78:1 › viewer exposes type tokens and persists theme preference ────────────────%0A Error: expect(received).toContain(expected) // indexOf%0A%0A Expected substring: "Georgia"%0A Received string: "system-ui, -apple-system, BlinkMacSystemFont, \"Segoe UI\", \"SF Pro Display\", sans-serif"%0A%0A 92 | });%0A 93 |%0A > 94 | expect(tokens.display).toContain("Georgia");%0A | ^%0A 95 | expect(tokens.body).toContain("system-ui");%0A 96 | expect(tokens.mono).toContain("monospace");%0A 97 | expect(tokens.ui).toContain("system-ui");%0A at /home/runner/work/SessionLedger/SessionLedger/tests/visual/harness/visual.spec.js:94:26
GitHub Actions: qgate / prepare: fix(daemon,viewer,release): bound ingest memory; wire real bundles; i…
Conclusion: failure
##[group]Run mkdir -p coverage
�[36;1mmkdir -p coverage�[0m
�[36;1mcargo llvm-cov --package session-ledger --features sqlite --all-targets --lcov --output-path coverage/lcov.info�[0m
�[36;1mecho "lcov size: $(wc -l < coverage/lcov.info) lines"�[0m
shell: /usr/bin/bash -e {0}
env:
CARGO_HOME: /home/runner/.cargo
CARGO_INCREMENTAL: 0
CARGO_TERM_COLOR: always
CACHE_ON_FAILURE: false
##[endgroup]
info: cargo-llvm-cov currently setting cfg(coverage); you can opt-out it by passing --no-cfg-coverage
info: running `rustup component add llvm-tools-preview --toolchain 1.96.0-x86_64-unknown-linux-gnu` to install the `llvm-tools-preview` component for the selected toolchain
info: downloading component llvm-tools
�[1m�[92m Downloading�[0m crates ...
�[1m�[92m Downloaded�[0m adler2 v2.0.1
�[1m�[92m Downloaded�[0m crypto-common v0.2.2
�[1m�[92m Downloaded�[0m autocfg v1.5.1
�[1m�[92m Downloaded�[0m hashlink v0.12.1
�[1m�[92m Downloaded�[0m anes v0.1.6
�[1m�[92m Downloaded�[0m tokio-macros v2.7.1
�[1m�[92m Downloaded�[0m time-core v0.1.9
�[1m�[92m Downloaded�[0m zmij v1.0.23
�[1m�[92m Downloaded�[0m wait-timeout v0.2.1
�[1m�[92m Downloaded�[0m unarray v0.1.4
�[1m�[92m Downloaded�[0m tempfile v3.27.0
�[1m�[92m Downloaded�[0m pin-project-lite v0.2.17
�[1m�[92m Downloaded�[0m zstd v0.13.3
�[1m�[92m Downloaded�[0m zstd-safe v7.2.4
�[1m�[92m Downloaded�[0m page_size v0.6.0
�[1m�[92m Downloaded�[0m smallvec v1.15.2
�[1m�[92m Downloaded�[0m tracing-attributes v0.1.31
�[1m�[92m Downloaded�[0m unicode-ident v1.0.24
�[1m�[92m Downloaded�[0m ciborium-io v0.2.2
�[1m�[92m Downloaded�[0m block-buffer v0.12.1
�[1m�[92m Downloaded�[0m rand_xorshift v0.4.0
�[1m�[92m Downloaded�[0m errno v0.3.14
�[1m�[92m Downloaded�[0m zerocopy-derive v0.8.55
�[1m�[92m Downloaded�[0m typenum v1.20.1
�[1m�[92m Downloaded�[0m thiserror v2.0.19
�[1m�[92m Downloaded�[0m rustc-demangle v0.1.28
�[1m�[92m Downloaded�[0m parking_lot v0.12.5
�[1...
GitHub Actions: qgate / 2_browser e2e · axe · responsive · visual.txt: fix(daemon,viewer,release): bound ingest memory; wire real bundles; i…
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: qgate / 1_prepare.txt: fix(daemon,viewer,release): bound ingest memory; wire real bundles; i…
Conclusion: failure
##[group]Run mkdir -p coverage
�[36;1mmkdir -p coverage�[0m
�[36;1mcargo llvm-cov --package session-ledger --features sqlite --all-targets --lcov --output-path coverage/lcov.info�[0m
�[36;1mecho "lcov size: $(wc -l < coverage/lcov.info) lines"�[0m
shell: /usr/bin/bash -e {0}
env:
CARGO_HOME: /home/runner/.cargo
CARGO_INCREMENTAL: 0
CARGO_TERM_COLOR: always
CACHE_ON_FAILURE: false
##[endgroup]
info: cargo-llvm-cov currently setting cfg(coverage); you can opt-out it by passing --no-cfg-coverage
info: running `rustup component add llvm-tools-preview --toolchain 1.96.0-x86_64-unknown-linux-gnu` to install the `llvm-tools-preview` component for the selected toolchain
info: downloading component llvm-tools
�[1m�[92m Downloading�[0m crates ...
�[1m�[92m Downloaded�[0m adler2 v2.0.1
�[1m�[92m Downloaded�[0m crypto-common v0.2.2
�[1m�[92m Downloaded�[0m autocfg v1.5.1
�[1m�[92m Downloaded�[0m hashlink v0.12.1
�[1m�[92m Downloaded�[0m anes v0.1.6
�[1m�[92m Downloaded�[0m tokio-macros v2.7.1
�[1m�[92m Downloaded�[0m time-core v0.1.9
�[1m�[92m Downloaded�[0m zmij v1.0.23
�[1m�[92m Downloaded�[0m wait-timeout v0.2.1
�[1m�[92m Downloaded�[0m unarray v0.1.4
�[1m�[92m Downloaded�[0m tempfile v3.27.0
�[1m�[92m Downloaded�[0m pin-project-lite v0.2.17
�[1m�[92m Downloaded�[0m zstd v0.13.3
�[1m�[92m Downloaded�[0m zstd-safe v7.2.4
�[1m�[92m Downloaded�[0m page_size v0.6.0
�[1m�[92m Downloaded�[0m smallvec v1.15.2
�[1m�[92m Downloaded�[0m tracing-attributes v0.1.31
�[1m�[92m Downloaded�[0m unicode-ident v1.0.24
�[1m�[92m Downloaded�[0m ciborium-io v0.2.2
�[1m�[92m Downloaded�[0m block-buffer v0.12.1
�[1m�[92m Downloaded�[0m rand_xorshift v0.4.0
�[1m�[92m Downloaded�[0m errno v0.3.14
�[1m�[92m Downloaded�[0m zerocopy-derive v0.8.55
�[1m�[92m Downloaded�[0m typenum v1.20.1
�[1m�[92m Downloaded�[0m thiserror v2.0.19
�[1m�[92m Downloaded�[0m rustc-demangle v0.1.28
�[1m�[92m Downloaded�[0m parking_lot v0.12.5
�[1...
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,toml}: Use the Rust toolchain pinned inrust-toolchain.toml; the workspace MSRV is Rust 1.85.
Validate Rust workspace changes with the prescribed locked build, all-features test suite, Clippy, and rustfmt checks where applicable.
Files:
crates/sl-daemon/src/archive.rscrates/sl-daemon/src/http.rscrates/sl-viewer/src/memory_tab.rscrates/sl-daemon/src/etl.rscrates/sl-viewer/src/app.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Fix Clippy warnings; do not add
#[allow]unless it includes a tracking-issue comment.
Files:
crates/sl-daemon/src/archive.rscrates/sl-daemon/src/http.rscrates/sl-viewer/src/memory_tab.rscrates/sl-daemon/src/etl.rscrates/sl-viewer/src/app.rs
crates/sl-daemon/**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
Use
cargo test --manifest-path crates/sl-daemon/Cargo.tomlas the fast inner-loop test command forsl-daemonchanges.
Files:
crates/sl-daemon/src/archive.rscrates/sl-daemon/src/http.rscrates/sl-daemon/src/etl.rs
crates/sl-viewer/**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
crates/sl-viewer/**/*.{rs,toml}: Thesl-viewercrate uses Dioxus 0.6; use the Dioxus CLI/toolchain for desktop development and bundling.
Usecargo check -p sl-vieweras the fast inner-loop check for viewer changes.
Files:
crates/sl-viewer/src/memory_tab.rscrates/sl-viewer/src/app.rs
crates/sl-viewer/**/*
📄 CodeRabbit inference engine (AGENTS.md)
When packaging the macOS viewer, account for the documented Electrobun/Dioxus code-signing requirements.
Files:
crates/sl-viewer/src/memory_tab.rscrates/sl-viewer/src/app.rs
🪛 PSScriptAnalyzer (1.25.0)
scripts/install.ps1
[warning] 91-93: Empty catch block is used. Please use Write-Error or throw statements in catch blocks.
(PSAvoidUsingEmptyCatchBlock)
[warning] Missing BOM encoding for non-ASCII encoded file 'install.ps1'
(PSUseBOMForUnicodeEncodedFile)
🪛 zizmor (1.28.0)
.github/workflows/release.yml
[info] 125-125: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 495-495: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 566-566: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 566-566: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 586-586: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 586-586: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🔇 Additional comments (7)
.github/workflows/release.yml (1)
513-520: LGTM!scripts/install.sh (1)
117-135: LGTM!Also applies to: 178-178
crates/sl-daemon/src/http.rs (2)
285-287: LGTM!
2156-2177: 📐 Maintainability & Code QualityRun the required daemon validation commands.
Before merge, run
cargo test --manifest-path crates/sl-daemon/Cargo.tomlas the fast inner-loop check. Then run the locked build, all-features test suite, Clippy, and rustfmt checks with the Rust toolchain pinned inrust-toolchain.toml.As per coding guidelines,
crates/sl-daemon/**/*.{rs,toml}changes require these validations.Source: Coding guidelines
crates/sl-viewer/src/app.rs (1)
3-3: LGTM!Also applies to: 19-19, 218-218
crates/sl-daemon/src/archive.rs (1)
403-426: 📐 Maintainability & Code QualityRun the prescribed daemon validation for both changed modules.
crates/sl-daemon/src/archive.rs#L403-L426: run the required daemon and workspace validation after the restore-bound changes.crates/sl-daemon/src/etl.rs#L234-L261: run the required daemon and workspace validation after the transcript-bound changes.Run
cargo test --manifest-path crates/sl-daemon/Cargo.toml, then run the pinned-toolchain locked build, all-features test suite, Clippy, and rustfmt checks.As per coding guidelines,
crates/sl-daemon/**/*.{rs,toml}changes require thesl-daemonfast test command and Rust workspace validation with locked build, all-features tests, Clippy, and rustfmt checks.Source: Coding guidelines
crates/sl-daemon/src/etl.rs (1)
86-87: 🩺 Stability & AvailabilityVerify the size-cap implementation in
crates/sl-daemon/src/etl.rs.The current evidence is insufficient:
metadata().len()only bounds compressed on-disk size for.jsonl.zstinputs, and a separate metadata check does not prevent the file from growing after the check and beforeread_sessionsstarts. Confirm howCodexDir::loaddecompresses / buffers these inputs before deciding whether bounded decompression or a bounded reader is needed.
| # `version` (no leading v): matches the installer naming and the | ||
| # release body below. Using the raw tag here split naming between | ||
| # archives (v-prefixed) and installers (no-v) and broke install.sh. | ||
| VER="${{ steps.version.outputs.version }}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pass workflow values through environment variables before Bash uses them.
Direct GitHub expression interpolation can modify generated shell source when a tag contains shell metacharacters. Bind each value in env and use normal quoted shell expansion.
.github/workflows/release.yml#L122-L125: bind the normalized version throughenvbefore assigningVER..github/workflows/release.yml#L490-L497: bindgithub.ref_nameasTAGthroughenvbefore deriving the version..github/workflows/release.yml#L563-L566: bind the release tag asRELEASE_TAGthroughenvbeforegh release download..github/workflows/release.yml#L586-L586: use the same environment-bound release tag forgh release upload.
🧰 Tools
🪛 zizmor (1.28.0)
[info] 125-125: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
📍 Affects 1 file
.github/workflows/release.yml#L122-L125(this comment).github/workflows/release.yml#L490-L497.github/workflows/release.yml#L563-L566.github/workflows/release.yml#L586-L586
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release.yml around lines 122 - 125, Pass workflow values
through step-level environment variables before Bash uses them: in
.github/workflows/release.yml lines 122-125 bind the normalized version in env
and use quoted shell expansion for VER; at lines 490-497 bind github.ref_name as
TAG and derive the version from it; at lines 563-566 bind the release tag as
RELEASE_TAG for gh release download; and at line 586 reuse the same
environment-bound RELEASE_TAG for gh release upload.
Source: Linters/SAST tools
| fn gunzip_bytes(data: &[u8], path: &Path) -> Result<Vec<u8>, ArchiveError> { | ||
| gunzip_bounded(data, path, MAX_RESTORE_BYTES) | ||
| } | ||
|
|
||
| fn gunzip_bounded(data: &[u8], path: &Path, max_bytes: usize) -> Result<Vec<u8>, ArchiveError> { | ||
| let mut decoder = GzDecoder::new(data); | ||
| let mut out = Vec::new(); | ||
| decoder | ||
| let read = decoder | ||
| .by_ref() | ||
| .take(max_bytes as u64 + 1) | ||
| .read_to_end(&mut out) | ||
| .map_err(|e| ArchiveError::Io { path: path.to_path_buf(), source: e })?; | ||
| if read > max_bytes { | ||
| return Err(ArchiveError::Io { | ||
| path: path.to_path_buf(), | ||
| source: std::io::Error::other(format!( | ||
| "archive decompresses beyond the {max_bytes} byte restore cap; refusing to buffer it" | ||
| )), | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)crates/sl-daemon/src/archive\.rs$|Cargo\.toml$|rust-toolchain\.toml'
echo "== archive outline =="
ast-grep outline crates/sl-daemon/src/archive.rs || true
echo "== relevant archive excerpts =="
sed -n '120,175p;226,266p;340,430p' crates/sl-daemon/src/archive.rs | cat -n
echo "== references to MAX_RESTORE_BYTES and gunzip =="
rg -n "MAX_RESTORE_BYTES|gunzip_bounded|gunzip_bytes|restore_bundle|read_to_end|fs::read|File::open" crates/sl-daemon/src/archive.rs crates/sl-daemon -SRepository: KooshaPari/SessionLedger
Length of output: 17092
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== sl-daemon Cargo deps =="
sed -n '1,220p' crates/sl-daemon/Cargo.toml | cat -n
echo "== rust-toolchain =="
cat rust-toolchain.toml
echo "== imports in archive.rs =="
sed -n '1,30p' crates/sl-daemon/src/archive.rs | cat -n
echo "== deterministic source verifier: calls before gunzip bounded =="
python3 - <<'PY'
from pathlib import Path
src = Path('crates/sl-daemon/src/archive.rs').read_text()
restore = src[src.index('pub fn restore_bundle'):src.index('pub fn find_archive_path')]
gunzip = src[src.index('fn gunzip_bounded'):src.index('// ---------------------------------------------------------------------------\n// Tests')]
print('restore calls std::fs::read before decompressed limit call:', 'fs::read(archive_path)' in restore and 'gunzip_bytes(&compressed' in restore)
print('gunzip_bounded input slice starts before GzDecoder:', 'GzDecoder::new(data)' in gunzip)
print('gunzip_bounded output cap takes max_bytes + 1:', 'take(max_bytes as u64 + 1)' in gunzip)
PYRepository: KooshaPari/SessionLedger
Length of output: 6456
Stream the archive input before decompression.
restore_bundle reads the full .json.gz file into memory, then gunzip_bounded applies the cap only while decompressing. A multi-gigabyte compressed archive can allocate the complete input before this cap runs.
Read from File into a streaming gzip decoder, apply an input-size policy if full I/O must also be bounded, and add a regression test that restores a large compressed input without allocating the complete archive.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/sl-daemon/src/archive.rs` around lines 239 - 258, The restore flow in
restore_bundle must stop loading the entire .json.gz input before gunzip_bounded
runs. Open the archive as a File and stream it through a gzip decoder,
preserving the MAX_RESTORE_BYTES decompressed-output cap; if the input itself
requires bounding, enforce that while reading rather than after buffering. Add a
regression test covering a large compressed archive and verifying restoration
does not allocate the complete compressed input.
| #[test] | ||
| fn size_cap_env_parsing_is_single_sequenced_check() { | ||
| // One test, no parallel env races: valid override, garbage, and | ||
| // sub-minimum values are handled deterministically. | ||
| std::env::set_var("SL_ETL_MAX_FILE_BYTES", (2 * 1024 * 1024).to_string()); | ||
| assert_eq!(max_etl_file_bytes(), 2 * 1024 * 1024); | ||
| std::env::set_var("SL_ETL_MAX_FILE_BYTES", "not-a-number"); | ||
| assert_eq!(max_etl_file_bytes(), DEFAULT_ETL_MAX_FILE_BYTES); | ||
| std::env::set_var("SL_ETL_MAX_FILE_BYTES", "0"); | ||
| assert_eq!(max_etl_file_bytes(), DEFAULT_ETL_MAX_FILE_BYTES); | ||
| std::env::remove_var("SL_ETL_MAX_FILE_BYTES"); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '/^\[package\]/,/^\[/p' crates/sl-daemon/Cargo.toml | rg '^edition\s*='
rg -n -C 2 --type rust 'std::env::(set_var|remove_var)|max_etl_file_bytes\s*\(' crates/sl-daemonRepository: KooshaPari/SessionLedger
Length of output: 5607
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== etl.rs outline/section =="
wc -l crates/sl-daemon/src/etl.rs
sed -n '1,55p' crates/sl-daemon/src/etl.rs
sed -n '250,280p' crates/sl-daemon/src/etl.rs
echo
echo "== workspace MSRV/toolchain files =="
for f in rust-toolchain.toml Cargo.toml; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,120p' "$f"
fi
done
echo
echo "== parallel test directives =="
rg -n 'parallel|#[should_panic|test|cargo test|rustc_test|doctest' crates/sl-daemon/src/etl.rsRepository: KooshaPari/SessionLedger
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== etl.rs relevant lines =="
sed -n '1,50p' crates/sl-daemon/src/etl.rs
sed -n '250,280p' crates/sl-daemon/src/etl.rs
echo
echo "== workspace MSRV/toolchain files =="
for f in rust-toolchain.toml Cargo.toml; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,140p' "$f"
fi
doneRepository: KooshaPari/SessionLedger
Length of output: 7352
Avoid mutating SL_ETL_MAX_FILE_BYTES in this unit test.
crates/sl-daemon/Cargo.toml is Rust 2021, so this is not an unsafe-API issue. The test still changes the process-global environment, which can interfere with other tests. Parse the value from a Option<&str> helper instead and call that helper with literal inputs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/sl-daemon/src/etl.rs` around lines 263 - 274, Refactor the size-cap
parsing flow around max_etl_file_bytes into an Option<&str>-accepting helper,
keeping the existing valid, invalid, sub-minimum, and default behaviors. Update
size_cap_env_parsing_is_single_sequenced_check to call the helper with literal
Some values and None instead of mutating SL_ETL_MAX_FILE_BYTES, and remove the
environment-variable setup and cleanup from the test.
Source: Coding guidelines
| /// | ||
| /// The map is bounded: unique `Idempotency-Key` values arriving at a long-running | ||
| /// daemon used to accumulate without eviction. When the cap is hit the cache is | ||
| /// reset (idempotency replay is an optimization, not a durability contract — | ||
| /// stale keys are re-validated against the body hash on the next request). |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Evict one entry instead of clearing the entire cache.
At Line 313, the 4,097th unique key removes all 4,096 stored body hashes and leaves only the new key. After this reset, ingest_bundle treats a previously seen key as absent. If the new body validates, it accepts the request instead of returning idempotency_conflict. This does not re-validate the key against its previous body hash as stated at Line 279.
Evict the oldest or least-recently-used entry instead. Update idempotency_cache_remains_bounded to verify the intended retained and evicted keys. The current entries.len() <= ... assertion also passes when only one entry remains.
Also applies to: 313-316, 2156-2177
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/sl-daemon/src/http.rs` around lines 275 - 279, Update the idempotency
cache capacity handling in the request ingestion logic to evict only one oldest
or least-recently-used entry when full, then retain the new key instead of
clearing the cache. Revise idempotency_cache_remains_bounded to assert that the
expected retained keys remain and the evicted key is absent, rather than
checking only entries.len().
| fn compile_bundles_from_sessions(sessions: &[Session]) -> Vec<ContinuationBundle> { | ||
| if sessions.is_empty() { | ||
| return sample_bundles(); | ||
| } | ||
| sessions.iter().map(session_ledger::distill::compile).collect() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not replace a failed corpus load with demo bundles.
Line 917 returns sample_bundles() for every empty session list. A load_sessions failure also leaves the list empty. The Bundles and Timeline views then show fabricated bundles instead of the load failure. Preserve the load outcome or pass an explicit demo-mode flag. Use sample bundles only after a confirmed successful empty corpus.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/sl-viewer/src/app.rs` around lines 915 - 919, Update
compile_bundles_from_sessions so an empty sessions slice is not treated as a
successful empty corpus: preserve and propagate the load outcome from
load_sessions, or pass an explicit demo-mode/success flag, and call
sample_bundles only when loading succeeded with no sessions. Ensure Bundles and
Timeline views retain the load failure instead of displaying fabricated bundles.
| /// The compiled-bundles tab — the original sidebar + detail panel. | ||
| #[component] | ||
| fn BundlesTab() -> Element { | ||
| let session_signal = use_context::<SessionContext>().0; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- rust-toolchain.toml ---'
sed -n '1,160p' rust-toolchain.toml
printf '%s\n' '--- Dioxus and MSRV declarations ---'
fd -HI '^Cargo\.toml$' . -x rg -n -C2 '^(rust-version|dioxus)\s*=|dioxus' {}
printf '%s\n' '--- Locked Dioxus packages ---'
fd -HI '^Cargo\.lock$' . -x rg -n -A5 -B2 '^name = "dioxus(-[a-z-]+)?"$' {}Repository: KooshaPari/SessionLedger
Length of output: 1753
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate Cargo.toml files ---'
fd -HI '^Cargo\.toml$' . -maxdepth 4
printf '%s\n' '--- Dioxus and MSRV declarations in found Cargo.toml files ---'
fd -HI '^Cargo\.toml$' . -maxdepth 4 -x sh -c 'echo "--- {} ---"; rg -n -C2 "^(rust-version|dioxus|dioxus-desktop|dioxus-cli|dioxus-live|dioxus-hot-reload|dioxus-server|dioxus-use-signal)\s*=|dioxus[[:space:]]*=|name = \"dioxus\"|name = \"dioxus-desktop\"" "$1" || true' sh {}
printf '%s\n' '--- root/package manifests containing sl-viewer references ---'
rg -n -C2 'sl-viewer|dioxus=\"0\.7\"|dioxus = "0\.7"|dioxus' --glob 'Cargo.toml' .Repository: KooshaPari/SessionLedger
Length of output: 358
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- candidate Cargo.toml files ---'
find . -maxdepth 4 -type f -iname 'Cargo.toml' -print | sort
printf '%s\n' '--- Dioxus and MSRV declarations in found Cargo.toml files ---'
find . -maxdepth 4 -type f -iname 'Cargo.toml' -print | sort | while IFS= read -r f; do
echo "--- $f ---"
rg -n -C2 '^(rust-version|dioxus|dioxus-desktop|dioxus-cli|dioxus-live|dioxus-hot-reload|dioxus-server|dioxus-use-signal)\s*=|dioxus[[:space:]]*=|name = \"dioxus\"|name = \"dioxus-desktop\"' "$f" || true
done
printf '%s\n' '--- root/package manifests containing sl-viewer references ---'
find . -maxdepth 4 -type f -iname 'Cargo.toml' -print | sort | while IFS= read -r f; do
rg -n -C2 'sl-viewer|dioxus=\"0\.7\"|dioxus = "0\.7"|dioxus' "$f" || true
doneRepository: KooshaPari/SessionLedger
Length of output: 2625
Update crates/sl-viewer/Cargo.toml to Dioxus 0.6.
crates/sl-viewer/Cargo.toml declares dioxus = "0.7" while the sl-viewer crate guideline requires Dioxus 0.6. The Rust toolchain and workspace MSRV are compatible with the stated Rust 1.85 requirement.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/sl-viewer/src/app.rs` around lines 922 - 925, Update the sl-viewer
crate’s Dioxus dependency in Cargo.toml from version 0.7 to 0.6, preserving the
existing dependency configuration and leaving the BundlesTab implementation
unchanged.
Source: Coding guidelines
| let mut pages = use_signal(Vec::<MemoryWikiPage>::new); | ||
| let mut selected_idx: Signal<Option<usize>> = use_signal(|| None); | ||
|
|
||
| // Reactive: re-derive the wiki whenever the loaded corpus changes so a | ||
| // late async session load is reflected instead of leaving an empty page. | ||
| use_effect(move || { | ||
| pages.set(all_wiki_pages_from_sessions(&ctx.0.read())); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve selection by session_id, not by index.
Line 68 replaces the page list after each corpus update. If session ordering changes, selected_idx can point to a different MemoryWikiPage. Store the selected session_id and resolve the selected page from that ID after regeneration. Clear the selection only if that session no longer exists.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/sl-viewer/src/memory_tab.rs` around lines 62 - 69, Update the
selection state around selected_idx to preserve the selected page by session_id
rather than list index when use_effect regenerates pages. After
all_wiki_pages_from_sessions updates the list, resolve the selected page using
its stored session_id and update the index accordingly; clear the selection only
when that session_id is no longer present.
| # appimagetool refuses to build (exit 1) when the desktop entry references an | ||
| # icon that is not present in the AppDir. Ship the brand icon so packaging | ||
| # succeeds instead of failing silently under continue-on-error. | ||
| ICON_SRC="$ROOT/assets/icons/sessionledger.iconset/icon_256x256.png" | ||
| if [ -f "$ICON_SRC" ]; then | ||
| install -m 0644 "$ICON_SRC" "$APPDIR/sessionledger.png" | ||
| else | ||
| echo "warning: brand icon not found at $ICON_SRC; AppImage will be built without an icon." >&2 | ||
| fi | ||
|
|
||
| cat >"$APPDIR/sessionledger.desktop" <<EOF | ||
| [Desktop Entry] | ||
| Type=Application | ||
| Name=SessionLedger | ||
| Comment=View SessionLedger session bundles | ||
| Exec=sl-viewer | ||
| Icon=sessionledger |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Do not reference an icon that was not installed.
If ICON_SRC is absent, line 34 reports that packaging continues without an icon. Line 43 still requires sessionledger, so appimagetool can fail and no AppImage is produced. Emit Icon=sessionledger only after the icon installation succeeds, or fail immediately when the icon is required.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packaging/linux/package-appimage.sh` around lines 27 - 43, Update the icon
handling around ICON_SRC and the generated sessionledger.desktop entry so
Icon=sessionledger is emitted only when the icon installation succeeds. If
ICON_SRC is missing, either omit the Icon entry from the desktop file or fail
packaging immediately; do not continue with a desktop reference to an
uninstalled icon.
| # Release archives historically used the tag verbatim (sl-viewer-v0.1.1-...) | ||
| # and later dropped the leading `v` (sl-viewer-0.1.1-...). Probe both spellings | ||
| # so the installer works against every published release. | ||
| $assetVersion = $Version.TrimStart("v") | ||
| $archive = $null | ||
| foreach ($candidate in @("sl-viewer-$Version-$Target.zip", "sl-viewer-$assetVersion-$Target.zip")) { | ||
| try { | ||
| Invoke-WebRequest -Uri "$baseUrl/$candidate" -Method Head -UseBasicParsing -ErrorAction Stop | Out-Null | ||
| $archive = $candidate | ||
| break | ||
| } catch { | ||
| # Not this spelling; try the next one. | ||
| } | ||
| } | ||
| if (-not $archive) { | ||
| throw "No sl-viewer archive found for $Version ($Target). Tried: sl-viewer-$Version-$Target.zip, sl-viewer-$assetVersion-$Target.zip" | ||
| } | ||
| $baseUrl = "https://github.com/$Repo/releases/download/$Version" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Initialize $baseUrl before the archive probe.
Lines 86-96 use $baseUrl before line 98 assigns it. Each HEAD request therefore uses an invalid relative URI, gets caught, and leaves $archive null. Move the $baseUrl assignment before the loop.
Proposed fix
+$baseUrl = "https://github.com/$Repo/releases/download/$Version"
$assetVersion = $Version.TrimStart("v")
$archive = $null
foreach ($candidate in @("sl-viewer-$Version-$Target.zip", "sl-viewer-$assetVersion-$Target.zip")) {
...
}
-$baseUrl = "https://github.com/$Repo/releases/download/$Version"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Release archives historically used the tag verbatim (sl-viewer-v0.1.1-...) | |
| # and later dropped the leading `v` (sl-viewer-0.1.1-...). Probe both spellings | |
| # so the installer works against every published release. | |
| $assetVersion = $Version.TrimStart("v") | |
| $archive = $null | |
| foreach ($candidate in @("sl-viewer-$Version-$Target.zip", "sl-viewer-$assetVersion-$Target.zip")) { | |
| try { | |
| Invoke-WebRequest -Uri "$baseUrl/$candidate" -Method Head -UseBasicParsing -ErrorAction Stop | Out-Null | |
| $archive = $candidate | |
| break | |
| } catch { | |
| # Not this spelling; try the next one. | |
| } | |
| } | |
| if (-not $archive) { | |
| throw "No sl-viewer archive found for $Version ($Target). Tried: sl-viewer-$Version-$Target.zip, sl-viewer-$assetVersion-$Target.zip" | |
| } | |
| $baseUrl = "https://github.com/$Repo/releases/download/$Version" | |
| # Release archives historically used the tag verbatim (sl-viewer-v0.1.1-...) | |
| # and later dropped the leading `v` (sl-viewer-0.1.1-...). Probe both spellings | |
| # so the installer works against every published release. | |
| $baseUrl = "https://github.com/$Repo/releases/download/$Version" | |
| $assetVersion = $Version.TrimStart("v") | |
| $archive = $null | |
| foreach ($candidate in @("sl-viewer-$Version-$Target.zip", "sl-viewer-$assetVersion-$Target.zip")) { | |
| try { | |
| Invoke-WebRequest -Uri "$baseUrl/$candidate" -Method Head -UseBasicParsing -ErrorAction Stop | Out-Null | |
| $archive = $candidate | |
| break | |
| } catch { | |
| # Not this spelling; try the next one. | |
| } | |
| } | |
| if (-not $archive) { | |
| throw "No sl-viewer archive found for $Version ($Target). Tried: sl-viewer-$Version-$Target.zip, sl-viewer-$assetVersion-$Target.zip" | |
| } |
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] 91-93: Empty catch block is used. Please use Write-Error or throw statements in catch blocks.
(PSAvoidUsingEmptyCatchBlock)
[warning] Missing BOM encoding for non-ASCII encoded file 'install.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/install.ps1` around lines 81 - 98, Move the `$baseUrl` assignment
above the archive-probing `foreach` loop so the HEAD requests in the `$archive`
resolution flow use the release download URL. Keep the candidate probing and
missing-archive error behavior unchanged.
|
Closed as stale. CI: browser e2e (axe/responsive/visual) FAIL + prepare FAIL — same failure pattern as PR #393. These appear to be pre-existing CI infrastructure issues (e2e test setup) rather than code regressions. Real value in this PR (memory safety bounds, Sigstore signing templates, real-data wire-up helpers) is already on main via separate commits. Recommend re-cutting as smaller, individually-testable PRs against current main, OR fixing the underlying e2e CI infrastructure first. |
User description
Merge-ready: all changes locally verified (daemon 178/178 tests, viewer 57/57, clippy clean).
CodeAnt-AI Description
Protect ingestion memory, load real viewer bundles, and repair release installation
What Changed
Impact
✅ Lower memory during large transcript ingestion✅ Safer archive restores against decompression bombs✅ Real session data in viewer bundles and timeline✅ Installers work across existing release archive names💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.