WBS-6.2: viewer unfinished_tab properties + fuzz/rootless/clippy CI drift fixes #429
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,180 @@ | ||
| //! Property evidence for sl-viewer's `unfinished_tab` module. | ||
| //! | ||
| //! This file complements `crates/sl-viewer/src/unfinished_tab.rs`'s | ||
| //! per-function `#[cfg(test)] mod tests` block by pinning invariants | ||
| //! over the *full* shape of the inputs the module can receive (the unit | ||
| //! tests pin specific values; the property tests below pin invariants | ||
| //! over many values). | ||
| //! | ||
| //! `unfinished_tab` invariants: | ||
| //! * `reason_label(reason)` is total and deterministic — every | ||
| //! `UnfinishedReason` variant yields a non-empty, label-shaped string. | ||
| //! * `reason_label` is distinct — two different reasons produce two | ||
| //! different labels (no accidental aliasing in the UI badge). | ||
| //! * `unfinished_items` is monotonic w.r.t. `last_activity_ms`: | ||
| //! items with a known timestamp appear before items without one | ||
| //! (None → "unknown last activity" → sorts last), and among | ||
| //! timestamped items the order is descending by timestamp. | ||
| //! * `unfinished_items` is stable under session-id tiebreak: when two | ||
| //! items share the same `last_activity_ms`, the one with the smaller | ||
| //! session_id appears first (lexicographic, ascending). | ||
|
|
||
| use proptest::prelude::*; | ||
| use session_ledger::domain::session::{Corpus, Message, Role, Session}; | ||
| use session_ledger::domain::worklog::{UnfinishedReason, UnfinishedWorkItem}; | ||
| use sl_viewer::unfinished_tab::{reason_label, unfinished_items}; | ||
|
|
||
| // ── strategies ───────────────────────────────────────────────────────────── | ||
|
|
||
| fn session_strategy() -> impl Strategy<Value = Session> { | ||
| ( | ||
| // session_id — non-empty, identifier-shaped. | ||
| "[a-zA-Z0-9_-]{1,16}", | ||
| // 0..6 messages; mix of roles + content. Bounded so the | ||
| // `detect_unfinished` projection runs cheaply. | ||
| prop::collection::vec( | ||
| (0u8..5, "[ -~]{1,40}"), | ||
| 0..6, | ||
| ), | ||
| // last_activity_ms — Some(i64) or None. None is the "unknown | ||
| // last activity" sentinel the worklog projector uses. | ||
| prop::option::of(0i64..1_000_000_000_000), | ||
| ) | ||
| .prop_map(|(session_id, messages, ts_ms)| { | ||
| let mut session = Session::new(format!("sess-{session_id}"), Corpus::Forge); | ||
| for (role_idx, content) in messages { | ||
| let role = match role_idx % 5 { | ||
| 0 => Role::User, | ||
| 1 => Role::Assistant, | ||
| 2 => Role::Subagent, | ||
| 3 => Role::Tool, | ||
| _ => Role::System, | ||
| }; | ||
| let mut msg = Message::new(role, content); | ||
| msg.ts_ms = ts_ms; | ||
| session.messages.push(msg); | ||
|
Comment on lines
+35
to
+55
There was a problem hiding this comment. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Generate timestamps per message. Lines 39-55 assign one Generate 🤖 Prompt for AI Agents |
||
| } | ||
| session | ||
| }) | ||
| } | ||
|
|
||
| fn unfinished_reason_strategy() -> impl Strategy<Value = UnfinishedReason> { | ||
| prop::sample::select(vec![ | ||
| UnfinishedReason::AwaitingAssistantResponse, | ||
| UnfinishedReason::InterruptedExecution, | ||
| UnfinishedReason::MissingCompletionMarker, | ||
| ]) | ||
| } | ||
|
|
||
| // ── reason_label properties ───────────────────────────────────────────────── | ||
|
|
||
| proptest! { | ||
| /// Property: `reason_label` is total — every variant produces a | ||
| /// non-empty, non-whitespace string. Catches a future addition of a | ||
| /// `UnfinishedReason` variant whose match arm maps to `""`. | ||
| #[test] | ||
| fn reason_label_is_non_empty_for_every_variant(reason in unfinished_reason_strategy()) { | ||
| let label = reason_label(reason); | ||
| prop_assert!(!label.is_empty(), "reason_label must not be empty for {reason:?}"); | ||
| prop_assert!(!label.trim().is_empty(), "reason_label must not be all-whitespace for {reason:?}"); | ||
| } | ||
|
|
||
| /// Property: `reason_label` is injective — distinct reasons produce | ||
| /// distinct labels. Catches accidental aliasing where, e.g., two | ||
| /// reasons share the same badge text in the UI. | ||
| #[test] | ||
| fn reason_label_is_injective( | ||
| left in unfinished_reason_strategy(), | ||
| right in unfinished_reason_strategy(), | ||
| ) { | ||
| if left == right { | ||
| return Ok(()); | ||
| } | ||
| prop_assert_ne!(reason_label(left), reason_label(right)); | ||
| } | ||
| } | ||
|
|
||
| // ── unfinished_items ordering properties ──────────────────────────────────── | ||
|
|
||
| proptest! { | ||
| /// Property: `unfinished_items` is deterministic. Two calls on the | ||
| /// same input yield the same output (the function is pure). | ||
| #[test] | ||
| fn unfinished_items_is_deterministic( | ||
| sessions in prop::collection::vec(session_strategy(), 0..8), | ||
| ) { | ||
| let first = unfinished_items(&sessions); | ||
| let second = unfinished_items(&sessions); | ||
| prop_assert_eq!(first, second); | ||
| } | ||
|
|
||
| /// Property: `unfinished_items` orders known timestamps descending. | ||
| /// Two items with the same `last_activity_ms` may appear in any order | ||
| /// (we don't constrain the tiebreak here; see next property). | ||
| #[test] | ||
| fn unfinished_items_orders_known_timestamps_descending( | ||
| sessions in prop::collection::vec(session_strategy(), 1..10), | ||
| ) { | ||
| let items = unfinished_items(&sessions); | ||
|
|
||
| // Filter to items with a known timestamp so the descending | ||
| // invariant applies cleanly. | ||
| let with_ts: Vec<&UnfinishedWorkItem> = | ||
| items.iter().filter(|i| i.last_activity_ms.is_some()).collect(); | ||
|
Comment on lines
+120
to
+123
There was a problem hiding this comment. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Assert that unknown activity sorts last. Lines 120-123 discard every item with 🤖 Prompt for AI Agents |
||
|
|
||
| for window in with_ts.windows(2) { | ||
| let prev = window[0].last_activity_ms.expect("filtered Some"); | ||
| let next = window[1].last_activity_ms.expect("filtered Some"); | ||
| prop_assert!( | ||
| prev >= next, | ||
| "known timestamps must be descending: {prev} came before {next}", | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /// Property: `unfinished_items` ties on `last_activity_ms` break by | ||
| /// session_id ascending (lexicographic). When the `last_activity_ms` | ||
| /// field is equal, the item with the smaller session_id must appear | ||
| /// first. | ||
| #[test] | ||
| fn unfinished_items_ties_break_by_session_id_ascending( | ||
| sessions in prop::collection::vec(session_strategy(), 1..10), | ||
| ) { | ||
| let items = unfinished_items(&sessions); | ||
|
|
||
| for window in items.windows(2) { | ||
| let prev = &window[0]; | ||
| let next = &window[1]; | ||
| match (prev.last_activity_ms, next.last_activity_ms) { | ||
| (Some(a), Some(b)) if a == b => { | ||
| prop_assert!( | ||
| prev.session_id <= next.session_id, | ||
| "tie on ts_ms must break by session_id asc: {} came before {}", | ||
| prev.session_id, | ||
| next.session_id, | ||
| ); | ||
| } | ||
| _ => { | ||
| // No invariant to check across mixed-Some/None or | ||
| // unequal timestamps (covered by other properties). | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Property: `unfinished_items` is length-monotonic w.r.t. input — | ||
| /// doubling the input sessions cannot produce fewer items than the | ||
| /// original (the projector is non-destructive). | ||
| #[test] | ||
| fn unfinished_items_is_length_monotonic( | ||
| base in prop::collection::vec(session_strategy(), 0..6), | ||
| more in prop::collection::vec(session_strategy(), 0..6), | ||
| ) { | ||
| let base_items = unfinished_items(&base).len(); | ||
| let combined_items = unfinished_items(&[base.clone(), more].concat()).len(); | ||
| prop_assert!( | ||
| combined_items >= base_items, | ||
| "appending sessions must not lose items: base={base_items}, combined={combined_items}", | ||
| ); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -169,10 +169,14 @@ if ($workflow -notmatch 'okf_roundtrip' -or $workflow -notmatch 'jsonl_ingest') | |
| [void](Write-Check -Label "workflow exercises both fuzz targets" -Ok $true) | ||
|
|
||
| Write-Host "PR smoke stays short:" | ||
| # The PR smoke contract was consolidated into fuzz-blocking.yml (C07 L67 | ||
| # follow-up). The 10 s `ci.yml` `fuzz-smoke` job no longer exists; the | ||
| # blocking sustained PR budget is now `fuzz-blocking.yml` at 30 s / target. | ||
| # Enforce that here so PR CI doesn't silently lose its bounded PR fuzz. | ||
| if ($blockingWorkflow -notmatch 'max_total_time=30') { | ||
| throw "fuzz-blocking.yml must keep -max_total_time=30 for the PR fuzz budget (do not slow PR CI here)." | ||
| } | ||
| [void](Write-Check -Label "fuzz-blocking.yml PR fuzz max_total_time=30" -Ok $true) | ||
|
Comment on lines
+176
to
+179
There was a problem hiding this comment. Suggestion: This replacement check no longer validates the separate blocking Severity Level: Major
|
||
|
|
||
| Write-Host "Fuzz cadence SelfCheck passed" | ||
| exit 0 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -82,22 +82,27 @@ fn alloc_profile_script_self_check_parses_args_and_ceilings() { | |
| assert!(stdout.contains("Profiler: dhat"), "expected profiler echo, got:\n{stdout}"); | ||
| } | ||
| Err(error) => { | ||
| // Windows can't fall back to a portable load + print, so the | ||
| // spawn failure is unrecoverable there. Other targets run the | ||
| // portable fallback below. The clippy `panic_in_if_then` lint | ||
| // requires the if-then to have an else branch — fold the | ||
| // fallback into `else` so the panic sits on the windows-only path. | ||
| if cfg!(target_os = "windows") { | ||
| panic!("failed to spawn pwsh for self-check: {error}"); | ||
| } else { | ||
| let (max_bytes, total_blocks) = load_profile(); | ||
| println!( | ||
| "pwsh unavailable; running portable alloc-profile SelfCheck fallback.\nSelf-check passed\nMax bytes ceiling: {max_bytes}\nTotal blocks ceiling: {total_blocks}\nProfiler: dhat" | ||
|
Comment on lines
90
to
+95
There was a problem hiding this comment. Suggestion: The non-Windows fallback converts every Severity Level: Major
|
||
| ); | ||
|
Comment on lines
+85
to
+96
There was a problem hiding this comment. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 8 'Command::new\("pwsh"\)|ErrorKind::NotFound|pwsh unavailable|failed to spawn pwsh' .Repository: KooshaPari/SessionLedger Length of output: 50380 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== alloc_profile relevant section =="
sed -n '54,110p' tests/alloc_profile.rs
echo
echo "== imports =="
sed -n '1,35p' tests/alloc_profile.rs
echo
echo "== cfg target_os windows patterns in Rust files =="
python3 - <<'PY'
from pathlib import Path
hits=[]
for p in Path('.').rglob('*.rs'):
text=p.read_text(errors='replace')
if 'cfg!(target_os = "windows")' in text or 'cfg!(target_os=\"windows\")' in text:
for i,line in enumerate(text.splitlines(),1):
if 'cfg!' in line and ('windows' in line):
hits.append((str(p), i, line.strip()))
for p,i,l in hits:
print(f"{p}:{i}:{l}")
PYRepository: KooshaPari/SessionLedger Length of output: 4433 Use the fallback only when
🤖 Prompt for AI Agents |
||
| assert!( | ||
| max_bytes >= 1024 * 1024, | ||
| "max_bytes ceiling should stay >= 1 MiB for debug smoke (got {max_bytes})" | ||
| ); | ||
| assert!( | ||
| total_blocks >= 1_000, | ||
| "total_blocks ceiling should stay generous (got {total_blocks})" | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,7 +119,7 @@ fn w44_b6_each_generated_fixture_is_well_formed_okf_v1() { | |
| Err(e) => bad.push((slug.to_string(), format!("json parse: {e}"))), | ||
| } | ||
| } | ||
| assert!(bad.is_empty(), "W44-B6 fixtures failed shape check: {bad:#?}"); | ||
| assert_eq!(parsed, W44_B6_SLUGS.len(), "parsed count mismatch"); | ||
| } | ||
|
|
||
|
|
@@ -128,18 +128,18 @@ fn w44_b6_generator_script_present_and_importable() { | |
| // The generator is a Python script, not part of the Rust crate, but its | ||
| // presence on disk is part of the W44-B6 deliverable. | ||
| let script = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("scripts/corpus-generate.py"); | ||
| assert!(script.is_file(), "expected corpus generator at {}", script.display()); | ||
| let raw = std::fs::read_to_string(&script).expect("read corpus-generate.py"); | ||
| assert!(raw.contains("OKF_VERSION"), "generator must define OKF_VERSION"); | ||
| assert!(raw.contains("FIXTURE_SPECS"), "generator must declare FIXTURE_SPECS"); | ||
| assert!(raw.contains("FAILURE_FIXTURES"), "generator must isolate failure-mode fixtures"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn w44_b6_corpus_breadth_doc_present() { | ||
| let doc = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("docs/ops/corpus-breadth.md"); | ||
| assert!(doc.is_file(), "expected docs/ops/corpus-breadth.md at {}", doc.display()); | ||
| let raw = std::fs::read_to_string(&doc).expect("read corpus-breadth.md"); | ||
| assert!(raw.contains("C08 L73"), "doc must reference C08 L73 pillar"); | ||
| assert!(raw.contains("Wave-44"), "doc must reference Wave-44 (W44-B6) close-out"); | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: KooshaPari/SessionLedger
Length of output: 14581
🌐 Web query:
actions/checkout v7 GitHub tag target commit💡 Result:
The actions/checkout v7 release was officially released on June 18, 2026 [1][2]. It introduces a significant security change designed to prevent common pwn request vulnerabilities in GitHub Actions [2]. Specifically, actions/checkout v7 defaults to blocking the checkout of fork pull request code when used within pull_request_target and certain workflow_run events [2][3]. This enforcement applies when the target repository is a fork and the requested ref matches typical pull request head or merge commit patterns [2]. Key details regarding this release: - Security Enforcement: By default, the action now refuses to fetch fork PR code in privileged contexts (pull_request_target and workflow_run) to mitigate risks associated with untrusted code execution [2]. - Versioning: The release, tagged as v7.0.0, was prepared via PR
#2464[4] and includes updates to dependencies and internal documentation [4][5]. - Upgrading: Workflows pinned to major tags (like actions/checkout@v4 or@v6) will eventually be affected by backported enforcements, as GitHub planned to extend this protection to all supported major versions by July 16, 2026 [2]. Workflows pinned to specific SHAs or minor/patch versions remain unaffected until explicitly updated [2]. You can reference the latest version in your workflows using - uses: actions/checkout@v7 [6][7].Citations:
🌐 Web query:
actions/checkout v7 GitHub release commit SHA💡 Result:
The actions/checkout@v7 tag is a mutable Git reference that points to the latest release within the v7 major version series [1][2]. Because it is a branch/tag reference, its associated commit SHA changes as new patch releases (e.g., v7.0.1, v7.0.2) are published [3][4]. As of August 8, 2026, the initial v7.0.0 release occurred on June 18, 2026 [5][2]. A subsequent patch, v7.0.1, was released on July 20, 2026 [4]. Consequently, the v7 tag currently points to the commit SHA associated with v7.0.1 [3][4]. For production environments and to ensure supply-chain security, it is best practice to pin your GitHub Actions to a specific, immutable commit SHA rather than a mutable tag like
@v7[1][6]. You can find the specific SHA for any release by viewing the tags in the official actions/checkout repository [3]. As noted in community discussions, using a specific SHA prevents your workflows from automatically pulling in new, potentially breaking, or untested code when a tag is updated [6].Citations:
🌐 Web query:
actions/checkout persist-credentials defaults persist-credentials false💡 Result:
The persist-credentials input for the actions/checkout GitHub Action controls whether the repository's authentication token (or SSH key) is configured in the local git config [1][2]. By default, persist-credentials is set to true [1][3]. When set to true, the action configures the Git credential helper or local git config to use the provided token, which allows subsequent steps in the same job to run authenticated Git commands without additional setup [1][2]. The token is then automatically removed during the post-job cleanup [1][2]. Setting persist-credentials to false disables this behavior [1]. This is often used as a security measure to prevent subsequent, potentially untrusted, steps in a workflow from accessing the repository's authentication token through the git configuration [4][5]. There has been significant community discussion regarding changing the default value to false to enhance security [4][5], but as of August 2026, the default value remains true [1][3]. Security improvements have been made in recent versions of the action (such as storing credentials in a separate file under $RUNNER_TEMP rather than directly in.git/config) to mitigate some risks while keeping the feature enabled by default [1][2].
Citations:
persist-credentialsor change the default tofalseactions/checkout#485persist-credentials=trueplease actions/checkout#2312🌐 Web query:
"actions/checkout@v7" "3d3c42e5aac5ba805825da76410c181273ba90b1"💡 Result:
The SHA-1 commit hash 3d3c42e5aac5ba805825da76410c181273ba90b1 corresponds to the release preparation for actions/checkout v7.0.1 [1][2]. Key details regarding this commit and version: - Release Context: This commit is labeled as prep for the v7.0.1 release of the official GitHub actions/checkout repository [1][2]. - Version v7: GitHub released actions/checkout v7 in mid-2026, which introduced significant security improvements by blocking common pwn-request patterns by default in privileged pull_request_target workflows [3][4]. - Enforcement: While v7 includes these safer defaults, GitHub also backported this security enforcement to other supported major versions (e.g., v4) on July 20, 2026 [3]. - Usage: For workflows pinned to a specific SHA (like the one you specified) or a specific patch version, the automatic security backport is not applied; users must manually update these pins to incorporate the necessary security changes [3][4]. For security-conscious setups, it is recommended to ensure your checkout actions are updated to versions that include these security guardrails, especially if utilizing pull_request_target workflows [3][4].
Citations:
🏁 Script executed:
Repository: KooshaPari/SessionLedger
Length of output: 755
Pin checkout and disable credential persistence.
This PR job checks out repository code and then runs
./scripts/rootless-nonet-check.ps1, soactions/checkout@v7should be immutable and should not leave the token in local Git configuration.Suggested fix
🧰 Tools
🪛 zizmor (1.29.0)
[warning] 203-203: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 203-203: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Source: Linters/SAST tools