Skip to content
8 changes: 5 additions & 3 deletions .github/workflows/a11y.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ jobs:
shared-key: a11y-keyboard
- uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
with:
node-version: "20"
node-version: "22"
cache: npm
- name: Install a11y deps
run: npm ci
Expand All @@ -59,8 +59,10 @@ jobs:
- name: Install Playwright Chromium
run: npx --yes playwright@1.62.1 install --with-deps chromium
- name: Run keyboard Tab-cycle
env:
SHARECLI_VISUAL_FIXTURE: "1"
# No SHARECLI_VISUAL_FIXTURE here: the fixture's WebSocket mock never
# dispatches a message, so the dashboard label never reaches exactly
# "connected" and the readiness wait times out. Against the real
# server the periodic snapshot sets the label and the cycle passes.
run: |
set -euo pipefail
./target/release/sharecli serve --bind 127.0.0.1:9000 &
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/mutants.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,15 @@ jobs:
cargo mutants --timeout 60 --jobs 2 \
-p ${{ matrix.crate }} \
--config ${{ matrix.config }} \
--json-outfile ${{ matrix.json_out }} \
-- --locked
# cargo-mutants exits non-zero if survivors remain — that fails this job.
# The JSON report is always written to mutants.out/outcomes.json.
test -f mutants.out/outcomes.json
- name: Upload mutants JSON
if: always()
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
with:
name: mutants-hard-${{ matrix.crate }}-${{ github.sha }}
path: ${{ matrix.json_out }}
path: mutants.out/outcomes.json
if-no-files-found: ignore
retention-days: 14
2 changes: 2 additions & 0 deletions .github/workflows/scorecard.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ permissions:
# `read-all` (previous value) blocked the token and failed the run with
# "error obtaining token: expired_token" during result signing.
id-token: write
# Required by the SARIF upload step (github/codeql-action/upload-sarif).
security-events: write
Comment on lines +24 to +25

Copy link
Copy Markdown

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:

#!/usr/bin/env bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

lines = Path(".github/workflows/scorecard.yml").read_text(encoding="utf-8").splitlines()
jobs = lines.index("jobs:")
assert "  security-events: write" not in lines[:jobs]

analysis = lines.index("  analysis:", jobs)
assert any(
    line == "      security-events: write"
    for line in lines[analysis:]
)
PY

Repository: KooshaPari/sharecli

Length of output: 245


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf 'Workflow file exists: '
test -f .github/workflows/scorecard.yml && echo yes || echo no

printf '\nScorecard workflow outline/size:\n'
wc -l .github/workflows/scorecard.yml
printf '\nRelevant workflow contents:\n'
cat -n .github/workflows/scorecard.yml

printf '\nAll security-events permissions in workflow:\n'
rg -n '^permissions:|security-events:' .github/workflows/scorecard.yml

Repository: KooshaPari/sharecli

Length of output: 2594


Scope security-events: write to the Scorecard job.

scorecard-action only needs contents: read, while github/codeql-action/upload-sarif needs security-events: write. The workflow-level permission grants security-events: write to every job in this workflow and any future jobs. Move security-events: write under jobs.analysis.permissions and keep only the Scorecard job’s required read permissions at the workflow level or job level as appropriate.

🧰 Tools
🪛 zizmor (1.29.0)

[warning] 25-25: overly broad permissions (excessive-permissions): security-events: write is overly broad at the workflow level

(excessive-permissions)

🤖 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/scorecard.yml around lines 24 - 25, Move security-events:
write from the workflow-level permissions into the permissions block for the
Scorecard analysis job containing scorecard-action and upload-sarif. Keep
workflow or job permissions limited to contents: read where required, ensuring
other and future jobs do not inherit security-events write access.

Source: Linters/SAST tools


jobs:
analysis:
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/trunk-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ jobs:
- name: Checkout
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1

# trunk-io/trunk-action's launcher script parses the GitHub event payload
# with jq, which the ubuntu-latest image no longer ships.
- name: Install jq
run: sudo apt-get update && sudo apt-get install -y jq

- name: Trunk Check
uses: trunk-io/trunk-action@v1

Expand Down
6 changes: 6 additions & 0 deletions crates/sharecli-core/src/speculation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,12 @@ pub fn spawn_speculation_task(
cache: CoalesceCache,
thermal_gate: Arc<dyn crate::ThermalGate>,
) {
// Best-effort background task. The hypervisor constructor may run outside
// a Tokio runtime (sync CLI wiring, unit tests); without a reactor there
// is nothing to spawn onto, so skip silently rather than panic.
if tokio::runtime::Handle::try_current().is_err() {
return;
}
Comment on lines +169 to +171

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The guard permanently disables speculation when Hypervisor::with_options is called outside a Tokio runtime, even if the same hypervisor is later used from an async runtime. Cache hits will continue accumulating in the tracker, but no task will ever drain or execute them, and the constructor provides no indication that this lifecycle was skipped. Defer task creation until an active runtime is available or return an explicit lifecycle state/error. [stale reference]

Severity Level: Major ⚠️
- ⚠️ Harness Hypervisor instances can lose speculative execution permanently.
- ⚠️ Repeated cached commands receive no proactive cache warming.
- ⚠️ Performance benefit is absent without runtime-aware task startup.

Fix in Cursor Fix in VSCode Claude

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** crates/sharecli-core/src/speculation.rs
**Line:** 169:171
**Comment:**
	*Stale Reference: The guard permanently disables speculation when `Hypervisor::with_options` is called outside a Tokio runtime, even if the same hypervisor is later used from an async runtime. Cache hits will continue accumulating in the tracker, but no task will ever drain or execute them, and the constructor provides no indication that this lifecycle was skipped. Defer task creation until an active runtime is available or return an explicit lifecycle state/error.

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
👍 | 👎

Comment on lines +166 to +171

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Add regression coverage for the no-runtime branch.

Add a test that calls spawn_speculation_task without an active Tokio runtime and confirms that it returns without panicking. Confirm that the test failed before this guard was added.

As per coding guidelines: Rust bug fixes require a failing test before the fix.

🤖 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/sharecli-core/src/speculation.rs` around lines 166 - 171, Add
regression coverage for spawn_speculation_task by invoking it from a synchronous
test with no active Tokio runtime and asserting it completes without panicking.
Ensure the test would fail without the Handle::try_current guard, while
preserving existing behavior when a runtime is available.

Source: Coding guidelines

tokio::spawn(async move {
loop {
tokio::time::sleep(SPECULATION_INTERVAL).await;
Expand Down
3 changes: 3 additions & 0 deletions crates/sharecli-session/tests/session_ledger.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
//! FR-011 / C10 — session ledger durability: observations survive a store
//! reopen, and heuristic-confidence observations persist without being marked
//! auto-resumable.
use sharecli_session::{
AgentSession, ObservationKind, ResolutionConfidence, SessionObservation, SessionStore,
SurfaceCapabilities, SurfaceRecord,
Expand Down
Loading