Add one-command Codex and pi integration with MCP search and startup hooks - #160

Adds a one-command, checkout-local tgrep integration for Codex and pi using MCP search tools, startup hooks, and shared services.

Changes:

• Adds transactional project/user installation, repair, doctor, and uninstall workflows.
• Adds MCP runtime, pi bridge, freshness handling, cancellation, and path validation.
• Adds documentation, integration tests, and Linux/macOS CI coverage.

Suppressed comments (7)

scripts/agent/install.py:215

• When both an inline TOML hooks table and a hooks.json file exist, this condition always chooses the JSON representation, even though the documented behavior is to use the inline TOML hooks whenever they are present. The newly added hook can therefore be written to a file Codex does not load while the inline configuration remains unchanged. Prefer the inline branch whenever "hooks" in parsed and only fall back to hooks.json when it is absent.

        if hooks_raw is not None or "hooks" not in parsed:
            hooks = json.loads(hooks_raw or b"{}")

scripts/agent/install.py:235

• These replacements are applied sequentially to the template and also scan values inserted by earlier replacements. A valid checkout or configuration path containing a token such as __TOOLS__ or __CONFIG__ will therefore be rewritten by a later iteration, producing invalid TypeScript or incorrect absolute paths. Use a single-pass placeholder substitution or collision-proof markers.

        for key, value in substitutions.items():
            template = template.replace(key, json.dumps(value))

scripts/agent/install.py:184

• For a supported project root outside Git, tgrep intentionally ignores .gitignore unless --no-require-git is supplied (see AGENTS.md:209-214). The default flags here remain empty even though the installer adds /.tgrep-agent/ to .gitignore, so the service indexes its own managed runtime/configuration and does not honor that exclusion. Detect non-Git project roots and include the matching index flag, or exclude the managed directory another way.

    flags = []
    if args.no_require_git:
        flags.append("--no-require-git")

scripts/agent/pi-extension.ts:43

• If the MCP child exits after connect() resolves but before this request is sent, child is undefined and the optional write becomes a no-op. The returned promise then waits the full 40-second timer instead of reporting the disconnect or reconnecting, so a crashed adapter can make every next tool call hang. Check that the child/stdin is still alive before writing and reject or reconnect when it is not.

      child?.stdin?.write(JSON.stringify({ jsonrpc: "2.0", id, method, params }) + "\n");

scripts/agent/runtime.py:238

• output_mode="count" takes the -c --with-filename branch, but this fallback stores the whole CLI line as text. Unlike the content/files modes, callers therefore receive an absolute path:count string (the command searches an absolute root), not a normalized path and count field. Parse the final count and normalize the filename before adding the result so the advertised count mode is structured and does not expose the checkout path.

            else:
                item = {"text": text}

scripts/agent/runtime.py:121

• A valid JSON scalar or array can be returned by a stale/unrelated process at the port in serve.json; reply.get then raises AttributeError, which is not caught here. Indexed requests can consequently crash instead of treating the service as unavailable and falling back to a scan. Validate the response shape or include AttributeError in the handled failures.

                    reply = json.loads(stream.readline(65536))
            return reply.get("result")
        except (OSError, ValueError, KeyError, TypeError):

scripts/agent/runtime.py:280

• The poll() check and killpg() are racy: the query can exit after poll() returns None, causing os.killpg to raise ProcessLookupError and mask the actual search result during normal cleanup/cancellation. Treat an already-gone process group as successfully terminated before waiting and closing the pipes.

            if proc.poll() is None:
                os.killpg(proc.pid, signal.SIGKILL)
            proc.wait()

• Files reviewed: 9/9 changed files
• Comments generated: 2
• Review effort level: Lite (auto)

Suppressed comments (6)

Previously missed (1) — in code that hasn't changed since the last review.

scripts/agent/pi-extension.ts:97

• Tool cancellation is ignored during connection setup: execute waits for connect() before passing the signal to the actual call, while connect() initializes the MCP child with request() without that signal. If startup hangs (or the signal is already aborted), the tool can wait the full 40-second initialization timeout and leave an unnecessary adapter process instead of rejecting promptly. Thread the abort signal through connection/initialization and check it before spawning.

scripts/agent/install.py:312

• This health check launches the installed runtime with the interpreter running doctor, but both Codex's MCP command and the generated pi extension embed sys.executable from installation time. If that Python path is later removed or changed (a documented repair case), doctor can report OK while the agent cannot start the integration. Record and probe the configured interpreter, or otherwise validate the embedded command before reporting success.

            process = subprocess.Popen([sys.executable, runtime_record["path"], "mcp", "--config", config_record["path"]],
                                       cwd=root, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)

scripts/agent/install.py:396

• These project paths are preflighted even for --scope user, although a user-scoped installation never owns or writes them. Consequently, an unrelated symlinked AGENTS.md or .gitignore in the current repository makes user-scope install, repair, or uninstall fail before touching the user destination. Restrict these two checks to project scope.

    for path in (manifest_path, base / "install.lock", base / "backups", root / "AGENTS.md", root / ".gitignore"):
        reject_symlinks(path)

scripts/agent/install.py:216

• This always appends a [mcp_servers.tgrep] table. A valid existing TOML configuration such as mcp_servers = { other = { command = "other" } } cannot be followed by a table header because the inline table is already closed, so the validation immediately after this block fails and installation cannot preserve that configuration. Merge inline-table forms (or explicitly report/support the representation) before appending the managed block.

        block = "\n".join([
            "[mcp_servers.tgrep]", f"command = {json.dumps(sys.executable)}",
            "args = " + json.dumps([str(runtime), "mcp", "--config", str(configuration)]),
            "startup_timeout_sec = 10", "tool_timeout_sec = 40",

scripts/agent/runtime.py:128

• The discovery file contains a PID and port, but this probe accepts any local process that returns a plausible status object and never verifies the recorded server identity or repository. After a crash, another tgrep server can reuse the saved port; ensure_server will then skip startup and indexed queries can be answered by the wrong repository. Validate the server/process identity before treating this status as live.

            info = json.loads((self.index / "serve.json").read_text())
            with socket.create_connection(("127.0.0.1", info["port"]), timeout=0.3) as sock:
                sock.sendall(b'{"jsonrpc":"2.0","id":1,"method":"status"}\n')
                with sock.makefile("rb") as stream:
                    reply = json.loads(stream.readline(65536))
            if not isinstance(reply, dict) or reply.get("jsonrpc") != "2.0" or reply.get("id") != 1:

scripts/agent/runtime.py:97

• The scalar-string limit is not applied to array members. An MCP caller can send up to 100 arbitrarily large file_types or glob strings, causing oversized subprocess argument lists or excessive memory use before tgrep runs. Apply the same per-item length bound (and ideally a total argument budget) when validating lists.

        if isinstance(value, list) and (len(value) > 100 or any(type(v) is not str or "\0" in v for v in value)):
            raise ValueError(f"Invalid list for {key}")

• Files reviewed: 9/9 changed files
• Comments generated: 3
• Review effort level: Lite (auto)

Suppressed comments (4)

scripts/agent/install.py:138

• When both agents are installed into a previously absent shared AGENTS.md or .gitignore, the second block sees the first managed block via self.get(path) and records created=False. Removing both blocks then leaves an empty file instead of deleting the file created by the integration. Base the flag on the original value in self.before[path], which is shared across all blocks for that path.

        records.append({"kind": "block", "path": str(path), "text": value, "created": original is None})

scripts/agent/install.py:336

• doctor only verifies that each managed block/file can be removed; it never parses the surrounding Codex config.toml. A user edit outside the owned block can leave that file syntactically invalid, while the direct MCP probe below still succeeds and doctor reports OK even though Codex cannot load the integration. Parse the host configuration (and the selected hook representation) before reporting success.

            for record in entry["records"]:
                if not Path(record["path"]).exists():
                    raise ValueError(f"Missing managed configuration: {record['path']}")
                check.remove_record(record)

scripts/agent/install.py:458

• The previous-binary reuse path bypasses the symlink check: a retained private base/bin/tgrep can be replaced with a symlink, is_file() follows it, and binary_path() then resolves and executes the external target for --version/--help and future hooks. Validate the manifest's private binary destination with the same owned-path check before assigning it (while keeping explicitly user-supplied binaries separate).

            if not args.binary and len(previous_binaries) == 1:
                previous_binary = previous_binaries.pop()
                if Path(previous_binary).is_file():
                    args.binary = previous_binary

scripts/agent/runtime.py:155

• A malformed but readable serve.json with a large integer PID or a port outside the socket range passes the current type checks; os.kill()/socket.create_connection() can then raise OverflowError, which escapes status() instead of treating the record as unavailable and falling back to a scan. Catch OverflowError here (or validate the ranges before using these values).

        except (OSError, ValueError, KeyError, TypeError):

• Files reviewed: 9/9 changed files
• Comments generated: 4
• Review effort level: Lite (auto)

Suppressed comments (7)

scripts/agent/install.py:468

• When a reinstall/repair supplies any one index option, this condition skips preservation of the entire previous option set. For example, changing only --max-filesize silently drops an existing --no-require-git setting (and vice versa), so unrelated indexing behavior changes unexpectedly. Preserve each old flag unless that specific option was overridden.

                if old and not (args.max_filesize or args.no_max_filesize or args.no_require_git):

scripts/agent/install.py:174

• Path.absolute() does not normalize ... If a user sets CODEX_HOME or PI_CODING_AGENT_DIR to a path containing .., installation records that spelling, but prepare_records() later rejects any .. component (line 312), so the installation cannot be repaired, doctored, or uninstalled. Normalize these paths lexically without resolving symlinks before recording them.

        return Path(os.environ.get("CODEX_HOME", str(Path.home() / ".codex"))).expanduser().absolute()
    return Path(os.environ.get("PI_CODING_AGENT_DIR", str(Path.home() / ".pi/agent"))).expanduser().absolute()

scripts/agent/install.py:459

• When reinstalling/repairing both agents, this only reuses a previous binary if all selected agents share one path. If Codex and pi were installed with different valid --binary values, a later combined operation silently replaces both with the PATH/private binary even though the documented behavior says reinstall preserves each previous binary unless a replacement is supplied. Resolve the binary per agent (or reject the ambiguous combined operation) instead of collapsing the set.

            previous_binaries = {manifest["agents"][a]["config"]["binary"] for a in agents if a in manifest["agents"]}
            if not args.binary and len(previous_binaries) == 1:
                previous_binary = previous_binaries.pop()
                if Path(previous_binary).is_file():
                    args.binary = previous_binary
            binary = binary_path(args, base)

scripts/agent/runtime.py:361

• The adapter accepts up to 16 active calls but runs only four workers. Calls five through 16 can remain queued behind long-running searches; cancellation only sets their event, which is not observed until a worker starts the task, so a cancelled request can wait for the earlier 30-second timeouts instead of rejecting promptly. Track the submitted futures or otherwise remove/cancel queued work when notifications/cancelled arrives.

    with concurrent.futures.ThreadPoolExecutor(max_workers=4) as pool:

scripts/agent/runtime.py:173

• The symlink check only covers the state/index directories, but this open follows a pre-existing state/serve.log symlink. That lets startup diagnostics be written outside the managed cache despite the service-state path checks (and the same audit is needed for other state files opened by the launcher). Reject leaf symlinks before opening these paths.

            with (self.state / "serve.log").open("ab") as log:

scripts/agent/runtime.py:208

• find_files invokes tgrep --files without passing the requested pattern, then filters only after every indexed/filesystem path has been emitted. A query such as *.rs or a no-match pattern therefore scans and transfers the entire repository before returning, which can dominate the 30-second budget on large trees; apply a bounded/index-side filename filter or add a dedicated filename query instead.

        if name == "find_files":
            cmd += ["--files", "--null", str(path)]
        else:

scripts/agent/runtime.py:225

• Count mode still requests JSON match records for every matching line and record() discards those records, keeping only the later end statistics. A heavily matching file can therefore produce and parse an unbounded stream despite the response byte/record caps, causing unnecessary work or a timeout; the CLI needs a count-only JSON/summary path that emits per-file stats without match events.

                # CLI count output cannot escape arbitrary filenames. JSON end
                # records provide matched-line counts with unambiguous paths.
                cmd += ["--json", "-C", "0"]

• Files reviewed: 9/9 changed files
• Comments generated: 2
• Review effort level: Lite (auto)

Suppressed comments (4)

scripts/agent/install.py:421

• Calling data_home.resolve() removes any symlink components before reject_symlinks(base) runs. With a symlinked XDG_DATA_HOME, user-scope installation therefore writes through the symlink target instead of rejecting it, contrary to the advertised protection for owned installation paths and the equivalent checks used for project state and agent destinations. Keep the absolute, non-resolved path for the preflight check.

    base = (root / ".tgrep-agent" if args.scope == "project" else data_home.resolve() / "tgrep-agent")

scripts/agent/runtime.py:260

• When the requested path is a subdirectory, tgrep emits paths relative to that search directory (for example main.rs for a query rooted at repo/src), but this converts them directly relative to self.root and returns main.rs instead of src/main.rs. The same missing prefix affects JSON content/count records below and makes repository-relative results and repository-relative find_files globs incorrect for scoped searches; normalize the CLI path relative to the requested directory before applying the repository-root conversion in every output branch.

                resolved = Path(text)
                if not resolved.is_absolute():
                    resolved = self.root / resolved
                if not resolved.resolve().is_relative_to(self.root):
                    return
                text = str(resolved.relative_to(self.root))
                pattern = args.get("pattern", "*")
                if name == "find_files" and not (fnmatch.fnmatchcase(text, pattern) or fnmatch.fnmatchcase(Path(text).name, pattern)):

scripts/agent/runtime.py:155

• An out-of-range integer in a stale or corrupted serve.json (for example, a port above 65535) passes these type checks and can make socket.create_connection raise OverflowError. That exception is not caught below, so status() aborts ensure_server instead of treating the server as unavailable and falling back to a fresh scan; catch OverflowError or validate port/PID bounds with the malformed-record checks.

        except (OSError, ValueError, KeyError, TypeError):

scripts/agent/runtime.py:131

• These checks only reject symlinked directories, not symlinked files within them. A pre-created state/start.lock or state/serve.log is followed by open(), and the child similarly follows index/serve.lock/serve.json, so a local redirect can make the integration lock or overwrite files outside the cache despite the documented symlink protection. Validate every service-owned leaf before launch (or use no-follow/atomic creation) as well.

        reject_symlinks(self.state)
        reject_symlinks(self.index)

• Files reviewed: 9/9 changed files
• Comments generated: 0 new
• Review effort level: Lite (auto)