fix: repair the Python suite so the aggregate gate can go live - #187
Merged
Conversation
Both are environment-independent source bugs — code paths that could never have run — surfaced while investigating why main's Python suite is red (six of eight CI slices), which blocks the aggregate gate in #186. plugins/memory/mem0: _start_prefetch(self, query) computed `effective_user_id = user_id or self._user_id`, but user_id is not one of its parameters, so any call raised NameError. It is a copy of the identical line in sync_turn fifty lines below, where user_id genuinely is a parameter. The value was never read — the closure filters via self._read_filters() — so the line is deleted rather than the parameter added. hermes_cli/gateway: the launchd plist builder assembled `core_args`, computed a `use_wrapper` flag, then called `prog_args.append(...)` — a name never assigned. All three were mutually inconsistent, so the function could not run to completion. The surrounding comment is unambiguous about intent (the wrapper execs the python + module args passed after it; fall back to direct-python when it is missing) and the three failing tests assert nothing about wrapper shape, so the comment is the spec: the wrapper path goes in front of core_args when usable, core_args alone otherwise. Deliberately not included: the tests importing _expand_value_from_environ and _redact_command_for_display, neither of which exists. Implementing them means inventing semantics in a security-adjacent config loader, and whether they were removed or never written is not answerable from a shallow clone. Also records a correction in the system log: my earlier "69 failing tests" figure was this sandbox, not the repo. Most of it is missing optional extras and a different aiohttp than uv.lock, and some of those failures would be wrong to "fix" — the lazy-deps pin test is correctly detecting local drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
…sses
Second instalment of the Python-suite repair. CI confirmed the first: slice 6
went 5 files/20 tests -> 4 files/17 tests with test_mem0_v3.py gone. This
clears the remaining four files in that slice.
gateway/status.py had TWO definitions of looks_like_gateway_command_line — the
canonical one at line 444 delegating to _gateway_command_subcommand (shlex
tokenization, profile-selector stripping, case-folding), and a hand-rolled
substring scan at line 569. Python keeps the last, so every caller silently got
the naive one. looks_like_gateway_runtime_command_line was duplicated the same
way. The substring version fails precisely the cases the canonical one exists
for: `--profile work gateway run` (the flag splits the substring), `--profile
gateway gateway run` (the profile value shadows the subcommand), bare
`gateway`, and `GATEWAY RUN`. _scan_venv_blockers' docstring warns about this
in as many words, so the regression it describes had reappeared one module
over. Deleting the duplicates restores the canonical behaviour unchanged —
14 tests across two files.
test_kanban_db referenced bare `hermes_state` at line 907 while line 922 does a
plain `import hermes_state`, making the name function-local for the whole body,
so the earlier reference could never resolve. It was redundant anyway: line 903
already clears that same set via the _hermes_state alias.
test_modal_snapshot_isolation died before reaching its subject: modal.py calls
lazy_deps.ensure("terminal.modal") first, and that asks whether the
distribution is installed, which a sys.modules stub cannot satisfy. The test
already stubs hermes_cli, tools, tools.environments and modal, so lazy_deps is
one more stub, not the thing under test. The production check is deliberately
untouched — it is security-adjacent, and loosening it to accept pre-imported
modules is not a change to make for a test's convenience.
Also corrects a claim in the log: I said test_gateway_command_line_matcher
fails in CI but passes locally, and used it to argue the environments disagree.
It fails locally too — verified by stashing the fix. It was missing from my
inventory because the background capture kept only the tail of the full-suite
output, so the file list was truncated.
No regressions: tests/gateway/ + tests/hermes_cli/ run 9,102 passing / 24
failing across 12 files, none of them files touched here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
… to ids
cron/scheduler: gateway/run.py's cron-ticker supervisor imports
get_ticker_heartbeat_age and ticker_heartbeat_is_stale from cron.scheduler.
Neither existed there — the age reader lives in cron/jobs.py and the staleness
predicate was defined nowhere in the repo. The import sits inside the
supervisor loop, so the watchdog meant to restart a wedged ticker would itself
have died on ImportError the first time it checked. Added the predicate and
re-exported the age reader beside it, since the supervisor and its tests both
treat cron.scheduler as the ticker's public surface.
The rule is taken from the existing docstrings rather than invented:
age > interval * stale_multiplier, and an unknown age is NOT stale —
get_ticker_heartbeat_age returns None on a missing or torn read and documents
that callers treat that as "cannot determine", not "dead". Restarting on a torn
read would turn a filesystem hiccup into a restart loop.
hermes_cli/providers: get_label("xai") returned "xai", which collapsed the xAI
API-key entry onto its own id and made it indistinguishable from the OAuth
sibling in the model picker. My first fix was wrong and the test caught it — I
added a fallback at the end of get_label, but that line is unreachable for this
case: get_provider returns a synthesized def for Hermes-only providers, with
name=_LABEL_OVERRIDES.get(canonical, canonical), so the id was already baked in.
Fixed at that source via one _display_name() helper shared by both call sites:
overrides, then the registry in hermes_cli.models (which already carries
ProviderEntry("xai", "xAI", ...)), then the id.
Not fixed by adding an _LABEL_OVERRIDES entry: that duplicates a name the
registry owns, which is exactly the duplication behind the gateway/status.py
bug in the previous commit. models.dev is a remote catalog, so this path is
also what every provider hits whenever it is unreachable.
No regressions: 774 tests across 90 provider/model files pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
Five names are imported from hermes_cli.config and none exist; two of them by production code in hermes_cli/backup.py. The restore path swallows the ImportError, so a config restore silently never reseals the integrity baseline and the watchdog reads an authorized restore as tampering. The machinery exists unwired in skills/devops/config-integrity-watchdog/, whose seal(quiet=True) parameter is documented for exactly this caller. Reconstructing it is four functions of security-adjacent behaviour, so it is reported for a decision rather than folded into a make-CI-green pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
hermes_cli/slack_cli.py used `as_yaml` in two places — the render call and the file extension — without ever assigning it, so `hermes slack manifest` raised NameError on every invocation. Every other option in that function is read via getattr(args, ..., default); as_yaml now is too, defaulting to False (JSON), which is what the default write target (slack-manifest.json) and the test's json.loads(stdout) both assume. The generated app manifest also omitted the channels:join bot scope, so `hermes slack invite --all` would fail per-channel with missing_scope on an otherwise valid install — conversations.join requires it. Not fixed here: test_gateway_platform_gating asserts _builtin_setup_fn returns bespoke setup functions for telegram/slack/matrix. Those return None by design — all three moved into plugins/platforms/*/adapter.py, and I verified each registers setup_fn=interactive_setup, so the invariant the test documents (they must not fall through to the generic wizard, which would break Matrix's empty-token password login) still holds. The test asserts the removed route rather than the invariant; replacing it properly needs the plugin registry's lookup API, so it is documented rather than deleted to make CI green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
Eight functions were referenced by tests and production but did not exist in
hermes_cli/config.py: get_config_lock_path, get_config_seal_path,
config_write_lock, config_read_lock, seal_config, verify_config_integrity,
_reseal_git_backed_integrity_baseline and restore_config. Three test files
failed at collection because of it, and hermes_cli/backup.py imported two of
them behind `except ImportError: pass` — so a config restore silently never
resealed and the watchdog read it as tampering.
They were deleted, not never written: config.py still imports `fcntl as _fcntl`
and `msvcrt as _msvcrt` at module scope and uses neither, which is exactly what
the lock helpers needed, and hermes_cli/config_integrity_cli.py still provides
the _find_core_module bridge the tests import.
save_config() now seals both baselines after a successful write — the local
config.yaml.sha256 sidecar and the git-backed JSONL log in the operator's
dotfiles repo. A write through save_config is authorized by definition, so
leaving those stale is what made the watchdog flag the model scanner.
Decisions worth calling out. A missing config and a missing seal both verify OK:
absence of a baseline is not a mismatch, and failing there would flag every
pre-existing install as compromised. Sealing never fails a write — the
git-backed half shells into a repo outside Hermes' control that can be absent,
non-git, mid-rebase or read-only. That reseal is a no-op unless the dotfiles
directory already exists, since the skill's seal() would otherwise mkdir a
baseline on machines that never opted in. Locks degrade to no-lock rather than
raising, and Windows readers go unlocked because msvcrt has no shared mode.
restore_config always writes a pre-restore backup, including when the live file
is unreadable — a corrupt config is the usual reason a restore is happening.
_expand_value_from_environ leaves an unset reference verbatim rather than
blanking it, so a typo'd variable stays legible instead of becoming an empty API
key. That one rule also resolves the apparent contradiction in its tests:
$TEST_VAR expands and literal-$var does not, purely because one name is exported
and the other is not. It is not wired into config loading — _expand_env_vars
walks whole documents and resolves ${env:NAME} SecretRefs with different
warning behaviour, so delegating would change semantics rather than restore a
helper.
Verified against the same baseline command as before: tests/gateway/ +
tests/hermes_cli/ go from 9,102 passing / 24 failing across 12 files to 9,139
passing / 11 failing across 8, with collection errors down from 3 to 1. Strictly
better on every axis, so the new sealing side effect regressed nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
Ruff (PLW1514) and the Windows-footgun checker both flagged the new `open(lock_path, "a+")`: text mode without an explicit encoding, which on Windows also brings newline translation. The file is only ever a lock handle — never read or written as text — so binary sidesteps both concerns rather than papering over them with an encoding argument. Both checks pass locally: `ruff check hermes_cli/ cron/ gateway/` is clean and `scripts/check-windows-footguns.py --all` reports no footguns across 927 files. Unrelated, for the record: the `gitleaks (diff)` failure on 25b74f9 is not a secret. Its install step died on `curl: (22) ... error: 503` fetching the release tarball, so the scanner never ran — but the job's failure-notice step prints "gitleaks flagged a secret in this diff" unconditionally, making a transient CDN failure look identical to a real detection. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
Ninth instance of the same defect class: _should_probe_ollama_api_show was referenced by tests but defined nowhere, so the file raised NameError. Regression guard for NousResearch#31555 — /api/show is an Ollama-server endpoint, so firing it at a hosted provider costs a doomed HTTP roundtrip (up to the 3s probe timeout) on a path that runs during agent startup. The rule comes from the three tests: skip for a known hosted provider, infer the provider from the base URL when none is named (which is how the OpenRouter case reached the probe at all), otherwise allow. The first attempt gated on _PROVIDER_PREFIXES and was wrong — that set contains plain `ollama`, because it answers "can this appear as a model prefix?", not "is this hosted?". test_allows_ollama_provider caught it. Local providers are now checked first via an explicit _LOCAL_OLLAMA_PROVIDERS; ollama-cloud stays out of it, being the hosted service that serves no /api/show. Found by reading CI's slice list rather than my sandbox: tests/agent/ — 351 files — had never been in any of my local sweeps, so this bug was absent from every inventory I had built. That also corrects my earlier claim that the remaining failures were all environmental: the environmental ones are real (root defeats chmod-based permission tests, no IPv6 for bind assertions), but that was a statement about a set I had not measured. tests/agent/: 3,926 passing / 12 failing across 7 files, none of them this one. ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
First full-suite run with a repaired venv — the first complete enumeration this session has had. 62 failures across 28 files, roughly half of them this sandbox's missing optional extras rather than repo bugs. Two of the real ones are features that do not exist rather than bugs: _redact_command_for_display (tools/approval) and per-model credential lockout (CredentialPool.select/_mark_exhausted take no model kwarg and the module has zero lockout machinery, against a 185-line spec). Both left for a decision. The contrast with the config-integrity cluster is the reason: that one had orphaned fcntl/msvcrt imports, a surviving bridge module and a skill implementing the machinery, so restoring it was recovery. There is no such trace here, and inventing cooldown semantics beside a DEAD/EXHAUSTED state machine that governs billing-bearing credentials is a feature decision. Also records two corrections. My first regression check was vacuous — git stash takes only uncommitted work, so with everything committed I compared the branch against itself and read the identical numbers as evidence. A worktree at origin/main gives the real answer: the tests/agent/ failures are pre-existing. And 'same compromised run' was wrong; runs are mixed, and log length (~180 vs ~1000+ lines) discriminates a setup death from a real test run, not the run id. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
pyproject.toml and uv.lock both pin aiohttp==3.14.3; tools/lazy_deps.py still pinned 3.14.1 in four places, and cbor2==6.1.2 against a uv.lock of 6.1.3. The consequence is in the failing test's own message: pyproject extras pins must match LAZY_DEPS pins for every shared package, otherwise `hermes update` downgrades the package. Every one of those lines carries a CVE list including an RCE (CVE-2026-34993), so installing any lazy feature — a Discord, Teams or Matrix platform, or the Modal terminal — would have pulled aiohttp back below the patched floor on a machine that already had the fix. Bumped the four aiohttp pins to 3.14.3 and cbor2 to 6.1.3, matching uv.lock exactly. Also corrected the comments that named the old version beside the new pin (`# aiohttp 3.14.1: CVE-…` next to `aiohttp==3.14.3`, once here and three times in pyproject.toml) — a version string that disagrees with the pin it annotates is the drift that produced this. Correction to my own triage: I had filed these two tests under "sandbox drift" because the message mentions an aiohttp version, assuming it reflected my stale venv. It does not — they compare declarations across pyproject/lazy_deps/uv.lock and never inspect what is installed, and CI fails them identically. test_project_metadata + test_packaging_metadata: 13 tests, 0 failures. pyproject.toml parses; ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
A 429 for one model says nothing about the others, but the pool could only bench a credential globally — so one rate-limited model removed the key from rotation everywhere. Correction to my own earlier triage: I reported that credential_pool.py had "zero model-lockout machinery" and deferred this as inventing a feature. That was wrong — I grepped for model_lockout/locked_models/model_locks; the feature is called exhausted_models, and two pieces were already shipping. _model_exhausted_until() lives in credential_pool.py and reads exactly the record shape the tests specify, and hermes_cli/auth_commands.py renders entry.extra["exhausted_models"] in auth status output. The display side has been live against a field nothing ever wrote — the same partial-deletion signature as the config-integrity cluster, consumers and helpers surviving without the producer. _mark_exhausted(model=...) now records the bench in extra["exhausted_models"] and leaves the credential-wide status alone. Terminal auth failures are deliberately excluded from that path: a revoked or invalid token is dead for every model, and recording it per-model would keep the credential in rotation for the rest, failing instantly on each. The existing _is_terminal_auth_failure classifier makes that call, so the DEAD/EXHAUSTED distinction is unchanged. select(model=...) filters entries benched for that model, keyword-only so existing no-argument callers are untouched. _prune_expired_model_lockouts() drops lapsed records, mirroring what _available_entries(clear_expired=True) does for the credential-wide status, and runs before filtering so a just-expired lockout frees its credential in the same call. reset_statuses() now counts a per-model lockout as status — a credential can be benched for a model with every credential-wide field clear, and reset must free that too rather than reporting "0 reset". tests/agent/test_model_lockout.py 4/4. Credential, pool and auth suites: 477 tests across 67 files, 0 failures. tests/agent/ overall 3,930 passing / 8 failing, up from 3,926 / 12 — no regressions in a module governing billing-bearing credential rotation. ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
…ent homes Two fixes in tools/approval.py, five tests. _redact_command_for_display was missing. Searching for consumers first — the lesson from the credential-pool commit — showed agent/redact.py already carries the Slack xapp-/xox[baprs]- patterns, URL-userinfo handling and redact_sensitive_text(), and that it satisfies every assertion in the spec unchanged. So the missing piece was a thin wrapper, not a redaction engine. It earns its own name from the contract in the test docstring: the original command still runs, this is display-only. Motivated by the 2026-07-13 incident — a heredoc appending a Slack app-level token to ~/.hermes/.env, where the approval prompt reproduced the command in full, so asking permission published the credential more widely than running it would have. The variable name survives so the reviewer can see which secret is being written; only the value is masked. force=True and redact_url_credentials=True come from redact_sensitive_text's own docstring — force is documented for "safety boundaries that must never return raw secrets regardless of the user's global logging redaction preference", and an approval echo into Slack is an egress boundary rather than the tool flow the URL default protects. The fifth failure was TestSensitiveRedirectPattern, and it corrects me twice. I first checked /home/alice/.ssh/authorized_keys, saw False, and called it a general security hole — wrong, that is not the current user's home and the fold is deliberately scoped to the running user's. The real issue is narrower: _home_prefix_fold_regex required two components below the root, which correctly rejects / and /home (folding /home would rewrite every user's path to ~), but /root is a legitimate home with one component. So on any root-run deployment — the Docker image, CI containers, sudo — ~/.ssh/authorized_keys was guarded while the identical /root/.ssh/authorized_keys was not. Replaced the count with the property the guard wants: reject a bare root, and reject a single component only when it contains other homes (home, users, var, mnt, …). Verified both directions: /root/.ssh/authorized_keys now flags, ~ still flags, /home/other/.ssh/authorized_keys is still left unfolded. test_approval.py 101 passing / 0 failing (was 96/5). tests/tools/ 5,375 passing / 18 failing across 3 files, all pre-existing: daytona (optional extra absent), execution-flag detection, and the chmod-under-root artifact. ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
…WAL checkpoint Three defects of the same shape: the mechanism survived, the thing that uses it did not. cron/model_policy.py is a complete, documented guardrail — it refuses to route a legal/finance cron job to a denylisted or non-allowlisted provider/model. It had zero callers. `enforce_privileged_model_route()` now runs in create_job() on the assembled record and in update_job() on the *merged* record (the privileged tags come from the stored prompt/skills while the offending route arrives in the update, so neither half is conclusive alone). Both raise before save_jobs(), so a rejected job never reaches disk. hermes_state._try_wal_checkpoint() executed `PRAGMA wal_checkpoint(TRUNCATE)` under a 16-line docstring explaining that TRUNCATE is what caused B-tree corruption on large databases (NousResearch#45383) and that the periodic path must be PASSIVE. The warning string and the tests still said PASSIVE; only the pragma had reverted. Now PASSIVE again; close() and the pre-VACUUM path keep TRUNCATE, as documented. test_gateway_platform_gating asserted the retired lookup: telegram/slack/ matrix moved to plugin-registered `setup_fn=interactive_setup` in NousResearch#41112, and _configure_platform() tries the registry entry first. The invariant — these three never fall through to _setup_standard_platform(), whose mandatory-first-var behaviour would break Matrix's password-login path — is now asserted against the mechanism actually in use. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
…ngs.json
The block was removed deliberately on 2026-07-28 (see docs/system-log) in
favour of .mcp.json.example: Claude Code documents ${VAR} expansion for
.mcp.json and ~/.claude.json but not for settings.json, so a server
declared there needs a literal absolute path and starts on exactly one
machine, failing silently everywhere else. An unrelated dependabot npm
bump (7e38fa5) re-added a copy carrying a hardcoded /Users/<name>/ path.
The `memory` server moves into .mcp.json.example so #134's
@modelcontextprotocol/server-memory@0.6.2 pin survives the removal (0.6.3
does not exist on npm). The `hooks` block is untouched — AGENTS.md is
explicit that settings.json keeps hooks only, and $CLAUDE_PROJECT_DIR
*does* expand there, which is how the tracked
.claude/hooks/session-start.sh is reached.
test_no_repository_local_claude_permissions_file asserted the file must
not exist at all, which would orphan that hook. It now asserts what
AGENTS.md actually requires: no `permissions` and no `mcpServers` key.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
…t refused
Found by the E2E validation AGENTS.md requires before wiring a dead module
into a live path — and it would have shipped a regression.
detect_privileged_workload_tags() scanned for its keywords as plain
substrings. Several of them are acronyms: "je" (journal entry) matches
"project", "subject", "rejected" and "Jenkins", and "books" matches
"notebooks". Harmless while the detector had no callers. Now that the
route gate runs on create_job()/update_job(), an over-broad match doesn't
just mislabel a job — a privileged tag plus any pinned provider/model
outside the six-entry allowlist refuses the job outright. "Summarize the
project changelog" with a model override would have been rejected.
Keywords now match at a token start, so inflections still tag
("reconciliation" -> "reconciliations", "attorney" -> "attorneys") while a
keyword buried inside an unrelated word does not. Acronyms are anchored at
both ends. The over-tagging the module documents as deliberate is intact:
every keyword that matched a real word before still matches.
E2E against a temp HERMES_HOME through the real create_job/update_job
chain: an ordinary pinned job saying "project" lands, a denied route is
refused without persisting, an allowlisted privileged route lands, a bad
update leaves the stored job untouched, and ordinary updates still work.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
…t test test_references_parallel_interrupt_aborts_wait raised the interrupt flag from inside the "fast" stub, before that stub's response reached its future. A poll landing in the gap saw an interrupt with the reference still in flight, so the slot was marked skipped and the assertion failed — rare locally, hit on a loaded CI runner. The production path is not at fault: _run_references_parallel() records completed futures before it checks the flag, and the interrupted sweep preserves any result that lands in between. The test simply wasn't creating the scenario it described. The interrupt now comes from the progress_callback the fan-out invokes right after recording a completed reference — "the interrupt arrives right after the fast reference finishes", ordered by the production code instead of by thread scheduling. Verified in both directions: forcing the old ordering with a 150ms linger reproduces the exact CI failure string, and the new ordering under the same delay returns the real output. agent/moa_loop.py is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
dizhaky
marked this pull request as ready for review
August 12, 2026 18:58
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What does this PR do?
main's Python suite is red — six of eight CI slices. #186 makes the aggregate gate actually block merges, so it is held as a draft until this is green; otherwise the gate's first act would be to block everything.This is the first instalment: two environment-independent source bugs, both code paths that could never have run.
Fixed
plugins/memory/mem0/__init__.py—NameError: name 'user_id'(3 tests, CI-confirmed)_start_prefetch(self, query)computedeffective_user_id = user_id or self._user_id, butuser_idis not one of its parameters. It's a copy of the identical line insync_turnfifty lines below, whereuser_idgenuinely is a parameter. The computed value was never read — the closure filters viaself._read_filters()— so the line is deleted rather than the parameter added.hermes_cli/gateway.py—NameError: name 'prog_args'(3 tests)The launchd plist builder assembled
core_args, computed ause_wrapperflag, then calledprog_args.append(...)— a name never assigned. All three were mutually inconsistent, so the function could not run to completion.The surrounding comment states the intent precisely ("the wrapper execs the python + module args we pass after it… if the wrapper is ever missing/non-executable, fall back to the direct-python form"), and the three failing tests assert nothing about wrapper shape. So the comment is the spec: the wrapper path goes in front of
core_argswhen usable,core_argsalone otherwise.A correction I owe on my own sizing
I initially reported "69 failing tests across 24 files" from a local full-suite run. That number is my sandbox, not this repo.
The venv is missing optional packages (
PIL,anthropic,hindsight_client_api) and carries a differentaiohttpthanuv.lock— 34 of those failures areFeatureUnavailable: lazy installs disabledorModuleNotFoundErrorfor optional extras. And some would be actively wrong to "fix":test_every_lazy_deps_exact_pin_matches_uv_lockfails locally withaiohttp=['3.14.1'], expected ['3.14.3'], which is the test correctly detecting drift in my environment. Editing the pin to match would corrupt the real one.The environments disagree in both directions —
tests/gateway/test_gateway_command_line_matcher.pyfails in CI (9 tests) and passes locally. CI is the oracle here, and part of this PR's job is to make CI enumerate the true remaining set.Deliberately not fixed
tests/hermes_cli/test_config_env_expansion.py::TestExpandValueFromEnviron(5) andtests/tools/test_approval.py(4) do function-level imports of_expand_value_from_environand_redact_command_for_display, neither of which exists.Implementing them means inventing semantics — the config one would have to expand
${VAR}, honour${VAR:-default}, expand bare$VAR, and yet leaveliteral-$varuntouched — in a security-adjacent config loader. Whether those helpers were deliberately removed or never written isn't answerable from a shallow clone. They wait for evidence rather than a guess.Related Issue
No issue. Discovered while landing #186.
Type of Change
Changes Made
plugins/memory/mem0/__init__.py— delete the undefined-name line.hermes_cli/gateway.py— assignprog_argsper the documented wrapper intent.docs/system-log/2026-08-12.md— write-up, including the sizing correction.How to Test
scripts/run_tests.sh tests/plugins/memory/test_mem0_v3.py— was 3 failures, now green.scripts/run_tests.sh tests/hermes_cli/test_gateway_service.py— was 3 failures, now green.Checklist
Code
scripts/run_tests.sh(notpytestdirectly, per AGENTS.md)Documentation & Housekeeping
docs/system-log/2026-08-12.md)cli-config.yaml.example,CONTRIBUTING.md/AGENTS.md, tool schemasScreenshots / Logs
Both files green after the change:
Next step is deliberately CI-driven: this PR's own run will enumerate what actually remains red in CI, as opposed to what is red in my sandbox. I'll work that list rather than the local one.
Generated by Claude Code