Commit Graph
4 Commits
Author SHA1 Message Date
sean-kim05andGitHub a5112f9433 fix(config): make env vars win over persisted JSON across all aliases (#689)
_read_json_overrides is documented to let env vars outrank the
persisted cli-config.json, but it decided per-alias and broke on the
first alias found in either env or the file. When a multi-alias field
(e.g. api_key via LLM_API_KEY/OPENAI_API_KEY) was set in the env under
one alias but stored in the file under another, the stale file value
was surfaced as an init kwarg and overrode the live env var. A
lowercase env var was also missed (settings use case_sensitive=False).

Decide whether a field is already set in the environment by checking
all of its aliases case-insensitively before consulting the file. Add
regression tests for the cross-alias and case-insensitive cases.

Closes #688
2026-07-06 11:36:41 -04:00
0xallamandClaude Opus 4.7 8414c59557 Strip narrative comments and module/helper docstrings
Five rounds of sweep across the tree. Net ~544 lines removed.

Removed:
- Section-divider banners and one-line section labels (# Display
  utilities, # ----- list_requests -----, # CVSS breakdown, etc.).
- Module-level prose docstrings on internal modules. Kept one-line
  summaries; trimmed multi-paragraph narration about SDK/Strix
  responsibility splits, cache strategies, three-source precedence.
- Internal-helper docstrings that just restate the function name —
  caido_api helpers (caido_url, get_client, view_request, etc.),
  settings-class one-liners (LLMSettings, RuntimeSettings, ...),
  UI helper docstrings.
- Args/Returns blocks on non-LLM-facing internal helpers
  (build_strix_agent, render_system_prompt, create_or_reuse,
  bootstrap_caido) — kept only the genuinely non-obvious params.
- Internal-history phrasing — "Mirrors main-branch shape",
  "pre-SDK harness", "previous lookup matched no attribute".
- Narrative comments inside function bodies that explained what the
  next line does, design rationale obvious from the surrounding code,
  or "we used to..." asides.
- Trailing periods on every error-string literal across the tool tree.
- Duplicated roundtripTime quirk comment (kept the LLM-facing copy in
  tools/proxy/tools.py).

Kept (every one names an upstream bug, vendored-code provenance, or
non-obvious data quirk):
- core/runner.py: SDK replay-with-empty-initial-input + on_agent_end
  lifecycle gap.
- runtime/docker_client.py: VERBATIM COPY block of the upstream
  _create_container body, pinned to SDK v0.14.6.
- runtime/session_manager.py: NO_PROXY for agent-browser CDP loopback.
- tools/proxy/caido_api.py: generated-pydantic Request.raw quirk,
  replay double-history pitfall.
- tools/proxy/tools.py: Caido roundtripTime=0 quirk for proxy
  captures.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-26 14:02:40 -07:00
0xallamandClaude Opus 4.7 46ff025209 feat(logging): per-scan `{run_dir}/strix.log` with scan/agent context tagging
Every scan now writes a complete log file at ``{run_dir}/strix.log``
captured from the moment ``run_dir`` is resolved through teardown.
Stdlib ``logging`` only — no parallel framework.

New ``strix/telemetry/logging.py``:
  * ``setup_scan_logging(run_dir, debug=)`` attaches a ``FileHandler``
    (DEBUG, all ``strix.*``) plus a ``StreamHandler`` (ERROR by
    default; DEBUG via ``STRIX_DEBUG=1``).
  * ``ContextVar``-backed ``scan_id`` and ``agent_id`` injected by a
    ``Filter`` so every line is auto-tagged across asyncio tasks
    without callers passing them explicitly.
  * Third-party noise (``httpx``, ``litellm``, ``openai``,
    ``anthropic``, ``urllib3``, ``httpcore``) capped at WARNING.
  * Returns a teardown handle for ``finally`` cleanup.

Wiring:
  * ``orchestration/scan.py`` calls ``setup_scan_logging`` once per
    scan after ``run_dir`` resolves; sets scan_id; tears down in
    ``finally``. Adds INFO logs for sandbox bring-up + scan
    start/end.
  * ``orchestration/hooks.py`` sets/clears ``agent_id`` ContextVar in
    ``on_agent_start`` / ``on_agent_end`` and emits INFO for agent
    lifecycle, DEBUG for every tool start/end and LLM call.
  * ``interface/main.py`` drops the ``setLevel(ERROR)`` silencer.

Coverage expanded across ~20 files (orchestration, agents, runtime,
llm, tools, interface, config, skills) with INFO for lifecycle and
DEBUG for verbose detail. Per the system instructions in
``logger.warning(f"…{e}")`` were converted to module logger calls.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 23:35:01 -07:00
0xallamandClaude Opus 4.7 1e641e56ce refactor(config): pydantic-settings revamp + drop `is_whitebox` plumbing
Replaces 200+ lines of bespoke env-loader / persist / change-detection
machinery with ``pydantic_settings.BaseSettings`` (already a transitive
of ``openai-agents → mcp``, no new direct dep).

What was wrong with ``Config``:

- 14 knobs flat in one namespace, weak grouping by comment-block.
- ``Config._applied_from_default`` and ``Config._config_file_override``
  were externally mutated from ``interface/main.py:532-534``. Private
  members were part of the public contract.
- Stringly-typed values: every caller had to coerce
  (``int(Config.get("llm_timeout") or "300")``,
  ``... not in {"0", "false", "no", "off"}``).
- Dead knob: ``strix_llm_max_retries`` declared, persisted, listed in
  ``_LLM_CANONICAL_NAMES`` — zero readers (``DEFAULT_RETRY``
  hardcodes ``max_retries=5``). Dropped.
- ``_LLM_CANONICAL_NAMES`` tuple maintained alongside class vars —
  duplicate source of truth.
- ``_tracked_names()`` introspected ``vars(cls).items()`` filtered on
  ``(v is None or isinstance(v, str))`` — fragile.
- Awkward path: ``strix/config/config.py`` inside ``strix/config/``
  with ``__init__.py`` just re-exporting.
- Dual access for the same fact: ``web_search`` read
  ``os.getenv("PERPLEXITY_API_KEY")`` while ``main.py`` read
  ``Config.get("perplexity_api_key")``.

New shape:

- ``strix/config/settings.py`` — typed dataclass tree:
  ``Settings.{llm,runtime,telemetry,integrations}``. Each sub-model is
  its own ``BaseSettings`` so it reads env independently. Field-level
  ``alias=`` and ``validation_alias=AliasChoices(...)`` mirror the
  existing flat env-var names — user-facing env contract is unchanged.
  Bool fields auto-parse ``"0"``/``"false"``/``"no"``/``"off"``;
  int fields auto-coerce.
- ``strix/config/loader.py`` — thin ``load_settings()``,
  ``apply_config_override(path)``, ``persist_current()`` with module
  cache. JSON file reader walks aliases to populate sub-models, dropping
  entries already covered by env (so env still wins).
- 13 callsites migrated from ``Config.get("...")`` to
  ``load_settings().<group>.<field>``.
- ``posthog._is_enabled()`` collapses to one line.
- ``--config <path>`` flow simplified: one
  ``apply_config_override(...)`` call replaces three lines of
  class-private mutation.

Drive-by — drop ``is_whitebox`` from ``scan_config`` dict:

- It was being derived as ``bool(args.local_sources)`` in three places
  (``cli.py``, ``tui.py``, ``main.py``) and stuffed into the dict for
  ``entry.py`` to read back. The fact is fully derivable from
  ``scan_config["targets"]`` — any target with ``type == "local_code"``.
- New helper ``is_whitebox_scan(targets)`` in ``interface/utils.py``
  alongside the other target-classification utilities.
- ``entry.py`` computes once; ``main.py``'s posthog start uses the same
  helper. Triplicate derivation gone.

Verified: ruff at baseline (3), mypy at baseline (69). Six smoke tests
pass — defaults / JSON-only / env-wins-over-JSON / alias-chain
fallback / bool parsing / ``is_whitebox_scan``.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 16:05:40 -07:00