fix(state): keep unrelated processes from blocking FTS recovery

[fangliquanflq]

What does this PR do?

Unrelated root-owned wrappers and log readers can no longer prevent a stale FTS index from recovering merely because an argument mentions Hermes. The fallback now preserves /proc/<pid>/cmdline argument boundaries and recognizes Hermes only from the executable or an exact Python module/script argument, while the open-file holder check remains unchanged.

Symptom

When another process has an unreadable /proc/<pid>/fd directory and its command arguments contain text such as hermes-agent, Hermes treats it as a possible state.db holder. Runtime and startup FTS repair are then deferred even though that process has no descriptor open on the database.

Impact

A corrupted FTS index can remain stale while unrelated SSH wrappers, journalctl, grep, or terminal commands are running. Session search silently stays on the slower LIKE fallback instead of restoring FTS search.

Bug Cause

Trigger: hermes_state.py:4783 / _foreign_state_db_holders() / unreadable foreign fd table

Causal chain:

1. The Linux fallback reads NUL-delimited process arguments and previously flattens them into one string.
2. _looks_like_hermes() searches that entire string for broad Hermes substrings, including ordinary data arguments.
3. The unrelated process is reported as a holder, so both stale-index recovery paths defer FTS repair.

Why it is wrong: Mentioning Hermes in an argument does not identify the process that could own a SessionDB connection.

Working sibling / contrast: Processes with readable fd tables are matched against the exact state.db, WAL, and SHM targets and do not use the command-line heuristic.

Ruled out: The descriptor-based holder guard is not the source of the false positive; it only reports watched database paths and remains unchanged.

Fix

Preserve argv boundaries when reading /proc/<pid>/cmdline. Treat a process as Hermes only when its executable basename is a Hermes binary, or when a Python/PyPy interpreter has an exact hermes_cli.main module or hermes_cli/main.py script argument. Regression cases cover unrelated wrappers and real Hermes launch forms.

Related Issue

Fixes #92401

Type of Change

• Bug fix (non-breaking change that fixes an issue)

Changes Made

• hermes_state.py - parse /proc command lines as argv and identify Hermes from executable positions instead of substrings.
• tests/state/test_fts_runtime_rebuild.py - cover false-positive wrappers and supported Hermes binary, module, and script launches.

How to Test

1. Run the argv regression selection:

scripts/run_tests.sh tests/state/test_fts_runtime_rebuild.py -k 'uninspectable_non_hermes_process_is_not_a_holder or uninspectable_hermes_process_remains_a_holder' -q

2. Confirm the selection reports 9 passed.
3. On Linux, run scripts/run_tests.sh tests/state/test_fts_runtime_rebuild.py -q to exercise the /proc holder path with POSIX path semantics.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run the repository test entry on the relevant regression selection and all selected tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested the argv classifier on Windows 11; Linux /proc verification is included in the review handoff

Documentation & Housekeeping

• Relevant documentation is N/A; the private helper docstrings describe the behavior
• cli-config.yaml.example is N/A; no config keys changed
• CONTRIBUTING.md and AGENTS.md are N/A; no architecture or workflow changed
• I've considered cross-platform impact; the modified holder path is Linux-only and other platform paths are unchanged
• Tool descriptions and schemas are N/A; no tool behavior changed

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

Reviewed hermes_state.py (+145/-22) and tests/state/test_fts_runtime_rebuild.py.

What's good

• The old substring matcher flagged anything whose arguments mentioned "hermes-agent" — journalctl reading unit files, grep over syslog, even tailscaled … --cmd="python -m hermes_cli.main gateway" as an argument string. Each false positive deferred FTS recovery indefinitely. The new rule — Hermes only from the executable name, or from an exact -m hermes_cli.main / */hermes_cli/main.py target parsed through Python's actual option grammar — is the right precision level for a decision that gates repair.
• _python_execution_target() handles the genuinely tricky bits: operand-taking short options bundled into one token (-Im), attached values (-mhermes_cli.main), operand-taking long options, -- terminator, and refusing -c (target unknowable) rather than guessing. That's more correct than most "parse python argv" attempts.
• Test matrix is adversarial in both directions — 11 non-holder cases including the embedded-command wrapper shapes, 10 holder cases including python.exe -- — plus the integration fallback test updated to argv format. Exactly what a heuristic change needs.

Suggestions

1. Trade direction worth stating: the change converts false positives (recovery starved) into potential false negatives — a Hermes instance launched as python -c "from hermes_cli.main import main; main()", via runpy, or under an unrecognized wrapper is now invisible to the uninspectable-holder fallback, so repair may proceed while it holds the DB. If that worst case is just "SQLite busy → repair fails → retried later", say so in a comment; if a live holder during repair is dangerous, the -c refusal should perhaps fail toward caution instead of None→not-a-holder.
2. Nit: _looks_like_python_executable accepts odd-but-harmless names like python... (all-dot suffix); harmless, just noting the shape check is loose by design.
3. Nit: the display string still joins argv with spaces (" ".join(argv)), which can re-flatten quoted args in the log message — cosmetic only since matching no longer depends on it.

Strong, well-tested precision fix; #1 is a documentation/comment ask, not a blocker.

[fangliquanflq]

Thanks for the detailed cross-check. I confirmed both points against the current tip:

• The mount-namespace descriptor collision is valid and complementary, but it is not caused by this PR’s argv fallback change. The “ruled out” statement is scoped to _foreign_state_db_holders: substring match on /proc cmdline permanently blocks FTS repair #92401’s reproduced wrapper/cmdline false positive; fix(state): identify state.db holders by dev+inode, not path string #96011 is the appropriately scoped fix for cross-namespace descriptor identity.
• The root-runner test concern is already addressed here. test_foreign_holder_uninspectable_process_cmdline_fallback explicitly makes the mocked os.listdir("/proc/222/fd") raise PermissionError before rewriting paths into the fake proc tree, so execution reaches the cmdline fallback independently of chmod or effective UID.

No code change is needed for these points. The current required Python test job and aggregate required-check gate are passing.

[lEWFkRAD]

Reviewed on Windows: merged the PR head (5ec422e) onto the current main tip in a clean worktree and ran the state suite.

• tests/state/: 102 passed, 6 skipped, 2 failed. Both failures are Windows-only environment artifacts, not PR defects:
  • test_foreign_holder_detection_proc_readlink_deleted_wal — Windows os.readlink returns \\?\ extended-length paths, so the canonical-path match can never fire and the assertion fails vacuously. Reproduced on plain main. Proposed as #96475 (draft).
  • test_foreign_holder_uninspectable_process_cmdline_fallback — the test fakes a Linux /proc tree (including chmod 000 to simulate an unreadable fd dir), but production _foreign_state_db_holders bails to []/unknown-holder before any of that when _IS_WINDOWS, and on Windows git-bash chmod 000 doesn't make the dir unreadable in the first place. The assert holders[0][0] == 222 gets -1 ("scan unavailable"). Pre-existing on main; the PR didn't introduce it but also doesn't fix it — worth a skipif follow-up from someone with Linux CI in the loop.
• On Linux/CI where these paths actually execute, the suite is clean and the premise holds: the holder-detection change is sound, the narrow-identity and clustered-option commits read correctly against main.

Verdict: approve on the merits, conditional on CI green on Linux runners. Not submitting a formal APPROVE review from this account so the maintainer call stays yours.

[benperry6]

Integration update: #97330 now carries the argv-aware contribution in its single current-main release commit 30b5baffdd9f15332bc4a2b520aa9dbdea6c0739.

The original contribution is preserved through the exact trailer Co-authored-by: fangliquanflq <fangliquan@qq.com> and the PR body records the complete #90871 / #91839 / #93191 / #93453 / #92419 lineage. This replaces the earlier separate-commit receipt because the latest review explicitly requested a smaller coherent train.

The rematerialized exact head passes the focused 55-test runner and the 186-test broad repair/WAL/FTS suite; Ruff, byte compilation, Windows footgun scan, and git diff --check also pass. Hosted workflows for the sole surviving commit are awaiting maintainer approval.

[alt-glitch]

This was generated by AI during triage.

Superseded by #97330, which preserves and integrates this argv-aware holder-detection contribution alongside the complementary inode-based repair guard.