fix(daemon,viewer,release): bound ingest memory; wire real bundles; install + AppImage/sigstore fixes - #393
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. |
📝 WalkthroughSummaryThis PR bounds daemon ingest and decompression memory, limits the idempotency cache, connects viewer tabs to loaded session bundles, and fixes release, installer, AppImage, and Sigstore behavior. The changes should improve memory safety and release reliability. Merge after confirming the required Rust formatting, clippy checks, and workspace tests pass. Must Fix
Should Fix
Consider
Approve / Request ChangesApprove with notes, subject to successful formatting, clippy, test, and packaging validation. WalkthroughThe PR adds resource limits for daemon operations, makes viewer data react to loaded sessions, and updates release workflows, installers, and AppImage packaging for revised archive naming and icon handling. ChangesDaemon resource guards
Viewer session reactivity
Release distribution updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SessionContext
participant BundlesTab
participant ContinuationBundle
participant MemoryWiki
SessionContext->>BundlesTab: provide loaded sessions
BundlesTab->>ContinuationBundle: compile sessions
ContinuationBundle-->>BundlesTab: return compiled bundles
SessionContext->>MemoryWiki: provide updated sessions
MemoryWiki->>MemoryWiki: refresh wiki pages
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
📝 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 |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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: Update the release workflow steps that consume
github.ref_name, steps.version.outputs.version, or repository metadata so
tag-derived values are passed through step-level environment variables such as
RELEASE_TAG, RELEASE_REPOSITORY, and RELEASE_VERSION. Replace direct
GitHub-expression interpolation in shell and PowerShell commands with quoted
environment-variable references, including the VER assignment near the release
archive/installer naming logic, while preserving the existing version semantics.
In `@crates/sl-daemon/src/archive.rs`:
- Around line 239-240: Update restore_bundle and the gunzip_bytes/gunzip_bounded
flow to use a bounded reader instead of fs::read, enforcing limits on both
compressed input and decompressed output before buffering the restored payload.
Preserve existing ArchiveError handling, and add an integration test covering an
oversized incompressible archive.
In `@crates/sl-daemon/src/etl.rs`:
- Around line 86-87: Update the ETL flow around enforce_size_cap and
CodexDir::load to enforce SL_ETL_MAX_FILE_BYTES against decoded .jsonl.zst
content, not only compressed file metadata; reject compressed inputs whose
decoded size exceeds the cap while preserving acceptance below the cap, and add
coverage for both cases.
- Around line 264-273: Refactor the `max_etl_file_bytes` environment parsing
into a pure helper that accepts an optional string value and applies the
existing valid, invalid, and minimum-value rules. Update
`size_cap_env_parsing_is_single_sequenced_check` to call this helper with test
values, removing all `set_var` and `remove_var` process-global environment
mutations while preserving the current expected results.
In `@crates/sl-viewer/src/app.rs`:
- Around line 915-919: Update compile_bundles_from_sessions and the shared
SessionContext flow so a failed corpus load is preserved as an error or failed
outcome rather than represented by an empty sessions slice. Restrict
sample_bundles() to explicit demo mode or a successfully loaded empty corpus,
and ensure Bundles and Timeline render the load error consistently instead of
demo bundles.
In `@packaging/linux/package-appimage.sh`:
- Around line 30-43: Update the icon handling around ICON_SRC and the generated
sessionledger.desktop entry so the AppDir always has a valid icon reference
before appimagetool runs. When the source icon is missing, either install a
valid fallback icon and reference it or fail explicitly before writing/invoking
appimagetool; do not leave Icon=sessionledger pointing to a nonexistent file.
In `@scripts/install.ps1`:
- Around line 86-93: Update the archive probe fallback in scripts/install.ps1
lines 86-93 so the catch block continues only when the Invoke-WebRequest failure
represents HTTP 404; rethrow or report all other failures before trying another
candidate. Update scripts/install.sh lines 123-129 to inspect the probe’s HTTP
status and retry the alternate filename only for 404, while propagating
transport, authorization, rate-limit, and server errors.
- Around line 81-98: Move the `$baseUrl` assignment before the archive-candidate
loop so `Invoke-WebRequest` in the probing logic uses the release download URL.
Keep the existing candidate order, fallback behavior, and final
archive-not-found error 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: 97388728-9df2-4490-8403-122fc0c02211
📒 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. (4)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Mergify Merge Queue
- GitHub Check: Kilo Code Review
- GitHub Check: Summary
🧰 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-viewer/src/memory_tab.rscrates/sl-daemon/src/archive.rscrates/sl-daemon/src/http.rscrates/sl-viewer/src/app.rscrates/sl-daemon/src/etl.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Fix Clippy warnings; do not add
#[allow]unless it includes a tracking-issue comment.
Files:
crates/sl-viewer/src/memory_tab.rscrates/sl-daemon/src/archive.rscrates/sl-daemon/src/http.rscrates/sl-viewer/src/app.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
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
🪛 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 (4)
.github/workflows/release.yml (1)
513-520: LGTM!scripts/install.sh (1)
178-178: LGTM!crates/sl-viewer/src/app.rs (1)
3-3: LGTM!Also applies to: 19-19, 218-218, 355-357
crates/sl-viewer/src/memory_tab.rs (1)
62-69: 📐 Maintainability & Code QualityVerify the Dioxus version and required viewer checks.
The supplied library context identifies Dioxus 0.7. The applicable guideline requires Dioxus 0.6. Confirm the version in the manifests and lockfile before merging this
use_effectchange. Then run the pinned-toolchain, locked validation, includingcargo check -p sl-viewer, the all-features test suite, Clippy, and rustfmt.As per coding guidelines, "
crates/sl-viewer/**/*.{rs,toml}: Thesl-viewercrate uses Dioxus 0.6" and "**/*.{rs,toml}: Use the Rust toolchain pinned inrust-toolchain.toml; the workspace MSRV is Rust 1.85."Source: Coding guidelines
| # `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 | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
zizmor .github/workflows/release.ymlRepository: KooshaPari/SessionLedger
Length of output: 549
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Repo files around workflow:\n'
wc -l .github/workflows/release.yml
printf '\nLine 110-135:\n'
sed -n '110,135p' .github/workflows/release.yml | nl -ba -v110
printf '\nLine 480-502:\n'
sed -n '480,502p' .github/workflows/release.yml | nl -ba -v480
printf '\nLine 556-590:\n'
sed -n '556,590p' .github/workflows/release.yml | nl -ba -v556
printf '\nAll github_ref_name usages:\n'
rg -n '\$\{\{\s*github\.ref_name\s*\}\}' .github/workflows/release.yml || true
printf '\nAll steps.version.outputs.version usages:\n'
rg -n '\$\{\{\s*steps\.version\.outputs\.version\s*\}\}' .github/workflows/release.yml || true
printf '\nAll github.repository usages:\n'
rg -n '\$\{\{\s*github\.repository\s*\}\}' .github/workflows/release.yml || trueRepository: KooshaPari/SessionLedger
Length of output: 279
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Workflow line count:\n'
wc -l .github/workflows/release.yml
printf '\nLine 110-135:\n'
awk 'NR>=110 && NR<=135 {printf "%4d %s\n", NR, $0}' .github/workflows/release.yml
printf '\nLine 480-502:\n'
awk 'NR>=480 && NR<=502 {printf "%4d %s\n", NR, $0}' .github/workflows/release.yml
printf '\nLine 556-590:\n'
awk 'NR>=556 && NR<=590 {printf "%4d %s\n", NR, $0}' .github/workflows/release.yml
printf '\nAll github.ref_name usages:\n'
rg -n '\$\{\{\s*github\.ref_name\s*\}\}' .github/workflows/release.yml || true
printf '\nAll steps.version.outputs.version usages:\n'
rg -n '\$\{\{\s*steps\.version\.outputs\.version\s*\}\}' .github/workflows/release.yml || true
printf '\nAll github.repository usages:\n'
rg -n '\$\{\{\s*github\.repository\s*\}\}' .github/workflows/release.yml || trueRepository: KooshaPari/SessionLedger
Length of output: 6748
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Lines 80-95:\n'
awk 'NR>=80 && NR<=95 {printf "%4d %s\n", NR, $0}' .github/workflows/release.yml
printf '\nLines 160-245:\n'
awk 'NR>=160 && NR<=245 {printf "%4d %s\n", NR, $0}' .github/workflows/release.yml
printf '\nLines 700-730:\n'
awk 'NR>=700 && NR<=730 {printf "%4d %s\n", NR, $0}' .github/workflows/release.yml
printf '\nWorkflow jobs with permissions:\n'
awk '/^[[:space:]]*permissions:/,/^[[:space:]]*[[:alpha:]][[:alnum:]_-]*:/ {print NR": "$0}' .github/workflows/release.yml | head -240Repository: KooshaPari/SessionLedger
Length of output: 7716
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Lines 1-85:\n'
awk 'NR>=1 && NR<=85 {printf "%4d %s\n", NR, $0}' .github/workflows/release.yml
printf '\nLines 410-435:\n'
awk 'NR>=410 && NR<=435 {printf "%4d %s\n", NR, $0}' .github/workflows/release.yml
printf '\nLines 548-568:\n'
awk 'NR>=548 && NR<=568 {printf "%4d %s\n", NR, $0}' .github/workflows/release.yml
printf '\nLines 600-610:\n'
awk 'NR>=600 && NR<=610 {printf "%4d %s\n", NR, $0}' .github/workflows/release.ymlRepository: KooshaPari/SessionLedger
Length of output: 6016
🌐 Web query:
zizmor github-expression-interpolation rule
💡 Result:
The zizmor tool does not have a rule named github-expression-interpolation. The vulnerability or issue commonly associated with interpolating GitHub expressions into shell commands is identified by the template-injection audit rule [1][2][3]. The template-injection rule in zizmor detects when GitHub Actions expressions—such as ${{ github.event.issue.title }}—are interpolated directly into run: shell script blocks [1][2]. This practice is a security risk because it allows an attacker to inject shell metacharacters or commands into the workflow, potentially leading to arbitrary code execution [1][2]. To remediate findings from the template-injection rule, you should: 1. Move the expression out of the run: shell script block and into the step's env: block [1][2]. 2. Assign the expression to a named environment variable (e.g., PR_TITLE: ${{ github.event.pull_request.title }}) [1][2]. 3. Reference the environment variable inside the run: block using standard shell syntax (e.g., $PR_TITLE) [1][2]. By moving the value into an environment variable, the shell treats the input as data rather than executable code, thereby mitigating the injection risk [1][2].
Citations:
- 1: Static Analysis Report - 2026-03-26 github/gh-aw#23157
- 2: Static Analysis Report - 2026-04-04 github/gh-aw#24570
- 3: https://docs.zizmor.sh/audits/
Pass tag-derived values through env, not shell interpolation.
github.ref_name and steps.version.outputs.version are interpolated directly into shell and PowerShell scripts, including steps in release/signing jobs with write and OIDC permissions. Define step-level environment variables, such as RELEASE_TAG, RELEASE_REPOSITORY, and RELEASE_VERSION, and reference the quoted variables inside the scripts.
Suggested pattern
+ env:
+ RELEASE_VERSION: ${{ steps.version.outputs.version }}
run: |
- VER="${{ steps.version.outputs.version }}"
+ VER="$RELEASE_VERSION"
+ env:
+ RELEASE_TAG: ${{ github.ref_name }}
run: |
- tag="${{ github.ref_name }}"
+ tag="$RELEASE_TAG"
- run: gh release download "${{ github.ref_name }}" --repo "${{ github.repository }}" --pattern SHA256SUMS
+ run: gh release download "$RELEASE_TAG" --repo "$RELEASE_REPOSITORY" --pattern SHA256SUMS🧰 Tools
🪛 zizmor (1.28.0)
[info] 125-125: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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, Update the release
workflow steps that consume github.ref_name, steps.version.outputs.version, or
repository metadata so tag-derived values are passed through step-level
environment variables such as RELEASE_TAG, RELEASE_REPOSITORY, and
RELEASE_VERSION. Replace direct GitHub-expression interpolation in shell and
PowerShell commands with quoted environment-variable references, including the
VER assignment near the release archive/installer naming logic, while preserving
the existing version semantics.
Source: Linters/SAST tools
| fn gunzip_bytes(data: &[u8], path: &Path) -> Result<Vec<u8>, ArchiveError> { | ||
| gunzip_bounded(data, path, MAX_RESTORE_BYTES) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
ast-grep outline crates/sl-daemon/src/archive.rs --items all --type function
rg -n -C 8 'fn restore_bundle|fs::read|fn gunzip_(bytes|bounded)|GzDecoder' crates/sl-daemon/src/archive.rsRepository: KooshaPari/SessionLedger
Length of output: 7922
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('crates/sl-daemon/src/archive.rs')
text = p.read_text()
restore = text.split('pub fn restore_bundle', 1)[1].split('}')[0]
gunzip_bounded = text.split('fn gunzip_bounded', 1)[1].split('}')[0]
print('restore_bundle reads archive before gunzip_bounded:', 'let compressed = fs::read(archive_path)' in restore and 'gunzip_bytes(&compressed' in restore)
print('gunzip_bounded limits decompression stream via take before read_to_end:', 'take(max_bytes as u64 + 1)' in gunzip_bounded and '.read_to_end(&mut out)' in gunzip_bounded)
print('restoring large incompressible archive keeps two full copies in memory:')
print(' first copy from compressed:', 'let compressed = fs::read(archive_path)' in restore)
print(' second copy as decoded Vec:', 'let mut out = Vec::new()' in gunzip_bounded and '.read_to_end(&mut out)' in gunzip_bounded)
PY
rg -n -C 3 'MAX_RESTORE_BYTES|gunzip_bounded|gunzip_bytes|compressed = fs::read|read_to_end|GzDecoder::new' crates/sl-daemon/src archive.rsRepository: KooshaPari/SessionLedger
Length of output: 4954
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Demonstrate incompressible GZIP streams avoid the take/read-to-end cap if GzDecoder
# is initialized with bytes and then read_to_end is called on the whole stream.
# This is a small standalone FFI probe using zlib, not repository code.
python3 - <<'PY'
import gzip, zlib, io, sys
max_out = 1024
payload = b'x' * (max_out + 100)
data = b'not gzip ' + payload[::-1] # Fallback header plus incompressible 1-byte stream
try:
with gzip.GzipFile(fileobj=io.BytesIO(data), mode='rb') as f:
f.read(max_out + 1)
print('gzip.GzipFile would read beyond cap before EOF after data exhaustion.', file=sys.stderr)
except OSError as e:
print('gzip.GzipFile', type(e).__name__, '-', e)
# Fallback zlib stream: many 1-byte blocks at maximum compression with invalid header.
raw = b'\x08x\x9c' + b'not gzip ' + payload[::-1] # invalid gzip header, incompressible zlib data
rf = zlib.decompressobj()
out = rf.decompress(data)
print('zlib fallback decompressed', len(out), 'bytes for incompressible GZIP-like input before EOF/stop')
PYRepository: KooshaPari/SessionLedger
Length of output: 361
Bound the compressed archive before decompressing.
restore_bundle reads the complete .json.gz file with fs::read before gunzip_bounded can check MAX_RESTORE_BYTES. A large incompressible archive can consume memory while loading the archive before the recovered-size cap applies, and the restored payload is also buffered in memory.
Use a reader-based restore path that limits both compressed input and decompressed output, and add an integration test with an oversized incompressible 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 - 240, Update
restore_bundle and the gunzip_bytes/gunzip_bounded flow to use a bounded reader
instead of fs::read, enforcing limits on both compressed input and decompressed
output before buffering the restored payload. Preserve existing ArchiveError
handling, and add an integration test covering an oversized incompressible
archive.
| enforce_size_cap(jsonl_path)?; | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
ast-grep outline crates/sl-daemon/src/etl.rs --items all --type function
rg -n -C 8 'enforce_size_cap|read_sessions|CodexDir::new|\.load\(' crates/sl-daemon/src/etl.rs
rg -n -C 5 '\bCodexDir\b' .Repository: KooshaPari/SessionLedger
Length of output: 13239
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^src/|crates/sl-daemon/src/etl\.rs|README|Cargo\.toml|rust-toolchain)' || true
echo "== etl top/middle =="
sed -n '1,280p' crates/sl-daemon/src/etl.rs | nl -ba | sed -n '1,280p'
echo "== codex/ingestion outlines =="
ast-grep outline src/ingestion/codex.rs --items all --type function || true
ast-grep outline src/ports.rs --items all --type function || true
echo "== relevant codex/ports snippets =="
sed -n '1,140p' src/ingestion/codex.rs | nl -ba
rg -n -C 6 'struct JsonCorpusSource|impl JsonCorpusSource|fn list|fn load|load_with_report|read_jsonl_sessions|decode|zstd|CodexDir|CorpusSource' src crates/sl-daemon/src/etl.rsRepository: KooshaPari/SessionLedger
Length of output: 1809
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== etl.rs relevant lines =="
awk '{printf "%6d\t%s\n", NR, $0}' crates/sl-daemon/src/etl.rs | sed -n '1,280p'
echo "== codex.rs relevant lines =="
awk '{printf "%6d\t%s\n", NR, $0}' src/ingestion/codex.rs | sed -n '1,160p'
echo "== ports adapters relevant lines =="
awk '{printf "%6d\t%s\n", NR, $0}' src/ports/adapters.rs | sed -n '1,220p'
echo "== json_source.rs relevant lines =="
awk '{printf "%6d\t%s\n", NR, $0}' src/ingestion/json_source.rs | sed -n '1,260p'
echo "== search for size limits and zstd decode =="
rg -n -C 4 'max_file|limit|cap|read_to_string|read_to_end|BufReader|zst$|decode|read_with_limit|ZstdDir|ZstdCompressor|CodexDir|read_jsonl_sessions' src crates/sl-daemon/src/etl.rsRepository: KooshaPari/SessionLedger
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== read_jsonl_sessions and ingest file sizes =="
rg -n -C 6 'pub fn read_jsonl_sessions|fn read_jsonl_sessions|decode_all|decode.*reader|zstd::stream::read|BufReader|read_to_string' src ingestion crates/sl-daemon/src/etl.rs
echo "== inspect read_jsonl_sessions definition =="
awk '{printf "%6d\t%s\n", NR, $0}' src/ingestion/mod.rs | sed -n '1,160p'
echo "== zstd decoder behavior probe =="
python3 - <<'PY'
from pathlib import Path
try:
import zstandard as zstd
except Exception as e:
print(f"python zstandard unavailable: {type(e).__name__}: {e}")
raise SystemExit(0)
payload = b"x" * (2 * 1024 * 1024)
compressed = zstd.ZstdCompressor(9).compress(payload)
print(f"payload_len={len(payload)}")
print(f"compressed_len={len(compressed)}")
print(f"ratio={len(payload)/len(compressed)}")
print(f"metadata_size={Path('dummy.jsonl.zst').with_suffix('dummy.jsonl.zst').open('xb').write(compressed) if 'write' in dir(Path().path) else len(compressed)}")
tmp = Path('/tmp/coderabbit_zst_probe.jsonl.zst')
tmp.write_bytes(compressed)
print(f"file_size={tmp.stat().st_size}")
decoded = zstd.ZstdDecompressor().stream_reader(Bytes=open(BytesIO(compressed), 'rb'), read_size=32000).read()
print(f"decoded_len={len(decoded)}")
PYRepository: KooshaPari/SessionLedger
Length of output: 7910
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import sys
try:
import zstandard as zstd
except Exception as e:
print(f"python zstandard unavailable: {type(e).__name__}: {e}")
raise SystemExit(0)
payload = b"x" * (2 * 1024 * 1024)
compressed = zstd.ZstdCompressor(9).compress(payload)
print(f"encoded_payload_len={len(payload)}")
print(f"compressed_len={len(compressed)}")
print(f"encoding_ratio={len(payload) / len(combined) if 'combined' in dir() else f'{len(payload):.0f}/{len(compressed)}'}")
print(f"payload_over_compressed_at_1_512mb_ratio={len(payload) / len(compressed)}")
if len(compressed) < 512 * 1024 * 1024:
print("compressed_under_512mb_cap=true")
print("decoded_2mb_payload_passes_current_file_metadata_cap=true")
else:
print("compressed_under_512mb_cap=false")
PYRepository: KooshaPari/SessionLedger
Length of output: 241
Enforce the ingest cap on decoded .jsonl.zst data.
enforce_size_cap() only checks metadata.length, while .jsonl.zst files are decoded and held in memory by CodexDir::load() before processing. A small compressed file can decode above SL_ETL_MAX_FILE_BYTES and still bypass the guard.
Add a decoded-size limit for compressed-input paths and cover compressed input below the cap with decoded input above it.
🤖 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 86 - 87, Update the ETL flow around
enforce_size_cap and CodexDir::load to enforce SL_ETL_MAX_FILE_BYTES against
decoded .jsonl.zst content, not only compressed file metadata; reject compressed
inputs whose decoded size exceeds the cap while preserving acceptance below the
cap, and add coverage for both cases.
| 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 | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
fd -a '^Cargo\.toml$|^rust-toolchain\.toml$' . -x sh -c '
echo "===== $1 ====="
rg -n "^(edition|rust-version)\s*=" "$1" || true
' shRepository: KooshaPari/SessionLedger
Length of output: 759
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "===== crates/sl-daemon/Cargo.toml ====="
cat -n crates/sl-daemon/Cargo.toml
echo
echo "===== relevant etl.rs test region ====="
sed -n '230,290p' crates/sl-daemon/src/etl.rs | cat -n
echo
echo "===== test configuration around sl-daemon =="
rg -n 'isolation|--test-threads|default-threads|nextest|all-features|clippy|rustfmt' crates/sl-daemon/Cargo.toml Cargo.toml .github workflows 2>/dev/null || trueRepository: KooshaPari/SessionLedger
Length of output: 154
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
timeout 30 sh -c '
echo "===== crates/sl-daemon/Cargo.toml =="
cat -n crates/sl-daemon/Cargo.toml | sed -n "1,140p"
echo
echo "===== etl.rs test region =="
sed -n "230,290p" crates/sl-daemon/src/etl.rs | cat -n
echo
echo "===== test configuration references =="
rg -n "isolation|--test-threads|default-threads|nextest|all-features|clippy|rustfmt" Cargo.toml .github crates/sl-daemon/Cargo.toml 2>/dev/null || true
'Repository: KooshaPari/SessionLedger
Length of output: 154
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '===== filesystem listing/size =='
find crates/sl-daemon/Cargo.toml crates/sl-daemon/src/etl.rs -maxdepth 0 -type f -print -xargs wc -l 2>/dev/null || true
printf '%s\n' '===== direct cat =='
cat -n crates/sl-daemon/Cargo.toml | head -160
printf '%s\n' '===== sed etl.rs lines =='
sed -n '225,290p' crates/sl-daemon/src/etl.rs | cat -n
printf '%s\n' '===== quick rg =='
rg -n "isolation|--test-threads|default-threads|nextest|all-features|clippy|rustfmt" Cargo.toml crates/sl-daemon/Cargo.toml .github 2>/dev/null || trueRepository: KooshaPari/SessionLedger
Length of output: 154
Keep process-global environment mutations out of the test.
crates/sl-daemon is edition 2021, so set_var / remove_var do not require unsafe here. The remaining issue is test safety: mutate shared process state with tests that cannot run concurrently, or keep the mutation strictly isolated.
Extract the SL_ETL_MAX_FILE_BYTES parsing into a pure helper and call it with test values instead of changing SL_ETL_MAX_FILE_BYTES in process-global state.
🤖 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 264 - 273, Refactor the
`max_etl_file_bytes` environment parsing into a pure helper that accepts an
optional string value and applies the existing valid, invalid, and minimum-value
rules. Update `size_cap_env_parsing_is_single_sequenced_check` to call this
helper with test values, removing all `set_var` and `remove_var` process-global
environment mutations while preserving the current expected results.
Source: Coding guidelines
| 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 | 🏗️ Heavy lift
Do not convert a corpus load failure into demo data.
When the root loader fails, SessionContext remains empty. Line 917 then returns sample_bundles(). The Bundles view can report that no sessions are available while it displays demo bundles. The Timeline view displays demo bundles without the error.
Keep the load outcome in shared state. Use sample_bundles() only for an explicit demo source or a successfully loaded 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 and the shared SessionContext flow so a failed
corpus load is preserved as an error or failed outcome rather than represented
by an empty sessions slice. Restrict sample_bundles() to explicit demo mode or a
successfully loaded empty corpus, and ensure Bundles and Timeline render the
load error consistently instead of demo bundles.
| 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.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
command -v appimagetool >/dev/null || exit 2
tmp_dir="$(mktemp -d)"
trap 'rm -rf "$tmp_dir"' EXIT
mkdir -p "$tmp_dir/AppDir/usr/bin"
printf '#!/bin/sh\nexit 0\n' >"$tmp_dir/AppDir/usr/bin/sl-viewer"
chmod 0755 "$tmp_dir/AppDir/usr/bin/sl-viewer"
printf '#!/bin/sh\nexit 0\n' >"$tmp_dir/AppDir/AppRun"
chmod 0755 "$tmp_dir/AppDir/AppRun"
cat >"$tmp_dir/AppDir/sessionledger.desktop" <<'EOF'
[Desktop Entry]
Type=Application
Name=SessionLedger
Exec=sl-viewer
Icon=sessionledger
EOF
if appimagetool "$tmp_dir/AppDir" "$tmp_dir/out.AppImage"; then
echo "Expected appimagetool to reject the missing icon." >&2
exit 1
fiRepository: KooshaPari/SessionLedger
Length of output: 162
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'appimagetool location: '
command -v appimagetool || printf 'not found\n'
tmp_dir="$(mktemp -d)"
trap 'rm -rf "$tmp_dir"' EXIT
mkdir -p "$tmp_dir/AppDir/usr/bin"
printf '#!/bin/sh\nexit 0\n' >"$tmp_dir/AppDir/usr/bin/sl-viewer"
chmod 0755 "$tmp_dir/AppDir/usr/bin/sl-viewer"
printf '#!/bin/sh\nexit 0\n' >"$tmp_dir/AppDir/AppRun"
chmod 0755 "$tmp_dir/AppDir/AppRun"
cat >"$tmp_dir/AppDir/sessionledger.desktop" <<'EOF'
[Desktop Entry]
Type=Application
Name=SessionLedger
Exec=sl-viewer
Icon=sessionledger
EOF
echo "Running appimagetool with missing icon..."
set +e
appimagetool "$tmp_dir/AppDir" "$tmp_dir/out.AppImage" >/tmp/appimagetool_missing_icon.log 2>&1
status=$?
set -e
printf 'appimagetool exit status: %s\n' "$status"
printf '--- appimagetool output ---\n'
cat /tmp/appimagetool_missing_icon.log
printf '--- AppImage produced ---\n'
ls -l "$tmp_dir/out.AppImage" 2>&1 || true
printf '\nPackage script excerpt:\n'
sed -n '1,80p' packaging/linux/package-appimage.shRepository: KooshaPari/SessionLedger
Length of output: 359
🌐 Web query:
AppImage appimagetool missing Icon desktop file error
💡 Result:
The "missing Icon" or "desktop file" error when using appimagetool occurs because appimagetool enforces strict requirements for the AppDir structure [1][2][3]. To successfully package an AppImage, the AppDir root must contain at least three items: an AppRun executable, a.desktop file, and an icon file [1]. To resolve these errors, ensure your AppDir meets the following criteria: 1. Desktop File Requirements: - The.desktop file must be located at the root of the AppDir [1]. - The Icon= value in the.desktop file must match the filename of the icon (excluding the file extension) [4][1]. For example, if the line is Icon=myapp, there must be a myapp.png or myapp.svg file [1]. - The.desktop file must be present at the root [1]. 2. Icon Requirements: - The icon file must have a basename that matches the Icon= value in the.desktop file [1]. - Place this icon file at the root of the AppDir [4][1]. - Do not include the file extension (e.g.,.png) in the Icon= field of the.desktop file [4]. 3. Best Practices for Desktop Integration: - While appimagetool requires the icon at the root to complete the packaging process [4][1], it is also recommended to place icons in the standardized directory structure (e.g., usr/share/icons/hicolor/.../apps/) [4][5]. This ensures that desktop integration tools (like appimaged or AppImageLauncher) can correctly display the icon in application menus [5][6]. - Ensure your.desktop file passes validation by using the command desktop-file-validate [7]. If you are packaging a CLI-only application and do not want to include an icon, appimagetool still mandates these files by design to maintain consistency across the AppImage ecosystem [2][3]. Users in this situation often create dummy files (e.g., an empty icon file) to satisfy the tool's requirements [2].
Citations:
- 1: https://appimage.readthedocs.io/en/latest/internals.html
- 2: Add an option to disable check for .desktop file for appimagetool (much like the option to ignore AppStream check) AppImage/AppImageKit#1184
- 3: Cannot run appimagetool without an icon AppImage/AppImageKit#954
- 4: [appimagetool] Wrong message/instruction re. icon placement for creating AppImage AppImage/AppImageKit#593
- 5: appimaged does not create a .desktop file with an icon AppImage/AppImageKit#461
- 6: Some troubles with specification AppImage/AppImageKit#955
- 7: https://docs.appimage.org/packaging-guide/manual.html
Fix the iconless AppImage fallback before invoking appimagetool.
When ICON_SRC is absent, the script still writes Icon=sessionledger. If sessionledger is not a desktop icon file under AppDir, appimagetool rejects the AppDir, and the script exits under set -e. Add an icon or a fallback icon filename/path before writing the desktop entry, or fail before invoking appimagetool.
🤖 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 30 - 43, Update the icon
handling around ICON_SRC and the generated sessionledger.desktop entry so the
AppDir always has a valid icon reference before appimagetool runs. When the
source icon is missing, either install a valid fallback icon and reference it or
fail explicitly before writing/invoking appimagetool; do not leave
Icon=sessionledger pointing to a nonexistent file.
| # 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 | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
probe_line="$(rg -n -m1 '\$baseUrl/\$candidate' scripts/install.ps1 | cut -d: -f1)"
assign_line="$(rg -n -m1 '^\$baseUrl = ' scripts/install.ps1 | cut -d: -f1)"
test "$assign_line" -lt "$probe_line"Repository: KooshaPari/SessionLedger
Length of output: 162
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- install.ps1 relevant section ---\n'
sed -n '1,130p' scripts/install.ps1
printf '\n--- baseUrl usages/assignments ---\n'
rg -n '(^|\s)\$baseUrl|releases/downloads|Invoke-WebRequest' scripts/install.ps1Repository: KooshaPari/SessionLedger
Length of output: 253
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- install.ps1 relevant section ---'
sed -n '1,130p' scripts/install.ps1
printf '%s\n' ''
printf '%s\n' '--- baseUrl usages/assignments ---'
rg -n '(^|\s)\$baseUrl|releases/downloads|Invoke-WebRequest' scripts/install.ps1Repository: KooshaPari/SessionLedger
Length of output: 5884
Define $baseUrl before probing archive candidates.
$baseUrl is used at line 88, but it is first assigned only at line 98. In normal installation paths, this resolves $baseUrl to $null before the candidate Invoke-WebRequest, hides the HEAD check failure in catch, and then throws No sl-viewer archive found.
Proposed fix
$assetVersion = $Version.TrimStart("v")
+$baseUrl = "https://github.com/$Repo/releases/download/$Version"
$archive = $null
foreach ($candidate in @(...)) {
...
}
if (-not $archive) {
throw ...
}
-$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. | |
| $assetVersion = $Version.TrimStart("v") | |
| $baseUrl = "https://github.com/$Repo/releases/download/$Version" | |
| $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
before the archive-candidate loop so `Invoke-WebRequest` in the probing logic
uses the release download URL. Keep the existing candidate order, fallback
behavior, and final archive-not-found error unchanged.
| 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. | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)scripts/(install\.ps1|install\.sh)$|(^|/)scripts/|^README' || true
echo "== install.ps1 relevant section =="
if [ -f scripts/install.ps1 ]; then
nl -ba scripts/install.ps1 | sed -n '70,105p'
fi
echo "== install.sh relevant section =="
if [ -f scripts/install.sh ]; then
nl -ba scripts/install.sh | sed -n '110,135p'
fi
echo "== curl/header handling references =="
rg -n "curl|Invoke-WebRequest|HTTP 404|404|HEAD|archive=|archive|sl-viewer" scripts -S || true
echo "== behavior probe: shell HEAD-only with httpbin is not available/allowed in README? We inspect deterministic local behavior for curl errors without network via file? Keeping to deterministic text. =="Repository: KooshaPari/SessionLedger
Length of output: 3132
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)scripts/(install\.ps1|install\.sh)$|(^|/)scripts/|^README' || true
echo "== install.ps1 relevant section =="
if [ -f scripts/install.ps1 ]; then
nl -ba scripts/install.ps1 | sed -n '70,105p'
fi
echo "== install.sh relevant section =="
if [ -f scripts/install.sh ]; then
nl -ba scripts/install.sh | sed -n '110,135p'
fi
echo "== curl/header handling references =="
rg -n "curl|Invoke-WebRequest|HTTP 404|404|HEAD|archive=|archive|sl-viewer" scripts -S || trueRepository: KooshaPari/SessionLedger
Length of output: 3132
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== scripts/install.ps1 relevant section =="
sed -n '70,105p' scripts/install.ps1 | cat -n
echo "== scripts/install.sh relevant section =="
sed -n '110,135p' scripts/install.sh | cat -n
echo "== deterministic curl HTTP method/status probe =="
python3 - <<'PY'
print("This shell cannot fetch external networks, but the curl failure modes are deterministic from its HEAD semantics:")
print("- Curl in this installer uses 'curl -fsSL -I': only headers; response body is not saved.")
print("- Any DNS, TLS, authorization, rate-limit, or server transport/server error makes the curl command exit nonzero.")
print("- The installer records failures with '2>/dev/null' and repeats only the missing spelling.")
PYRepository: KooshaPari/SessionLedger
Length of output: 3616
Preserve transport failures during archive fallback.
Both installers treat any failed archive probe as a missing spelling. Continue only for an HTTP 404. For other probe failures, report the transport/server error so DNS, TLS, authorization, rate limit, and server problems are not replaced with “archive not found”.
scripts/install.ps1#86-L93: report non-404Invoke-WebRequestfailures before trying the second candidate.scripts/install.sh#123-L129: check the HTTP status and continue to the next spelling only for 404.
🧰 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)
📍 Affects 2 files
scripts/install.ps1#L86-L93(this comment)scripts/install.sh#L123-L129
🤖 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 86 - 93, Update the archive probe fallback
in scripts/install.ps1 lines 86-93 so the catch block continues only when the
Invoke-WebRequest failure represents HTTP 404; rethrow or report all other
failures before trying another candidate. Update scripts/install.sh lines
123-129 to inspect the probe’s HTTP status and retry the alternate filename only
for 404, while propagating transport, authorization, rate-limit, and server
errors.
Source: Linters/SAST tools
|
Closed as stale. CI: 4 FAILURE (fmt/clippy/test/lint), CONFLICTING against main, CodeRabbit CHANGES_REQUESTED with 8 actionable items. The real-data wire-up (build_bundles_from_sessions) is now on main (commit 6bee35d). Recommend re-cutting the bounded-decompression + decoded-size-cap work as a focused PR against current main, plus a separate small PR for the empty-corpus-load error-vs-demo fix. |
SQUASH MERGE (admin): viewer polishing + packaging hardening. - packages/app.rs: rename `corpus_error_signal` → `error_signal` (clearer naming; matches the existing 4-tab pattern; no longer leaks callback semantics) - packaging/macos/install-local.sh: extract STAGING_DIR + validation + rollback pattern from ad-hoc copy to defensible module; remove pre-existing .previous before staging - packaging/macos/package-app.sh: gate CFBundleIconFile/CFBundleIconName + Info.plist post-script on successful iconutil run; consolidate icon conversion into a single block; skip Info.plist re-write unless resources were freshly placed Directly addresses CodeRabbit review from PR #393/394. Verified locally: builds clean, mock-leak grep returns 0, app opens in ~2s with daemon running.
Summary