Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: 5 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,11 @@ jobs:
continue-on-error: true
steps:
- uses: actions/checkout@v7
with:
# gitleaks-action scans `firstCommit^..head`; with the default
# depth-1 fetch the first commit's parent is missing and the scan
# fails with "unknown revision" on any multi-commit PR.
fetch-depth: 0
- name: gitleaks
uses: gitleaks/gitleaks-action@v3
env:
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
41 changes: 34 additions & 7 deletions .github/workflows/trunk-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@
# Handles: ruff, mypy, clippy, golangci-lint, prettier, eslint, shellcheck, etc.
# Free for open source; cached for fast runs
# =============================================================================
#
# NOTE (2026-08): the lane runs the same linters that `.trunk/trunk.yaml`
# enables (actionlint + taplo + yamllint) via direct, pinned installs instead
# of trunk-io/trunk-action. trunk-action's managed tool bootstrap repeatedly
# failed on ubuntu-latest ("Binary not found" / "jq not installed on system!"
# inside its launcher), while the tools themselves install cleanly. actionlint
# runs from the official docker image (no download-and-execute in the lane).
# The `.trunk/` config remains the local-developer source of truth
# (`trunk check`).

name: Trunk Check

Expand All @@ -28,11 +37,29 @@ jobs:
- name: Checkout
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1

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

- name: Trunk Upgrade (on schedule only)
if: github.event_name == 'schedule'
uses: trunk-io/trunk-action@v1
- name: Run actionlint
# Official pinned image; avoids curl|tar installs (SonarCloud flags
# download-and-execute patterns as a security issue on new code).
# `-shellcheck=` disables the image's bundled shellcheck: trunk's
# actionlint integration runs without shellcheck too (and the repo's
# workflows carry pre-existing SC2034/SC2086 advisories that trunk
# does not gate on).
uses: docker://rhysd/actionlint:1.7.12
with:
trunk-args: --upgrade
args: -shellcheck=

- name: Install taplo
run: cargo install taplo-cli --locked --version 0.9.3

- name: Install yamllint
run: sudo apt-get update && sudo apt-get install -y yamllint
Comment on lines +54 to +55

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Changed/workflow files:"
git ls-files .github/workflows/trunk-check.yml | sed -n '1,20p'

echo
echo "Workflow excerpt:"
if [ -f .github/workflows/trunk-check.yml ]; then
  nl -ba .github/workflows/trunk-check.yml | sed -n '1,90p'
fi

echo
echo "yamllint references:"
rg -n "yamllint|Trunk|pin|pinned|tool" .github/workflows .trunk 2>/dev/null || true

echo
echo "Workflow runner and apt cache config:"
rg -n "runs-on:|apt|cache|pin|install" .github/workflows/trunk-check.yml

Repository: KooshaPari/sharecli

Length of output: 275


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
p=Path('.github/workflows/trunk-check.yml')
print('exists:', p.exists())
if p.exists():
    lines=p.read_text().splitlines()
    for i,l in enumerate(lines,1):
        if i in [*range(1,16),*range(40,55)] or any(s in l for s in ['yamllint','Trunk','pin']):
            print(f'{i}: {l}')
PY

Repository: KooshaPari/sharecli

Length of output: 1930


🌐 Web query:

Ubuntu apt pin package yamllint version default install behavior

💡 Result:

On Ubuntu, the apt package manager determines the default installation version of a package like yamllint by selecting the candidate with the highest priority [1][2]. By default, this is the version available in your configured package repositories with the highest version number [1]. To control or lock a specific version, you can use APT pinning [1][3]. This is generally considered more reliable than commands like apt-mark hold for system-wide configuration, as it is respected by all apt-based tools [4]. To pin the yamllint package to a specific version, create a file in the /etc/apt/preferences.d/ directory (e.g., /etc/apt/preferences.d/yamllint) with the following format [1][5][3]: Package: yamllint Pin: version 1.37.1-1 Pin-Priority: 1001 In this configuration: 1. Package: Specifies the package name [1][3]. 2. Pin: Specifies the exact version string you wish to pin [6][3]. You can verify the exact version strings available on your system using the command: apt-cache policy yamllint [6][7]. 3. Pin-Priority: Setting this to 1001 or higher forces apt to install that specific version, even if it constitutes a downgrade or overrides a newer version available in the repositories [2][8][7]. After creating this file, you can verify that the pinning is correctly applied by running apt-cache policy yamllint again; the output should show the pinned version as the candidate [8][7]. Note that pinning by version requires you to specify the exact version string exactly as it appears in the package database, which sometimes includes an epoch (e.g., 5:1.2.3) [6][8].

Citations:


🌐 Web query:

Trunk tools pinned version policy pinned tools

💡 Result:

Trunk employs a config-as-code philosophy to manage tools, linters, and runtimes, ensuring consistent environments across local development and CI [1][2]. All such dependencies are pinned within your repository's.trunk/trunk.yaml configuration file [3][2]. Pinning and Version Management: - Version Pinning: You can pin a specific version of a tool, linter, or runtime by appending an exclamation mark (!) to the version string in your.trunk/trunk.yaml file [4][5]. For example: lint: enabled: - pylint@2.17.5! - Preventing Upgrades: When a version is pinned with the! suffix, the trunk upgrade command will ignore it, preventing unintended updates [4][5]. - Automatic Management: When you enable a tool via trunk tools enable @, Trunk automatically updates the configuration to lock that version [3]. Trunk handles the hermetic installation and caching of these tools (in ~/.cache/trunk or.trunk/tools), ensuring they do not rely on or pollute your host system's environment [3][6][2]. - Eponymous vs. Additional Tools: If a linter depends on a tool, they may need to keep their versions synced [4]. For eponymous tools (where the tool name matches the linter name), the tool is implicitly enabled with the linter's version [7]. Additional tool dependencies must be explicitly enabled and versioned independently in the tools.enabled section of your configuration [7]. Upgrading: - To upgrade tools, you use the trunk upgrade command [4][8]. This command updates tools to their latest public releases unless they have been explicitly pinned with the! suffix [4]. You can also apply these upgrades to specific configuration files using the --apply-to flag [4][8].

Citations:


Pin the yamllint version in CI.

apt-get install -y yamllint installs the repository candidate, so Ubuntu/apt updates can change the linter version and its output. Match the pinned-tool approach used for the other direct installs.

Example fix
-        run: sudo apt-get update && sudo apt-get install -y yamllint
+        run: python -m pip install --disable-pip-version-check "yamllint==<approved-version>"
🤖 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/trunk-check.yml around lines 47 - 48, Update the “Install
yamllint” workflow step to install an explicitly pinned yamllint version instead
of the unversioned apt package, matching the version-pinning approach used by
the other direct installs while preserving the existing apt update and
installation flow.

Source: MCP tools


- name: Run remaining trunk linters
run: |
set -euo pipefail
# actionlint runs in the docker step above (reads
# .github/actionlint.yaml for ignore rules).
# Same whitespace-only formatting contract as `trunk fmt`.
taplo fmt --check
# Same relaxed ruleset as `.trunk/trunk.yaml`'s yamllint section.
yamllint -c .trunk/configs/.yamllint.yaml .
7 changes: 7 additions & 0 deletions .trunk/configs/.yamllint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@

extends: default

# Vendored/generated directories are not linted (gitignored locally; never
# present on CI checkouts, but direct `yamllint .` runs see them).
ignore: |
node_modules/
target/
.git/

rules:
line-length: disable
comments:
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
10 changes: 7 additions & 3 deletions src/audit_log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,14 @@ mod tests {

use super::*;

/// Serializes all audit tests that mutate process-global env vars.
/// The tests otherwise race on `SHARECLI_AUDIT_LOG` (e.g. one test
/// removes the var while another is mid-emit, so the file is never
/// written and `read_to_string` fails with `NotFound`).
static ENV_LOCK: Mutex<()> = Mutex::new(());

#[test]
fn emit_if_configured_respects_env_gate() {
static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
let _env = ENV_LOCK.lock().unwrap();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WARNING: ENV_LOCK.lock().unwrap() is not poison-tolerant

The previous WRITE_LOCK used unwrap_or_else(|e| e.into_inner()) to recover from poisoned locks. If any of these tests panics while holding ENV_LOCK, all subsequent env-touching tests will panic instead of recovering. The same pattern repeats at lines 213 and 227.

Suggested change
let _env = ENV_LOCK.lock().unwrap();
let _env = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("gated.jsonl");
Expand Down Expand Up @@ -205,9 +210,9 @@ mod tests {

#[test]
fn path_respects_env_override() {
let _env = ENV_LOCK.lock().unwrap();
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("custom.jsonl");
let _guard = WRITE_LOCK.lock().unwrap_or_else(|e| e.into_inner());
unsafe {
std::env::set_var("SHARECLI_AUDIT_LOG", &path);
}
Expand All @@ -219,7 +224,6 @@ mod tests {

#[test]
fn rotates_when_over_max_bytes() {
static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
let _env = ENV_LOCK.lock().unwrap();
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("audit.jsonl");
Expand Down
Binary file modified tests/visual/dashboard/desktop.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 6 additions & 6 deletions tests/visual/dashboard/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,33 @@
"surface": "src/dashboard.html",
"fixture": "empty-pool + operator envelope (fresh sharecli serve / visual WS fixture)",
"platform": "ubuntu-24.04 + Playwright Chromium",
"seed_sha": "91448b2",
"seed_date": "2026-07-18",
"seed_sha": "a958f78",
"seed_date": "2026-08-10",
"baselines": [
{
"name": "mobile",
"file": "mobile.png",
"bytes": 11043,
"bytes": 75563,
"viewport": { "width": 375, "height": 812 },
"artifact_source": "artifacts/playwright/mobile-375.png",
"max_pixel_delta_pct": 0.2
},
{
"name": "tablet",
"file": "tablet.png",
"bytes": 14120,
"bytes": 86045,
"viewport": { "width": 768, "height": 1024 },
"artifact_source": "artifacts/playwright/tablet-768.png",
"max_pixel_delta_pct": 0.1
},
{
"name": "desktop",
"file": "desktop.png",
"bytes": 14227,
"bytes": 107391,
"viewport": { "width": 1280, "height": 800 },
"artifact_source": "artifacts/playwright/desktop-1280.png",
"max_pixel_delta_pct": 0.1
}
],
"notes": "Committed Ubuntu PNG baselines with byte-size lock. Hard diff: scripts/visual/compare_screenshots.mjs + visual-soft.yml. Capture with SHARECLI_VISUAL_FIXTURE=1; regen with UPDATE_VISUALS=1 only from the ubuntu-24.04 gate."
"notes": "Committed Ubuntu PNG baselines with byte-size lock. Hard diff: scripts/visual/compare_screenshots.mjs + visual-soft.yml. Capture with SHARECLI_VISUAL_FIXTURE=1; regen with UPDATE_VISUALS=1 only from the ubuntu-24.04 gate. Regenerated 2026-08-10 from workflow_dispatch run 31346443985 (main a958f78, playwright 1.62.1 chromium v1234): the 2026-07-18 goldens predate the dashboard's taller mobile layout (page grew 431px; tablet/desktop drifted ~2.9%)."
}
Binary file modified tests/visual/dashboard/mobile.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/visual/dashboard/tablet.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading