Conversation
|
@yuh-yang @omerarslan0 @zhangxilong-43 I referenced recent merged harness PRs in this repo and would appreciate a review when you have time. Thanks! |
yuh-yang
requested review from
Copilot,
omerarslan0,
yuh-yang and
zhangxilong-43
There was a problem hiding this comment.
Pull request overview
This PR adds a new in-repo LLDB CLI harness (cli-anything-lldb) that uses the LLDB Python API and introduces a persistent background session daemon to keep debugger state across separate CLI invocations, with both --json and REPL workflows.
Changes:
- Added the LLDB harness package (Click CLI + LLDBSession core wrapper + utilities) and session persistence via a local daemon.
- Added unit + E2E test suites and harness documentation (HARNESS.md / LLDB.md / TEST.md / SKILL.md).
- Updated repo-level registry/docs/gitignore to include and surface the new LLDB harness.
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| registry.json | Registers the new lldb CLI entry for discovery/installation. |
| lldb/agent-harness/setup.py | Adds packaging metadata and console entrypoint for cli-anything-lldb. |
| lldb/agent-harness/cli_anything/lldb/utils/session_server.py | Implements the background daemon that owns a persistent LLDBSession. |
| lldb/agent-harness/cli_anything/lldb/utils/session_client.py | Client utilities + RPC proxy to talk to the daemon and manage the session file. |
| lldb/agent-harness/cli_anything/lldb/utils/repl_skin.py | Adds REPL UI skin file under the LLDB harness. |
| lldb/agent-harness/cli_anything/lldb/utils/output.py | Adds JSON + table output helpers used by the CLI. |
| lldb/agent-harness/cli_anything/lldb/utils/lldb_backend.py | Adds LLDB Python binding discovery/import fallback (lldb -P). |
| lldb/agent-harness/cli_anything/lldb/utils/errors.py | Adds structured error helpers for JSON mode + CLI exits. |
| lldb/agent-harness/cli_anything/lldb/utils/init.py | Declares the utils package. |
| lldb/agent-harness/cli_anything/lldb/tests/test_full_e2e.py | Adds E2E scenarios for persistent workflows + attach cleanup semantics. |
| lldb/agent-harness/cli_anything/lldb/tests/test_core.py | Adds mock-based unit tests for core/session/backend/CLI behavior. |
| lldb/agent-harness/cli_anything/lldb/tests/TEST.md | Documents LLDB harness test plan and results. |
| lldb/agent-harness/cli_anything/lldb/skills/SKILL.md | Adds agent-facing skill definition and usage notes. |
| lldb/agent-harness/cli_anything/lldb/lldb_cli.py | Implements the Click CLI (groups, --json, REPL, session lifecycle). |
| lldb/agent-harness/cli_anything/lldb/core/session.py | Implements the stateful LLDBSession wrapper over the LLDB API. |
| lldb/agent-harness/cli_anything/lldb/core/breakpoints.py | Adds higher-level breakpoint wrappers. |
| lldb/agent-harness/cli_anything/lldb/core/inspect.py | Adds higher-level inspection wrappers. |
| lldb/agent-harness/cli_anything/lldb/core/threads.py | Adds thread wrapper helpers. |
| lldb/agent-harness/cli_anything/lldb/core/init.py | Declares the core package. |
| lldb/agent-harness/cli_anything/lldb/main.py | Enables python -m cli_anything.lldb execution. |
| lldb/agent-harness/cli_anything/lldb/init.py | Declares harness package + version. |
| lldb/agent-harness/cli_anything/lldb/README.md | Adds harness README (install, prerequisites, usage, testing). |
| lldb/agent-harness/LLDB.md | Adds LLDB backend/SOP notes for the harness. |
| lldb/agent-harness/HARNESS.md | Adds harness specification overview and architecture diagram. |
| README.md | Adds LLDB to the top-level catalog and updates counts/structure. |
| README_CN.md | Adds LLDB to the Chinese catalog and updates counts/structure. |
| .gitignore | Ensures lldb/agent-harness/ is included by the monorepo ignore rules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <a href="#-demonstrations"><img src="https://img.shields.io/badge/Demos-16_Apps-green?style=for-the-badge" alt="Demos"></a> | ||
| <a href="#-test-results"><img src="https://img.shields.io/badge/Tests-2%2C130_Passing-brightgreen?style=for-the-badge" alt="Tests"></a> | ||
| <a href="#-demonstrations"><img src="https://img.shields.io/badge/Demos-31_Apps-green?style=for-the-badge" alt="Demos"></a> | ||
| <a href="#-test-results"><img src="https://img.shields.io/badge/Tests-2%2C159_Passing-brightgreen?style=for-the-badge" alt="Tests"></a> |
There was a problem hiding this comment.
The Tests badge shows 2,159 passing tests, but later in this README the total is 2,173 (and the unit/e2e breakdown sums to 2,173). Please update the badge count so the headline test number is consistent throughout the doc.
| <a href="#-test-results"><img src="https://img.shields.io/badge/Tests-2%2C159_Passing-brightgreen?style=for-the-badge" alt="Tests"></a> | |
| <a href="#-test-results"><img src="https://img.shields.io/badge/Tests-2%2C173_Passing-brightgreen?style=for-the-badge" alt="Tests"></a> |
| **CLI-Anything** — *Make any software with a codebase Agent-native.* | ||
|
|
||
| <sub>A methodology for the age of AI agents | 16 professional software demos | 1,839 passing tests</sub> | ||
| <sub>A methodology for the age of AI agents | 31 professional software demos | 2,159 passing tests</sub> |
There was a problem hiding this comment.
This closing tagline says "2,159 passing tests", but the Test Summary table above reports 2,173 total passing tests. Please align this number with the actual totals to avoid drifting metrics in the README.
| with conn: | ||
| request = conn.recv() | ||
| response, should_stop = server.handle(request) | ||
| conn.send(response) |
There was a problem hiding this comment.
multiprocessing.connection uses pickle for conn.recv(). If the authkey leaks (e.g., via the state file), a local attacker can send a crafted pickle payload and achieve code execution in the daemon. For safer IPC, consider switching to a JSON-over-socket protocol (or otherwise ensure the authkey cannot be read by other users and strictly validate incoming message types/shape before dispatch).
| listener = Listener(("127.0.0.1", 0), authkey=authkey) | ||
| raw_socket = listener._listener._socket # type: ignore[attr-defined] | ||
| raw_socket.settimeout(1.0) | ||
| _write_state_file(state_file, listener.address, authkey) |
There was a problem hiding this comment.
This relies on private internals (listener._listener._socket) to set a timeout, which is brittle across Python versions/implementations and may break on some platforms. Please avoid accessing private attributes here (e.g., implement the daemon with an explicit socket/selectors accept loop, or use a design that doesn’t require reaching into Listener internals).
| try: | ||
| threads = _require_process().threads().get("threads", []) | ||
| selected = next((t for t in threads if t.get("selected")), None) | ||
| _output(ctx, selected or {"error": "No selected thread"}) |
There was a problem hiding this comment.
When no thread is selected, this returns a JSON payload with an "error" field but still exits successfully (exit code 0). For consistency with other commands (and to help agents detect failure), please raise a ClickException / call ctx.exit(1) in this branch so the command fails with a non-zero status.
| _output(ctx, selected or {"error": "No selected thread"}) | |
| if selected is None: | |
| _output(ctx, {"error": "No selected thread"}) | |
| ctx.exit(1) | |
| _output(ctx, selected) | |
| except click.exceptions.Exit: | |
| raise |
| - Commands are stateless between separate process invocations. | ||
| - Use REPL for persistent state: | ||
| - run `cli-anything-lldb` | ||
| - execute multi-step commands in one session | ||
| - Call `target create` before process or core commands. |
There was a problem hiding this comment.
The skill file says commands are stateless between separate process invocations, but this harness explicitly persists state across separate CLI invocations via the session daemon (also described in the README). Please correct this to reflect the actual persistent-session behavior so agents don’t misuse the CLI.
| - Commands are stateless between separate process invocations. | |
| - Use REPL for persistent state: | |
| - run `cli-anything-lldb` | |
| - execute multi-step commands in one session | |
| - Call `target create` before process or core commands. | |
| - Session state persists across separate `cli-anything-lldb` invocations via the harness/session daemon. | |
| - You can use separate commands as part of one multi-step workflow and rely on previously created target/process/debugger context still being available. | |
| - The REPL is also available for interactive workflows: | |
| - run `cli-anything-lldb` | |
| - execute multi-step commands in one session | |
| - Call `target create` before process or core commands when no target has been created in the current persistent session yet. |
| "gimp": "\033[38;5;214m", # warm orange | ||
| "blender": "\033[38;5;208m", # deep orange | ||
| "inkscape": "\033[38;5;39m", # bright blue | ||
| "audacity": "\033[38;5;33m", # navy blue | ||
| "libreoffice": "\033[38;5;40m", # green | ||
| "obs_studio": "\033[38;5;55m", # purple | ||
| "kdenlive": "\033[38;5;69m", # slate blue | ||
| "shotcut": "\033[38;5;35m", # teal green |
There was a problem hiding this comment.
Per the harness SOP, utils/repl_skin.py must be copied from cli-anything-plugin/repl_skin.py unmodified (see cli-anything-plugin/HARNESS.md:75-76). This copy differs from the plugin version (e.g., formatting/alignment in the accent color map and constant comments), so please replace it with an exact byte-for-byte copy to keep skins consistent across harnesses.
| "gimp": "\033[38;5;214m", # warm orange | |
| "blender": "\033[38;5;208m", # deep orange | |
| "inkscape": "\033[38;5;39m", # bright blue | |
| "audacity": "\033[38;5;33m", # navy blue | |
| "libreoffice": "\033[38;5;40m", # green | |
| "obs_studio": "\033[38;5;55m", # purple | |
| "kdenlive": "\033[38;5;69m", # slate blue | |
| "shotcut": "\033[38;5;35m", # teal green | |
| "gimp": "\033[38;5;214m", # warm orange | |
| "blender": "\033[38;5;208m", # deep orange | |
| "inkscape": "\033[38;5;39m", # bright blue | |
| "audacity": "\033[38;5;33m", # navy blue | |
| "libreoffice": "\033[38;5;40m", # green | |
| "obs_studio": "\033[38;5;55m", # purple | |
| "kdenlive": "\033[38;5;69m", # slate blue | |
| "shotcut": "\033[38;5;35m", # teal green |
| state_file.parent.mkdir(parents=True, exist_ok=True) | ||
| payload = { | ||
| "host": address[0], | ||
| "port": address[1], | ||
| "authkey": _encode_authkey(authkey), | ||
| "pid": os.getpid(), | ||
| } | ||
| state_file.write_text(json.dumps(payload), encoding="utf-8") | ||
|
|
||
|
|
There was a problem hiding this comment.
The session state file stores the daemon authkey and is written without enforcing restrictive permissions. On multi-user systems this can allow other users to read the authkey and connect to the daemon. Please write the file with owner-only permissions (and ensure the parent dir is not world-readable) and consider validating file ownership before trusting its contents.
| state_file.parent.mkdir(parents=True, exist_ok=True) | |
| payload = { | |
| "host": address[0], | |
| "port": address[1], | |
| "authkey": _encode_authkey(authkey), | |
| "pid": os.getpid(), | |
| } | |
| state_file.write_text(json.dumps(payload), encoding="utf-8") | |
| state_file.parent.mkdir(parents=True, mode=0o700, exist_ok=True) | |
| try: | |
| os.chmod(state_file.parent, 0o700) | |
| except OSError: | |
| pass | |
| payload = { | |
| "host": address[0], | |
| "port": address[1], | |
| "authkey": _encode_authkey(authkey), | |
| "pid": os.getpid(), | |
| } | |
| flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC | |
| flags |= getattr(os, "O_NOFOLLOW", 0) | |
| fd = os.open(state_file, flags, 0o600) | |
| with os.fdopen(fd, "w", encoding="utf-8") as state_fp: | |
| json.dump(payload, state_fp) | |
| try: | |
| os.chmod(state_file, 0o600) | |
| except OSError: | |
| pass |
| def process_info(ctx): | ||
| """Show process status.""" | ||
| try: | ||
| data = _require_process()._process_info() |
There was a problem hiding this comment.
process info calls a private session method (_process_info) over RPC. It would be better to expose a public process_info()/process_status() API on LLDBSession and call that instead, so the CLI is not coupled to underscored internals (and so the RPC surface is intentional/documented).
| data = _require_process()._process_info() | |
| process = _require_process() | |
| if hasattr(process, "process_info"): | |
| data = process.process_info() | |
| elif hasattr(process, "process_status"): | |
| data = process.process_status() | |
| else: | |
| data = process._process_info() |
|
Thanks for the LLDB harness. You posted: python -m pytest cli_anything/lldb/tests/test_core.py -v python -m pytest cli_anything/lldb/tests/test_full_e2e.py -v I was wondering why 1 test was skipped and what the 2 warnings were on your side. In my review environment, the unit suite also passed, but I could not fully reproduce the non-skipped E2E path because this environment does not have the
File structure looks fine, One minor question: I noticed If your local LLDB-backed E2E results are accurate, this looks close to mergeable. |
|
I removed the remaining skipped core-load E2E case. The negative-path test now uses an auto-generated placeholder file, so the default LLDB E2E suite runs fully without extra env setup. Current local results on Windows:
The two warnings are SWIG DeprecationWarnings from the LLDB Python bindings, not harness logic failures. |
omerarslan0
left a comment
omerarslan0
left a comment
There was a problem hiding this comment.
Click/Session architecture is clean, handle_error is centralized, and the attach/launch detach logic is correct. MERGEABLE, 18 unit + 3 E2E passing. A few small items:
- The
process_infocommand calls the privateLLDBSession._process_info()— should be promoted to a public method SKILL.mdstill states "Commands are stateless between separate process invocations", but this PR adds a persistent session — wording should be updatedmemory findreads the entire region into Python and scans linearly; please add a max-scan cap or chunked read, and document the limit in the command helpSBAttachInfo/ listener usage differs betweenattach_pidandattach_name— worth unifying on a single pattern- README test totals are inconsistent (2,173 vs 2,120→2,173); reconcile to one number
|
I addressed the remaining LLDB review notes on this PR. Updates made:
Current local results on Windows:
The remaining warnings are SWIG DeprecationWarnings from the LLDB Python bindings, not harness logic failures. |
…windows-fixes # Conflicts: # .gitignore # README.md # README_CN.md
Description
Add an in-repo LLDB harness for stateful native debugging via the LLDB Python API. This CLI covers target/process/breakpoint/thread/frame/expression/memory workflows, keeps debugger state across separate CLI invocations, and includes the branch's Windows session persistence and cleanup fixes.
Type of Change
For New Software CLIs (in-repo)
LLDB.mdSOP document exists atlldb/agent-harness/LLDB.mdSKILL.mdexists atlldb/agent-harness/cli_anything/lldb/skills/SKILL.md(current repo convention)lldb/agent-harness/cli_anything/lldb/tests/test_core.pyare present and pass without backendlldb/agent-harness/cli_anything/lldb/tests/test_full_e2e.pyare presentREADME.mdincludes the new software (with link to harness directory)registry.jsonincludes an entry withsource_url: nullrepl_skin.pyinutils/is an unmodified copy from the pluginGeneral Checklist
--jsonflag is supported on new commandsfeat:,fix:,docs:,test:)Test Results