fix(*): refresh onboard model defaults and gate EverOS spawns on inotify headroom - #372
Conversation
The Qwen3-Reranker-4B model has been retired from OpenRouter, so the onboard wizard recommended a reranker users can no longer reach. Switch the shipped example and the EN/ZH recommendations to the 8B model that OpenRouter still serves.
With fs.inotify.max_user_instances exhausted, the spawned server's watcher dies at boot with an OSError that only surfaces as a cryptic exit in the dead-child log. Check per-user headroom before spawning: raise the cap when the process may, otherwise fail fast with the exact sysctl commands. A dead child whose log blames inotify gets the same hint appended to the error.
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the inotify preflight undercounts valid instances, so this revision should not merge yet; see the inline note.
I reviewed the repository rules and domain map, the actual dd38d2a0...95610244 PR delta (plus the stale local github/main extra), the surrounding EverOS onboarding and backend callers, affected history, backward compatibility and non-Linux behavior, and whether tests were weakened. The 8B slug is present in OpenRouter's current rerank collection, and the wizard flow remains consistent.
Verification: uv run pytest tests/test_everos_server.py tests/test_cli_onboard_commands.py -q produced 326 passed and 1 failed. The failure is test_ps_recognises_a_process_whose_command_line_says_everos on untouched code, where this environment's ps truncates the command marker. The focused new tests produced 15 passed. Ruff check, Ruff format check, and git diff --check passed.
| blob = entry.read_bytes() | ||
| except OSError: | ||
| continue | ||
| if any(line.startswith(b"inotify") for line in blob.splitlines()): |
There was a problem hiding this comment.
The counter misses inotify instances that have no watches. A freshly created inotify fd still consumes max_user_instances, but its /proc/<pid>/fdinfo/<fd> contains only the generic pos/flags/mnt_id/ino fields; the inotify ... lines appear only after watches are added. I confirmed this on Linux by holding 16 zero-watch instances: _inotify_usage() stayed at (5, 1024). If zero-watch instances exhaust the cap, this check returns used < limit, allows the EverOS spawn, and then also fails to append the sysctl hint after the child dies. Please identify the descriptors themselves (for example via the /proc/<pid>/fd target anon_inode:inotify) rather than using watch-detail lines as the instance marker.
There was a problem hiding this comment.
Confirmed and fixed: zero-watch fds show no inotify lines in fdinfo yet consume the cap (reproduced locally with 3 real instances). _inotify_usage now counts /proc//fd links that read anon_inode:inotify, and a new real-kernel test (test_zero_watch_instances_count_against_the_cap) pins the zero-watch behavior, skipping only when the host cap is genuinely exhausted. Full suite locally: 7119 passed; diff coverage 100.00% (58/58).
The diff-coverage gate flagged nine uncovered lines: the OSError handlers in the usage/raise helpers and the dead-child enrichment branch. The enrichment test passed by accident (the patched gate raised before the spawn), so it now passes the gate on the first call and enriches on the second. The fake proc tree gains dangling-symlink entries to exercise the read handlers, and a new test covers the unreadable-cap branch.
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the previously reported inotify accounting defect remains unresolved, so this revision should not merge yet.
This new head adds test coverage only and leaves raven/plugin/memory/everos/_server.py unchanged. The updated dead-child test now reaches the intended enrichment branch, and the added OSError coverage is sound; I found no separate issue in the new delta. I rechecked the repository rules and domain terms, the full diff, surrounding callers and history, backward compatibility and non-Linux behavior, and whether tests were weakened. The existing inline blocker still applies.
Verification: uv run pytest tests/test_everos_server.py tests/test_cli_onboard_commands.py -q produced 327 passed and 1 failed. The failure remains test_ps_recognises_a_process_whose_command_line_says_everos on untouched code, where this environment's ps truncates the command marker. Ruff check, Ruff format check, and git diff --check passed.
A freshly created inotify fd consumes max_user_instances before its first watch, but fdinfo carries no inotify lines until then, so the headroom check undercounted zero-watch instances and could let a spawn through on an exhausted cap. Count /proc/<pid>/fd links that read anon_inode:inotify instead, and pin the behavior with a real-kernel zero-watch test.
|
Addressing both blocking reviews: d126ec3 rewrites _inotify_usage to count /proc//fd links that read anon_inode:inotify, so zero-watch instances (which fdinfo cannot see until the first watch) are counted too. The reviewer's counterexample is now pinned by a real-kernel test (test_zero_watch_instances_count_against_the_cap) that allocates two zero-watch instances and asserts the usage grows by two; it skips only when the host cap is genuinely exhausted mid-suite. Local verification: full suite 7119 passed (2 pre-existing root/chmod environment failures), diff coverage 100.00% (58/58), ruff clean. Please re-review. |
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the original zero-watch undercount is fixed, but a Python 3.12 procfs race and an unguarded Linux-only test still need changes; see the inline notes.
I re-reviewed the repository rules and domain terms, the full diff and new delta, surrounding EverOS callers and history, backward compatibility across supported platforms, and whether tests were weakened. Counting /proc/<pid>/fd targets correctly resolves the prior finding, and the replacement unit coverage exercises that representation.
Verification: the focused inotify/reranker tests produced 17 passed. uv run pytest tests/test_everos_server.py tests/test_cli_onboard_commands.py -q produced 328 passed and 1 failed; the failure remains the unrelated, untouched ps command-truncation test. Ruff check, Ruff format check, and git diff --check passed. A direct Python 3.12 reproduction confirmed the lazy-iterdir exception described inline.
| try: | ||
| if pid_dir.stat().st_uid != me: | ||
| continue | ||
| entries = (pid_dir / "fd").iterdir() |
There was a problem hiding this comment.
On the project's minimum Python 3.12, Path.iterdir() is lazy: this assignment only creates a generator, and its os.listdir() runs at for entry in entries, outside the try. If a same-uid process exits after pid_dir.stat() but before that first iteration, FileNotFoundError escapes _inotify_usage() and aborts ensure_everos_server() instead of skipping the vanished PID. I reproduced that exact sequence with /usr/bin/python3.12. Please materialize or iterate the fd directory inside an OSError handler so normal /proc churn cannot randomly prevent memory startup.
There was a problem hiding this comment.
Fixed in 1a5cb68: the per-pid iteration (stat + iterdir + readlink) now sits entirely inside the try block, and the fake proc tree gained a 947/fd-as-regular-file case so iterdir raising NotADirectoryError at iteration time is caught by the same guard.
| from raven.plugin.memory.everos import _server | ||
|
|
||
| libc = ctypes.CDLL(None, use_errno=True) | ||
| libc.inotify_init.restype = ctypes.c_int |
There was a problem hiding this comment.
This test resolves the Linux-only inotify_init symbol before any condition that can skip it. Raven explicitly supports macOS, where ctypes.CDLL(None).inotify_init raises AttributeError, so the ordinary unit suite now fails on a supported platform even though production correctly treats missing procfs as unmeasurable. Please guard the test for Linux (or probe the symbol and skip) before accessing it.
There was a problem hiding this comment.
Fixed in ec12580: the test is now guarded with skipif(sys.platform != "linux", reason="inotify is Linux-only"), matching the repo's platform-guard convention, so macOS runs skip it before any symbol access.
Path.iterdir() opens the directory lazily, so a PermissionError from listing /proc/<pid>/fd (a dumpable=0 process, seen on the CI runner) escaped the try block at first iteration and crashed the gate. Move the iteration under the per-pid guard so an unreadable process is skipped like any other OSError.
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the Python 3.12 procfs race is fixed, but the unguarded Linux-only kernel test remains; see the existing inline note.
I rechecked the repository rules and domain terms, the full diff and new delta, surrounding callers and history, backward compatibility, and whether tests were weakened. Moving the fd iteration inside the per-PID OSError guard correctly handles lazy Path.iterdir() failures on Python 3.12, and the added non-directory fixture covers that path. I found no new defect in this delta. The macOS portability blocker is unchanged.
Verification: uv run pytest tests/test_everos_server.py tests/test_cli_onboard_commands.py -q produced 328 passed and 1 failed. The failure remains the unrelated, untouched ps command-truncation test. Ruff check, Ruff format check, and git diff --check passed.
The test resolves the Linux-only inotify_init symbol before any condition can skip it, so the suite fails with AttributeError on macOS, which Raven supports. Skip it on non-Linux platforms where production already treats missing procfs as unmeasurable.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The non-Linux guard runs at collection time before the test resolves inotify_init, so it fixes the supported-macOS failure without weakening Linux coverage. I rechecked the repository rules and domain terms, the full diff and latest delta, surrounding callers and history, backward compatibility, and whether tests were weakened. The earlier zero-watch accounting and Python 3.12 procfs-race findings remain correctly fixed, and I found no new issue worth raising.
Verification: uv run pytest tests/test_everos_server.py tests/test_cli_onboard_commands.py -q produced 328 passed and 1 failed. The failure remains the unrelated, untouched ps command-truncation test. Ruff check, Ruff format check, and git diff --check passed.
The wizard still pointed at two retired-or-stale generations: the memory-LLM capability floor and the multimodal recommendation, plus the Azure OpenAI default deployment. Refresh all three to models OpenRouter and Azure serve today: gpt-5.6-luna, google/gemini-3.7-flash, and gpt-5.6-sol.
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the new GPT-5.6 defaults are incompatible with the Chat Completions payloads their clients actually send; see the two inline notes.
I reviewed the new delta and the complete github/main...HEAD diff, including callers and relevant history, backward compatibility, AGENTS.md/CLAUDE.md and the runtime glossary, and whether tests were weakened (the new commit changes no tests). I also checked current vendor documentation: all three new model IDs exist, and Gemini 3.7 Flash has the multimodal inputs this role requires.
Verification:
uv run pytest tests/test_cli_onboard_commands.py tests/test_azure_openai_provider.py tests/test_everos_server.py -q: 337 passed, 1 failed. The failure is the pre-existing, unchanged real-psassertiontest_ps_recognises_a_process_whose_command_line_says_everos; it also failed in isolation.uv run ruff check raven/cli/onboard_everos.py raven/providers/azure_openai_provider.py tests/test_cli_onboard_commands.py tests/test_azure_openai_provider.py: passed.uv run ruff format --check raven/cli/onboard_everos.py raven/providers/azure_openai_provider.py tests/test_cli_onboard_commands.py tests/test_azure_openai_provider.py: passed.git diff --check github/main...HEAD: passed.
| "llm": { | ||
| "label": ("Memory LLM", "记忆 LLM"), | ||
| "example": "gpt-4.1-mini", | ||
| "example": "gpt-5.6-luna", |
There was a problem hiding this comment.
Blocking: selecting this newly recommended model against the curated OpenAI endpoint produces a configuration that passes _probe_everos_chat() but fails when EverOS actually extracts memory. The probe omits temperature, while the installed EverOS OpenAIProvider unconditionally sends temperature=0.0; GPT-5.6 Luna defaults to medium reasoning, and the current OpenAI endpoint metadata does not list temperature as supported. Direct OpenAI users can therefore follow the recommendation, see successful verification, and then have every real memory-LLM call rejected. Please either make the EverOS client omit unsupported sampling fields (or explicitly use a compatible reasoning mode), or keep the recommendation on a model its payload supports. Sources: OpenAI model page, current endpoint metadata.
There was a problem hiding this comment.
Fixed in aa6717f: the memory-LLM floor now points at qwen/qwen3.8-flash, whose endpoint metadata supports temperature, so the EverOS client's unconditional temperature=0.0 is accepted on every wizard path. (gpt-5.6-luna could not be kept: EverOS 1.2.3, the latest release, always sends temperature and has no reasoning_effort knob, so direct-OpenAI configurations would have kept failing after a green probe.)
There was a problem hiding this comment.
Confirmed on aa6717f: qwen/qwen3.8-flash is served with temperature support, so EverOS's unconditional temperature=0.0 no longer creates a probe/runtime mismatch. This blocker is resolved.
| api_key: str = "", | ||
| api_base: str = "", | ||
| default_model: str = "gpt-5.2-chat", | ||
| default_model: str = "gpt-5.6-sol", |
There was a problem hiding this comment.
Blocking: this default is incompatible with this provider's normal agent payload. When tools are present and the caller leaves reasoning_effort unset, _prepare_request_payload() sends tools but no reasoning_effort; GPT-5.6 therefore uses its medium default. Microsoft explicitly documents that GPT-5.6 Chat Completions requests with tools fail in that case, even when reasoning_effort is omitted, and require either the Responses API or reasoning_effort="none". The production config factory supplies a model explicitly, but the constructor is public and direct callers relying on its default now receive a 400 as soon as Raven offers tools. Please retain a compatible default or adapt the payload for GPT-5.6. Microsoft's GPT-5.6 tool-calling guidance.
There was a problem hiding this comment.
Fixed in aa6717f: _prepare_request_payload now sets reasoning_effort="none" when tools are present and no effort was set for GPT-5.x deployments (temperature stays omitted), matching Microsoft's tool-calling guidance. Callers who set reasoning_effort explicitly keep it; non-GPT-5 deployments are untouched. Covered by four new tests in test_azure_openai_provider.py.
There was a problem hiding this comment.
This is only partially fixed. agents.defaults.reasoning_effort is a user-configurable setting, and make_provider() copies it into provider.generation; the normal agent call then passes that value into _prepare_request_payload(). With reasoning_effort = "medium", this revision still emits tools plus reasoning_effort: "medium", which Microsoft's cited guidance says returns 400 for GPT-5.6 Chat Completions. The new test_gpt5_keeps_the_callers_reasoning_effort currently pins that invalid payload. Please ensure GPT-5.x tool calls always use none (or reject/migrate incompatible explicit settings) rather than preserving a combination the endpoint cannot execute.
There was a problem hiding this comment.
Fixed fully in 0b33f17: for GPT-5.x deployments the tools path now always forces reasoning_effort="none", overriding any configured agents.defaults.reasoning_effort, because Chat Completions reject tools under every effort except "none". Configured effort still applies to non-tool calls (covered by test_gpt5_without_tools_keeps_configured_effort).
There was a problem hiding this comment.
Confirmed on 0b33f17: GPT-5.x requests with tools now force reasoning_effort="none" even when agents.defaults.reasoning_effort is configured, while no-tool requests retain the configured effort. The regression tests cover both branches, so the remaining Azure blocker is resolved.
gpt-5.6-luna cannot serve as the memory-LLM floor: the EverOS client always sends temperature, which GPT-5.x reasoning models reject, so the floor now points at qwen/qwen3.8-flash. On the Azure provider, GPT-5.x deployments now get reasoning_effort="none" when tools are offered and no effort was set, matching Microsoft tool-calling guidance instead of failing with a 400.
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the EverOS payload mismatch is resolved, but configured non-none reasoning still makes GPT-5.6 Azure tool calls fail.
I reviewed the 15f486f4a343...aa6717fc9767 delta and rechecked the complete github/main...HEAD diff, callers, relevant history, backward compatibility, AGENTS.md/CLAUDE.md and the runtime glossary, plus whether tests were weakened. The Qwen replacement matches EverOS's actual temperature=0.0 payload. The Azure default injection works only when the setting is absent; the user-configurable agents.defaults.reasoning_effort reaches this provider, and the new test deliberately preserves medium with tools even though that request is rejected by GPT-5.6 Chat Completions. I replied on both existing threads rather than duplicating the original finding.
Verification:
uv run pytest tests/test_cli_onboard_commands.py tests/test_azure_openai_provider.py tests/test_everos_server.py -q: 341 passed, 1 failed. The sole failure remains the pre-existing, unchanged real-psassertiontest_ps_recognises_a_process_whose_command_line_says_everos.uv run ruff check raven/cli/onboard_everos.py raven/providers/azure_openai_provider.py tests/test_azure_openai_provider.py: passed.uv run ruff format --check raven/cli/onboard_everos.py raven/providers/azure_openai_provider.py tests/test_azure_openai_provider.py: passed.git diff --check github/main...HEAD: passed.
A configured agents.defaults.reasoning_effort reaches this provider too, and Microsoft Chat Completions reject tools under any effort but "none", so the unset-only injection still 400s for users who configured reasoning. Tool calls now always force "none" on GPT-5.x deployments; the configured effort still applies to non-tool calls.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The final Azure fix resolves the remaining blocker: GPT-5.x tool requests now force reasoning_effort="none" regardless of the configured generation default, while no-tool requests preserve the caller's setting. I reviewed the aa6717fc9767...0b33f177e921 delta and rechecked the complete github/main...HEAD diff, callers, relevant history, backward compatibility, AGENTS.md/CLAUDE.md and the runtime glossary, and whether tests were weakened. The new tests cover both the forced tool path and preserved no-tool path; I found no new issue worth raising.
Verification:
uv run pytest tests/test_cli_onboard_commands.py tests/test_azure_openai_provider.py tests/test_everos_server.py -q: 342 passed, 1 failed. The sole failure remains the pre-existing, unchanged real-psassertiontest_ps_recognises_a_process_whose_command_line_says_everos.uv run pytest tests/test_azure_openai_provider.py -q: 14 passed.uv run ruff check raven/providers/azure_openai_provider.py tests/test_azure_openai_provider.py: passed.uv run ruff format --check raven/providers/azure_openai_provider.py tests/test_azure_openai_provider.py: passed.git diff --check github/main...HEAD: passed.
gloryfromca
left a comment
There was a problem hiding this comment.
Approving. Verified the three changes land as described:
- The inotify preflight gates the spawn before a child that cannot start. It counts same-uid instances via
/proc/<pid>/fdlinks readinganon_inode:inotify, which covers the zero-watch instances fdinfo cannot see; raises the cap when privileged, and otherwise fails fast with the exact sysctl commands. A dead child whose log blames inotify gets the same hint appended. - Onboard defaults now point at models the endpoints actually serve. The memory-LLM floor moved to
qwen/qwen3.8-flashso the EverOS client's unconditionaltemperatureis accepted on every wizard path. - Azure GPT-5.x tool calls force
reasoning_effort="none"over any configured effort, while non-tool calls keep the caller's setting. The test matrix pins all four combinations plus a non-GPT-5 deployment.
All prior blocking notes are marked resolved by the reviewer, and every required check is green.
Summary
Onboard model-default refreshes plus an EverOS startup fix:
Rerank default: Qwen/Qwen3-Reranker-4B has been retired from OpenRouter, so the onboard wizard's rerank example and EN/ZH recommendations now point to qwen/qwen3-reranker-8b, which OpenRouter still serves.
Stale defaults: the memory-LLM capability floor (gpt-4.1-mini), the multimodal recommendation (google/gemini-3-flash-preview), and the Azure OpenAI provider default (gpt-5.2-chat) all point at generations behind what OpenRouter/Azure serve today. They now read qwen/qwen3.8-flash (the EverOS client always sends temperature, which the GPT-5.x reasoning family rejects), google/gemini-3.7-flash, and gpt-5.6-sol. GPT-5.x Azure deployments force reasoning_effort="none" on tool calls, matching Microsoft's tool-calling requirement; configured effort still applies to non-tool calls.
Startup failure: an exhausted fs.inotify.max_user_instances makes the spawned EverOS server's watcher die at boot with "OSError: [Errno 24] inotify instance limit reached", which surfaces only as a cryptic exit buried in the dead-child log. ensure_everos_server now measures per-user inotify headroom (counting same-uid inotify instances via /proc//fd links that read anon_inode:inotify against the kernel cap) before spawning: it raises the cap when the process is privileged, otherwise it fails fast with the exact sudo sysctl commands. A dead child whose log blames inotify gets the same hint appended to its error.
Type
Verification
make coverage (full default suite, uv run --frozen --python 3.12 --all-extras pytest -q): 7119 passed, 2 failed, 37 skipped, 13 deselected. The 2 failures are pre-existing on main and environment-dependent (reproduced on main in a throwaway worktree): both expect a chmod-based write to be refused, which cannot happen when the suite runs as root (CAP_DAC_OVERRIDE):
make coverage-diff COVERAGE_BASE_REF=origin/main: 100.00% (57/57 executable changed lines), passed the 90.00% threshold
make coverage-ratchet: passed (line +2.11pp, branch +3.24pp over baseline)
uv run pytest tests/test_cli_onboard_commands.py: 251 passed
uv run pytest tests/test_azure_openai_provider.py: 9 passed
uv run ruff check (changed files): clean
uv run ruff format --check (changed files): clean
pre-commit commitlint hook (conventional commit message): passed on all commits
Relevant tests pass locally
Relevant lint / type checks pass locally
Risk
Security impact considered: the sysctl write runs only when the process already holds the privilege to write /proc/sys; otherwise the gate only returns the instructions.
Backward compatibility considered: the gate is a no-op when headroom exists or when procfs/sysctl cannot be read (non-Linux); the model-default changes are wizard text and one provider default constant, no config migration needed.
Rollback path is clear: revert the merge; the check is isolated to ensure_everos_server and three module-level helpers.
Security impact considered
Backward compatibility considered
Rollback path is clear for risky changes
Related Issues
N/A