Skip to content

fix: resolve workspace materialization race on Windows (#671)#690

Open
toshalkumbhar8979-design wants to merge 512 commits into
usestrix:mainfrom
toshalkumbhar8979-design:fix-windows-sandbox-race
Open

fix: resolve workspace materialization race on Windows (#671)#690
toshalkumbhar8979-design wants to merge 512 commits into
usestrix:mainfrom
toshalkumbhar8979-design:fix-windows-sandbox-race

Conversation

@toshalkumbhar8979-design

Copy link
Copy Markdown

Description

Fixes #671

The Bug:
When running multi-file local scans (strix --target <repo>) on Windows Docker Desktop, the TUI hangs during workspace materialization. This is caused by a concurrency race condition where docker exec latency on Windows leads to workers executing the RESOLVE_WORKSPACE_PATH_HELPER before it has been fully written, resulting in ExecTransportError or WorkspaceArchiveWriteError.

The Solution:
This PR bypasses the race condition by modifying strix/runtime/backends.py to manage SandboxConcurrencyLimits specifically for Windows environments.

  • OS Detection: Detects sys.platform == 'win32' and lowers the default concurrency limit to 1 (forcing serial execution) for manifest_entries and local_dir_files.
  • Fallback for Linux/WSL: Leaves the limit at the SDK default (None) for non-Windows environments to preserve speed.
  • Environment Override: Adds support for the STRIX_DOCKER_CONCURRENCY environment variable, allowing users to manually dial concurrency up or down regardless of their OS.

Testing:
Verified locally on Windows 11 / Docker Desktop. Multi-file targets now materialize cleanly without hanging the TUI or throwing archive errors.

0xallam and others added 30 commits January 22, 2026 11:37
Updates LiteLLM from ~1.80.7 to ~1.81.1 which includes
full support for z.ai (Zhipu AI) provider using the 'zai/model-name'
format. This enables Strix to work with z.ai subscription
credentials by setting STRIX_LLM="zai/glm-4.7" with appropriate
LLM_API_KEY and LLM_API_BASE environment variables.

Changes:
- Updated litellm version constraint in pyproject.toml
- No breaking changes to Strix API or configuration

Closes #ISSUE_ID (to be linked if applicable)

Signed-off-by: legendevent <legendevent@users.noreply.github.com>
The memory compressor was calling litellm.completion() without passing
the api_key and api_base parameters, causing authentication errors when
LLM_API_KEY is set but provider-specific env vars (OPENAI_API_KEY, etc.)
are not. This matches the pattern used in dedupe.py.
Bumps [pypdf](https://github.com/py-pdf/pypdf) from 6.6.0 to 6.6.2.
- [Release notes](https://github.com/py-pdf/pypdf/releases)
- [Changelog](https://github.com/py-pdf/pypdf/blob/main/CHANGELOG.md)
- [Commits](py-pdf/pypdf@6.6.0...6.6.2)

---
updated-dependencies:
- dependency-name: pypdf
  dependency-version: 6.6.2
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Remove hardcoded github.com/gitlab.com/bitbucket.org host lists from
infer_target_type. Instead, detect git repositories on any host by
querying the standard /info/refs?service=git-upload-pack endpoint.

Works for any self-hosted git instance.
Improve the finish_scan tool schema to produce more professional
pentest reports: expand parameter descriptions with structural
guidance, rewrite recommendations example with proper urgency tiers
instead of Priority 0/1/2, fix duplicated section titles, and clean
up informal language.
Enable native text selection across tool components and agent messages
with automatic clipboard copy, toast notification, and decorative icon
stripping. Replace Padding wrappers with Text to support selection
across multiple renderables.
Bumps [pillow](https://github.com/python-pillow/Pillow) from 11.3.0 to 12.1.1.
- [Release notes](https://github.com/python-pillow/Pillow/releases)
- [Changelog](https://github.com/python-pillow/Pillow/blob/main/CHANGES.rst)
- [Commits](python-pillow/Pillow@11.3.0...12.1.1)

---
updated-dependencies:
- dependency-name: pillow
  dependency-version: 12.1.1
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [cryptography](https://github.com/pyca/cryptography) from 44.0.1 to 46.0.5.
- [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst)
- [Commits](pyca/cryptography@44.0.1...46.0.5)

---
updated-dependencies:
- dependency-name: cryptography
  dependency-version: 46.0.5
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [protobuf](https://github.com/protocolbuffers/protobuf) from 6.33.4 to 6.33.5.
- [Release notes](https://github.com/protocolbuffers/protobuf/releases)
- [Commits](https://github.com/protocolbuffers/protobuf/commits)

---
updated-dependencies:
- dependency-name: protobuf
  dependency-version: 6.33.5
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
… and CVSS

Replace 12 flat parameters (code_file, code_before, code_after, code_diff,
and 8 CVSS fields) with structured nested XML fields: code_locations with
co-located fix_before/fix_after per location, cvss_breakdown, and cwe.

This enables multi-file vulnerability locations, per-location fixes with
precise line numbers, data flow representation (source/sink), CWE
classification, and compatibility with GitHub/GitLab PR review APIs.
- Specify encoding="utf-8" in registry.py _load_xml_schema()
- Specify encoding="utf-8" in skills/__init__.py load_skills()
- Prevents cp949/shift_jis/cp1252 decoding errors on non-English Windows
… multi-part suggestions

Rewrote the code_locations parameter description to make fix_before/fix_after
semantics explicit: they are literal block-level replacements mapped directly
to GitHub/GitLab PR suggestion blocks. Added guidance for multi-part fixes
(separate locations for non-contiguous changes like imports + code), common
mistakes to avoid, and updated all examples to demonstrate multi-line ranges.
* feat: add to readme new keys

* feat: shoutout strix models, docs

* fix: mypy error

* fix: base api

* docs: update quickstart and models

* fixes: changes to docs

uniform api_key variable naming

* test: git commit hook

* nevermind it was nothing

* docs: Update default model to claude-sonnet-4.6 and improve Strix Router docs

- Replace gpt-5 and opus-4.6 defaults with claude-sonnet-4.6 across all docs and code
- Rewrite Strix Router (models.mdx) page with clearer structure and messaging
- Add Strix Router as recommended option in overview.mdx and quickstart prerequisites
- Update stale Claude 4.5 references to 4.6 in anthropic.mdx, openrouter.mdx, bug_report.md
- Fix install.sh links to point to models.strix.ai and correct docs URLs
- Update error message examples in main.py to use claude-sonnet-4-6

---------

Co-authored-by: 0xallam <ahmed39652003@gmail.com>
0xallam and others added 23 commits June 9, 2026 01:46
* fix: resolve pre-commit check failures

- Change RuntimeError to TypeError for type validation in report/writer.py
- Update pyupgrade to v3.21.2 for Python 3.14 compatibility

* feat(cli): add --max-budget-usd flag

Raises BudgetExceededError in ReportUsageHooks after each LLM call when
accumulated cost reaches the limit, with clean "stopped" status and
child-agent cancellation in non-interactive mode.

* test: add budget enforcement unit tests

7 tests covering no-budget, under-budget, at-limit, over-limit, error
message content, None report state, and exception hierarchy.
Also adds pytest/pytest-asyncio to dev deps and a mypy override for tests.

* fix(budget): validate positive budget and check the live cost ledger

Two hardening fixes for --max-budget-usd enforcement:

- Reject non-positive budgets. ReportUsageHooks now raises ValueError for
  max_budget_usd <= 0, and the CLI validates the flag via a custom argparse
  type so '--max-budget-usd 0' fails fast with a friendly message instead of
  silently killing the scan on the first model response.
- Read the live cost. The budget check now reads ReportState.get_total_llm_cost()
  (the live ledger) instead of the persisted run-record snapshot, so it stays
  accurate even when a usage save fails after a model call.

* fix(budget): stop the entire scan deterministically when the limit is hit

Previously a BudgetExceededError was handled per-agent: it was swallowed in
interactive mode (the loop kept waiting), a child's error escaped its detached
task as an unretrieved-exception warning, the parent was never released from
wait_for_message, and the stop was logged at ERROR with a traceback as if the
agent had failed.

Replace that with a single scan-wide signal on the coordinator:

- AgentCoordinator.trigger_budget_stop() sets a flag and wakes every parked
  agent; wait_for_message returns as soon as the flag is set.
- The run loops check coordinator.budget_stopped and raise to exit cleanly,
  marking themselves 'stopped'. The root's exception reaches run_strix_scan's
  handler, which cancels descendants and tears the scan down once; child
  exceptions are swallowed in their detached task.
- The budget stop is logged at INFO, not as a failure.

This is deterministic regardless of tree depth or which agent first sees the
limit, fixing the interactive/TUI hang where a deep agent's stop never reached
a parked root. Also re-raises BudgetExceededError explicitly in the stream
handler so it can't be mistaken for the LiteLLM 'after shutdown' race.

* fix(budget): treat a budget stop as a clean stop in the TUI

Add an explicit BudgetExceededError handler in the TUI scan thread so that, if
the error ever reaches it, the budget stop is logged as a graceful stop rather
than surfaced as a red scan error by the broad 'except Exception'. The runner
normally absorbs the error and returns cleanly, so this is defensive depth for
a money-spending feature.

* docs(cli): document --max-budget-usd behavior and limitations

Clarify that the budget is cumulative across all agents, checked after each
model response, that the scan stops cleanly (not as a failure), that the value
must be > 0, and that spend can slightly overshoot due to in-flight calls and
best-effort cost estimation.

* Apply suggestions from code review

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

---------

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
* fix: resolve pre-commit check failures

- Change RuntimeError to TypeError for type validation in report/writer.py
- Update pyupgrade to v3.21.2 for Python 3.14 compatibility

* chore: add pytest test infrastructure

Mirror the layout introduced on feature/438-token_budget: pytest +
pytest-asyncio dev deps, asyncio_mode auto, a tests.* mypy override, and
pytest in the mypy pre-commit hook deps so the tests/ package type-checks.

* feat: add --mount and large-target pre-flight for local repos (usestrix#492)

Large local targets were copied into the sandbox file-by-file via the SDK
LocalDir entry, which stalls on big repos and could leave /workspace empty.

- --mount <path> bind-mounts a host directory read-only at /workspace/<subdir>
  instead of copying it, bypassing the per-file stream.
- A size pre-flight (STRIX_MAX_LOCAL_COPY_MB, default 1024) fails fast with a
  clear message suggesting --mount when a non-mounted local target is too big.

* fix: reject empty --mount paths

An empty or whitespace-only --mount value resolves to the current working
directory and would silently bind-mount it into the sandbox. Reject it.

* fix: dedupe local targets so a dir is never both copied and mounted

If the same directory is passed via --target and --mount (or as duplicate
values), it previously produced two targets — copied AND bind-mounted, and
the copied one could trip the size pre-flight. Dedupe by resolved path,
preferring the bind mount.

* fix: treat non-positive STRIX_MAX_LOCAL_COPY_MB as disabled

Previously a value of 0 (or negative) made every local target count as
oversized, aborting all local scans. Now <= 0 disables the pre-flight.

* fix: log unreadable subtrees during size pre-flight

os.walk silently swallowed directory-listing errors, so a permission-denied
subtree could make a large repo under-count and slip past the pre-flight.
Surface such omissions via an onerror warning.

* docs: document --mount and STRIX_MAX_LOCAL_COPY_MB

Add CLI reference + example for --mount, document the size pre-flight env var,
note the read-only-is-not-a-hard-boundary caveat and that remote repos are not
size-checked, and clarify the backends docstring on when bind mounts apply.

* Update strix/interface/main.py


* Update strix/runtime/docker_client.py


---------
…trix#596)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…sestrix#637)

Line 72 was over-indented, causing an IndentationError on import of strix/report/writer.py and breaking main. Also bump the mirrors-mypy pre-commit hook to v1.17.1 to avoid the mypy 1.16.0 internal crash (python/mypy#19412) on openai/_client.py.
Co-authored-by: Alex Schapiro <46074070+bearsyankees@users.noreply.github.com>
* fix: capture cost for streamed LiteLLM responses

* docs: note LiteLLM streaming metadata callbacks
…ation, Django (usestrix#617)

* Add five community security skills for agent specialization

Expand coverage with OAuth flow testing, AWS misconfigurations, prototype
pollution, insecure deserialization, and Django framework playbooks.

* Address Greptile review feedback on AWS and deserialization skills

- Use head-bucket for S3 existence checks instead of duplicating s3 ls
- Add Node.js to insecure_deserialization frontmatter description

* Clarify S3 existence vs public listing checks in aws skill

Split unauthenticated enumeration into separate head-bucket/HTTP
and s3 ls steps with interpretation guidance per review.

* some tools ads

---------

Co-authored-by: bearsyankees <bearsyankees@gmail.com>
…sestrix#626)

* feat(report): SARIF 2.1.0 emitter for CI / code-scanning integration

Strix emits CSV + markdown + JSON but no SARIF, so findings can't feed
GitHub code-scanning, an ASPM, or any SARIF-consuming CI gate. Add a
stdlib-only emitter (strix/report/sarif.py) and always write findings.sarif
from ReportState._save_artifacts, beside the existing artifacts.

Design invariants (learned from running this in production):
- Stable partialFingerprints.primaryLocationLineHash per finding, so a
  re-scan that re-words a title doesn't churn code-scanning alert IDs.
- Class/category hashing so the same vuln class maps to a stable ruleId
  across scans rather than drifting.
- Findings with no code location anchor to SECURITY.md with a synthetic
  location marker instead of being silently dropped.
- Always emit (even with zero findings) so a clean re-scan overwrites a
  stale findings.sarif and code-scanning auto-resolves fixed alerts.
- tool.driver.version reports the strix package version.
- Fully isolated in its own try/except: a SARIF build error must never
  break the CSV/MD/run-record path.

Verified end-to-end on v1.0.4 against a SQLi/cmd-inj/weak-hash fixture:
3 findings -> valid SARIF 2.1.0, 3 results, real code locations, distinct
per-finding fingerprints.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(report): complete SARIF code scanning metadata

---------
Co-authored-by: bearsyankees <bearsyankees@gmail.com>
@greptile-apps

greptile-apps Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR changes Docker workspace materialization and agent run configuration handling. The main changes are:

  • Adds Windows-specific Docker concurrency limiting.
  • Adds a STRIX_DOCKER_CONCURRENCY environment override.
  • Moves concurrency handling from backend creation to session startup.
  • Normalizes tool_choice before streamed agent runs.

Confidence Score: 4/5

These issues should be fixed before merging.

  • Docker backend startup can fail when concurrency limits are applied.
  • The Windows materialization race can still run without limits when the import path fails.
  • Configured tool-call behavior can still be changed for later cycles and child agents.

strix/runtime/backends.py and strix/core/execution.py

Important Files Changed

Filename Overview
strix/runtime/backends.py Adds Docker concurrency limit handling, but the SDK import and session startup paths can still break or skip the limit.
strix/core/execution.py Adds tool-choice normalization, but it can still overwrite configured string values on the shared run config.
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
strix/runtime/backends.py:63-66
**Wrong import path** The concurrency limit type is still loaded from SDK surfaces that do not export it in the pinned API. On the default Windows path, both import attempts can fail before the Docker backend starts. If a Windows user sets `STRIX_DOCKER_CONCURRENCY` to another value, the same import failure is swallowed and the backend continues with `concurrency_limits=None`, so workspace materialization can still run concurrently.

### Issue 2 of 3
strix/runtime/backends.py:91
**Start rejects kwargs** Passing `concurrency_limits` to `session.start()` still targets the wrong SDK call. The Docker sandbox session `start()` method accepts no keyword arguments, so any path that successfully builds `start_kwargs` fails backend startup with an unexpected-keyword error before workspace materialization can run.

### Issue 3 of 3
strix/core/execution.py:357-358
**Tool choice overwritten** This branch still rewrites every string `run_config.tool_choice`, not only the Bedrock-incompatible `"auto"` value. When a caller configures `"required"` or `"none"`, the shared `RunConfig` is mutated to `{"type": "auto"}` before `Runner.run_streamed()`, so later cycles and child agents can run with automatic tool selection instead of the configured policy.

Reviews (3): Last reviewed commit: "fix: resolve workspace materialization r..." | Re-trigger Greptile

Comment thread strix/runtime/backends.py Outdated
Comment thread strix/runtime/backends.py Outdated
Comment thread strix/runtime/backends.py Outdated
Comment thread strix/core/execution.py
Comment thread strix/runtime/backends.py
Comment on lines +63 to +66
try:
from agents.sandbox.artifacts import SandboxConcurrencyLimits
except ImportError:
from agents.sandbox import SandboxConcurrencyLimits

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Wrong import path The concurrency limit type is still loaded from SDK surfaces that do not export it in the pinned API. On the default Windows path, both import attempts can fail before the Docker backend starts. If a Windows user sets STRIX_DOCKER_CONCURRENCY to another value, the same import failure is swallowed and the backend continues with concurrency_limits=None, so workspace materialization can still run concurrently.

Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/runtime/backends.py
Line: 63-66

Comment:
**Wrong import path** The concurrency limit type is still loaded from SDK surfaces that do not export it in the pinned API. On the default Windows path, both import attempts can fail before the Docker backend starts. If a Windows user sets `STRIX_DOCKER_CONCURRENCY` to another value, the same import failure is swallowed and the backend continues with `concurrency_limits=None`, so workspace materialization can still run concurrently.

How can I resolve this? If you propose a fix, please make it concise.

Comment thread strix/runtime/backends.py
if concurrency_limits is not None:
start_kwargs["concurrency_limits"] = concurrency_limits

await session.start(**start_kwargs)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Start rejects kwargs Passing concurrency_limits to session.start() still targets the wrong SDK call. The Docker sandbox session start() method accepts no keyword arguments, so any path that successfully builds start_kwargs fails backend startup with an unexpected-keyword error before workspace materialization can run.

Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/runtime/backends.py
Line: 91

Comment:
**Start rejects kwargs** Passing `concurrency_limits` to `session.start()` still targets the wrong SDK call. The Docker sandbox session `start()` method accepts no keyword arguments, so any path that successfully builds `start_kwargs` fails backend startup with an unexpected-keyword error before workspace materialization can run.

How can I resolve this? If you propose a fix, please make it concise.

Comment thread strix/core/execution.py
Comment on lines +357 to +358
elif hasattr(run_config, "tool_choice") and (run_config.tool_choice == "auto" or isinstance(run_config.tool_choice, str)):
run_config.tool_choice = {"type": "auto"}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Tool choice overwritten This branch still rewrites every string run_config.tool_choice, not only the Bedrock-incompatible "auto" value. When a caller configures "required" or "none", the shared RunConfig is mutated to {"type": "auto"} before Runner.run_streamed(), so later cycles and child agents can run with automatic tool selection instead of the configured policy.

Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/core/execution.py
Line: 357-358

Comment:
**Tool choice overwritten** This branch still rewrites every string `run_config.tool_choice`, not only the Bedrock-incompatible `"auto"` value. When a caller configures `"required"` or `"none"`, the shared `RunConfig` is mutated to `{"type": "auto"}` before `Runner.run_streamed()`, so later cycles and child agents can run with automatic tool selection instead of the configured policy.

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Local-code scans fail on Windows Docker Desktop during sandbox workspace materialization (ExecTransportError / WorkspaceArchiveWriteError)