pr-review: harden 'Ask an agent' per-line Q&A#253
Draft
gnguralnick wants to merge 149 commits into
Draft
Conversation
Reimplements the DI/test-seam refactor against the FastAPI->Flask migration that landed on main (PR #175 merged it in). The prior approach used FastAPI Depends/dependency_overrides, neither of which exists under flask-sock; this version achieves the same goal with the Flask state container. - create_application drops the agent_manager / claude_auth_service / welcome_resender test-only params. It unconditionally builds and starts the live AgentManager (conftest's AgentManager.start no-op keeps that cheap and observe-free under test). Tests that need a seeded manager build the app, then set state_of(app).agent_manager; auth tests set state_of(app).claude_auth_service / .welcome_resender. - SystemInterfaceState.broadcaster is now derived from agent_manager (a property) instead of a stored field, so a single manager seed repoints the broadcaster too and the two can never diverge. The is_agent_manager_owned flag is gone; the app always owns the manager it builds, so shutdown() always stops it. - Agent messaging: the discover/send collaborators move off the private _send_message_to_agent's defaulted params onto a MngrMessenger FrozenModel that AgentManager owns. The send_message free function and _send_message_to_agent are gone; tests construct MngrMessenger(discover=, send=) directly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a [create_templates.modal] block so minds can launch a workspace on a Modal sandbox (the "modal" compute provider), mirroring the lima template (no Dockerfile build; toolchain provisioned over SSH; provider flipped on per-create; no autostart since sandboxes are ephemeral). Re-grounded off fresh main, replacing the earlier orphan/squash branch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Delete the libs/telegram_bot package and the send-telegram-message / read-telegram-history skills; drop the TELEGRAM_* pass-env vars, the telegram-bot dependency / workspace member / source, and the Dockerfile copy. send-user-message now uses its inline fallback as the sole channel (probe-and-dispatch contract unchanged). Scrub telegram from assorted docs/docstrings (README, CLAUDE.md, settings, runtime_backup, cloudflare_tunnel, system_interface, dealing-with-the-unexpected, build-web-service reserved names). Regenerated uv.lock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Teaches the workspace agent how to reach the Minds API through the latchkey minds-api-proxy (latchkey curl http://latchkey-self.invalid/minds-api-proxy/...), to discover routes/types from GET /api/schema, and to use the core capabilities: list/inspect workspaces, create a fresh workspace (with operation polling), SSH into another workspace (keypair + POST .../ssh, incl. the local reverse-tunnel case), read/export backups, and recover/lifecycle/destroy. The headline workflow strings these together to migrate content out of an old or broken workspace into a clean new one, then hands cleanup to the new workspace's own agent via `mngr message`. Includes the latchkey permission flow for a specific workspace: the `minds-workspaces` scope gates each verb (only the schema is allowed by default), and a per-workspace grant is requested via a `type: "workspace"` permission request carrying the verb list + `target_workspace_id`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…retry form A submitted OAuth code consumes the single-use sign-in session end-to-end (the backend terminates its 'claude auth login' subprocess unconditionally, and the modal clears its local session id). The failure path returned the user to the same code-entry form, but the session was already gone -- so the 'Verify & finish' button silently no-opped (no request, no error, no feedback). Route OAuth code failures to the full error screen instead, and make that screen's only action 'Start over', which restarts sign-in from the beginning (spawning a fresh OAuth session). The header close button / backdrop still dismiss the modal. API-key submit failures are unchanged (retryable in place). Adds a regression test and a changelog entry.
…unsc
When the docker daemon's runsc runtime is registered with --overlay2=none (now
required so a workspace's rootfs -- and mngr's SSH provisioning written to it --
survives a container restart), the container's /run lives on gVisor's
gofer-backed filesystem, which returns EOPNOTSUPP for os.link() of a socket
inode. supervisord installs its control socket via a hard link, so it wedges
("Unlinking stale socket") and never starts system_interface, leaving the
workspace stuck on the "Loading workspace" loader.
Add default_start_args=["--tmpfs","/run"] to [providers.docker] so /run is a
tmpfs where the hard link works -- mirroring the tmpfs /run the aws-<region>
provider start args and the ovh/vultr/imbue_cloud host setup already apply.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Changes for new Minds V1 API
Content-addressed guard (scripts/_provision_guard.sh): setup_system.sh skips when the workspace tree was already provisioned (marker under /var/lib/minds/provision keyed by git tree hash), so a Lima create booting a pre-baked image for the same tree skips ~25-30s of redundant system setup. Never skips without a matching marker. install_dependencies/build_workspace are deliberately NOT guarded -- they emit in-repo outputs the create re-materializes from git, so they must run every create.
Satisfies test_meta_ratchets::test_prevent_bash_without_strict_mode. The file is sourced by setup_system.sh (already strict), so this is a no-op for callers and keeps the lib safe to source anywhere; guard logic re-validated under set -e.
lima: skip setup_system provisioning when already baked (issue 2306)
Dockview is configured with defaultRenderer: "always", so an inactive chat tab stays mounted while an ancestor is hidden with display:none. mithril's m.redraw() is global, so the hidden ChatPanel keeps running its scroll logic against a scroll element that reports scrollTop/scrollHeight/clientHeight all as 0. maybePage() then mapped that zero position to event 0 and fired a JUMP that replaced the loaded window with the start of the conversation, and applyScrollPosition() clobbered the retained scrollTop to 0. Guard the scroll-management hooks (maybePage, applyScrollPosition, the viewport measure, and the ResizeObserver) so they skip while the element is not measurable, and force a redraw when the tab becomes visible again so the position is restored. The measurability predicate is extracted into a pure, unit-tested helper (scrollVisibility.ts) following the scrollFollow.ts / virtualWindow.ts pattern, plus a Playwright regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 2659502) (cherry picked from commit 338b4102c602d124e95797c9005999cea67c5457)
…g scenario (#193) * Harden update-system-interface: ground UI fixes in the real motivating scenario; close preview tab on teardown A UI fix delegated through this skill validated against a fixture the worker invented rather than the real conversation that motivated the change, producing a CSS selector that never matched the real DOM plus a passing-but-meaningless test. Close the process gaps at the three points where the real scenario should have entered, and fix a teardown gap: - Delegate: the lead must capture the real motivating DOM/measurements and pass them in the brief under a new `## Real scenario to reproduce` section, as the worker's reproduction fixture and acceptance target. - Preview: the preview opens on the worker's own empty agent, not the motivating conversation; the lead must open the real agent, re-measure, and confirm the number actually moved before asking the user. - Worker sub-skill: the fixture must reproduce the lead-provided DOM shape (not a guess), assert that shape, and the regression test must fail-before/pass-after. - Teardown: `unpreview` tears down the preview servers/services but leaves the `si-preview` dockview tab open; add an explicit step to close it (layout panel vs. service lifecycle are separate concerns). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Revise real-scenario hardening: point the worker at the live conversation instead of transcribing the DOM The worker is not cut off from the motivating conversation -- the system interface discovers agents from the shared MNGR_HOST_DIR, so an instance the worker boots renders the same real conversations the user sees. So the lead names the motivating agent and the worker looks at it firsthand, rather than the lead measuring the DOM and the worker reconstructing it from prose. --------- Co-authored-by: runtime-backup <runtime-backup@mindsbackup.local> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Gabriel Guralnick <gabriel@imbue.com>
The Lima provisioning-skip-guard change made setup_system.sh source its sibling scripts/_provision_guard.sh via "$(dirname "$0")", but the Dockerfile copies only setup_system.sh into /usr/local/bin (renamed to fct-setup-system). At build time dirname "$0" resolves to /usr/local/bin, so the source target /usr/local/bin/_provision_guard.sh does not exist and the fct-setup-system RUN fails with 'No such file or directory'. Copy _provision_guard.sh alongside setup_system.sh so the source resolves. The Lima path was unaffected because it runs the script in place from scripts/, where the guard already sits beside it.
…ngr #2310) Set 2 CPU / 4 GB and the 24h sandbox + idle timeouts on the [create_templates.modal] block via setting__extend, so the create template owns these values rather than the mngr_modal provider defaults (the provider side reverts them to its minimal defaults in imbue-ai/mngr#2310). Reorganized the block to mirror the vultr/aws/imbue_cloud cloud-template layout. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Tracks 4d3c69a342064e1ad4f5dfead6f6a77b7262d28f in mngr.
vendor/mngr: refresh from mngr main (4d3c69a34)
…ted state
Push dependency injection to a single composition root and remove the last
test monkeypatches, so production code reads as if no tests existed and every
collaborator is injected (not constructed-then-patched) under test.
- main is the composition root: `build_production_state` wires the real object
graph; `main` starts the agent manager (the sole `AgentManager.start` caller)
and serves. `build_application` only constructs, so it never spawns observe.
- create_application(state: SystemInterfaceState) is now a pure assembler: it
wires routes/plugins/error-handling onto the app and attaches the injected
state. It constructs nothing and starts nothing. The config / http-client /
filter parameters move up into the composition root.
- AgentManager.build gains an injectable `messenger` (defaults to the real
MngrMessenger), so the message-send path is fakeable without patching.
- testing.build_test_state is the test-side composition root: it builds a
SystemInterfaceState with fakes for whichever collaborators a test overrides
and cheap real instances for the rest, never starting the manager.
testing.RecordingMngrMessenger records sends for the one test that asserts on
the send path. Tests now inject via build_test_state instead of post-build
`state_of(app).x = ...` mutation, create_application test params, or
`patch("...MngrMessenger.send_to_agent")`.
- conftest no longer monkeypatches AgentManager.start (nothing starts the
manager in tests), so PREVENT_MONKEYPATCH_SETATTR drops 1 -> 0.
get_state() is kept as the deliberate Flask-boundary locator: Flask owns
handler invocation, so handlers read the injected state from it but never
construct or patch -- the composition-root pattern applied to a web framework.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vendor/mngr was only refreshed at release time and drifted hundreds of commits behind mngr main between releases, so the minds-launch-to-msg health check (binary from mngr main, agent from this repo's committed vendor) risked a vendor-skew wedge. Add sync-vendor-mngr.yml: twice daily, an hour before each launch-to-msg cron, clone mngr main, re-archive it into vendor/mngr (same git archive flow as mngr's just sync-vendor-mngr), and push to main. No-ops when mngr main has not moved. mngr is public so reading needs no credential, and the push to this repo's own main uses the built-in GITHUB_TOKEN. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ot a None sentinel `messenger: MngrMessenger | None = None` plus an `if messenger is not None else MngrMessenger()` branch read as a test-only seam: `None` was never a real state, only "tests override, production doesn't." Replace it with a module-level `_DEFAULT_MESSENGER` constant (a shared frozen, stateless MngrMessenger with the real discover/send) used directly as the default, mirroring the sibling `mngr_binary: str = _DEFAULT_MNGR_BINARY` in the same signature. No None, no branch; the default is the production value, and tests pass a fake. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add daily workflow to sync vendor/mngr to mngr main
Tracks ccb62268d4f4006f7b741a9bb1c12f21cdf9807d in mngr.
…#196) * docker provider: default to runc, add docker_runsc overlay for gVisor [providers.docker] no longer hardcodes docker_runtime="runsc", so it uses Docker's default runtime (runc), which is available everywhere -- notably macOS, where gVisor's runsc is not. This drops the need for the MNGR__PROVIDERS__DOCKER__DOCKER_RUNTIME=runc workaround on hosts without gVisor. Add a docker_runsc create-template overlay that opts a create into the gVisor runtime: `--template docker --template docker_runsc` reuses the entire docker template body and only switches the container runtime to runsc, so the runc/runsc choice is the single difference with no duplicated body to keep in sync. The minds desktop app stacks the overlay by default on Linux and omits it on macOS. Co-authored-by: Sculptor <sculptor@imbue.com> * docker template test: update stale gVisor-selection docstring PR #196 moves the gVisor (runsc) runtime selection out of the [providers.docker] block and into the opt-in `docker_runsc` create-template overlay. Update the parenthetical in test_docker_template_hardens_start_args_and_drops_sys_ptrace, which still claimed the runtime is selected via `docker_runtime` in [providers.docker] and "not a create-template setting" -- both now false. The assertions are unchanged (this test never asserted the runtime). Co-authored-by: Sculptor <sculptor@imbue.com> --------- Co-authored-by: Gabriel Guralnick <gabriel@imbue.com> Co-authored-by: Sculptor <sculptor@imbue.com>
`build_test_state` exposed nine override parameters but only five are ever passed by a test (config, agent_manager, claude_auth_service, welcome_resender, latchkey_http_client). The other four -- provider_names, include_filters, exclude_filters, and the service-proxy http_client -- were never substituted, so their None-defaulting was dead flexibility. Drop them as parameters and fix those state fields to their production defaults inline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…to gabriel/knowing-to-refresh
The recurring failure mode when editing a service is verifying a change by writing test data into the live store and then deleting it as cleanup, with a too-broad delete taking real records with it. - Scaffold now generates a DATA_DIR constant that defaults to runtime/<name>/ but honors a per-service <PACKAGE_UPPER>_DATA_DIR env var, so a service's data location can be redirected without code edits. - build-web-service documents routing all persistent state through DATA_DIR and why it keeps later edits safe. - update-service adds a 'protect the user's data' section: verify read-only or against a disposable copy pointed at by the override, never delete from the live store to clean up, snapshot before any in-place change, and leave the data dir in place on teardown.
Tracks 8440a5d45199c7287d4a20cdf8d37fb1bf147e0b in mngr.
Tracks c55d2b225c360b12d3a7a8396c98eff65b502301 in mngr.
vendor/mngr: refresh from mngr c55d2b225 (minds v0.3.6)
…plate into mngr/pr-review-ux-actions
Extract the "boot a throwaway instance of a service on a spare port" motion -- previously hand-rolled in the update-service data-isolation prose and baked into the system-interface preview -- into one shared, unopinionated script, .agents/shared/scripts/serve_isolated_instance.py (up/down). It takes the launch command, cwd, and env overrides as parameters, picks a free port, injects it, waits for health, and either returns the loopback URL (for the agent's own data-safe testing) or, given the preview flags, registers it and wraps it in the labeled "preview" frame as a tab. The frame wrapper (preview_wrapper_server.py) moves into shared alongside it. update-system-interface's preview/unpreview become thin adapters that delegate boot/teardown to the shared script, passing the system-interface specifics (neuter layout persistence, probe /api/agents, register inner app + wrapper). The merge and reveal/auto-rollback machinery -- the part whose failure strands the user with no UI -- stays owned by reveal_system_interface.py and was intentionally not generalized. Scaffolded Flask services now read their listen port from a <PKG>_PORT env override (mirroring <PKG>_DATA_DIR), so a throwaway can bind a spare port beside the live one. update-service documents the shared-script invocation for data-safe testing and the optional user-facing preview, and tells agents to retrofit both overrides onto older services.
…-ai/forever-claude-template into mngr/pr-review-ux-actions
Mirror the Jedi-backed Python intelligence for JavaScript/TypeScript files: declaration-aware hover (signature + doc comment) and go-to-definition that resolves locals, parameters, and relative imports across the cached repo tree. Covers .js/.jsx/.ts/.tsx and their .mjs/.cjs/.mts/.cts variants. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The doc-comment hover only picked up the single comment node immediately above a declaration. Two gaps: a const/let/var comment is a sibling of the outer lexical declaration (not the inner declarator), so it was missed entirely; and a run of // lines is one node per line, so only the last was shown. Climb to the statement node and gather the whole contiguous comment block (stopping at a blank line), joined in source order. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two issues surfaced while reviewing a real JS file: - Monaco bundles a TS/JS language service that auto-registers its own hover and go-to-definition providers, producing a duplicate 'Go to Definition' context-menu entry whose navigation fails (it only sees the single open model, no repo context). Disable the built-in providers via setModeConfiguration so our backend-driven ones are the only ones; syntax highlighting (a separate grammar) is unaffected. - Hovering a const showed only 'const NAME: Type', omitting the value. Include the initializer when it is short and single-line, so a constant's value is visible on hover. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Names bound by a destructuring pattern (const { session } = ...) or a
require() call produced no hover/definition, which is most of a CommonJS
file's top-level names. Now:
- expand object/array destructuring patterns into their bound identifiers
when collecting scope bindings, so each name resolves to its binding;
- recognize require('m') like an import -- relative specifiers follow to the
file, external ones fall back to the local binding so hover still works;
- hover shows the binding shape and source, e.g.
const { session } = require('electron').
Member/property access (obj.method) still resolves to nothing: that needs
type inference, which a syntactic parser cannot provide.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e server For repos the user explicitly prepares, JS/TS hover + go-to-definition come from a real TypeScript language service (member + inferred types, library .d.ts) instead of tree-sitter. A subtle 'Enable rich types' pill launches a headless claude -p agent inside the cached tree that installs the repo's dependencies (npm/pnpm/...) and a pinned typescript@5 language server under .pr-review-prep/; jshover/jsdef then use tsintel for prepared trees, falling back to tree-sitter on any error. node_modules and .pr-review-prep are excluded from file listing and search. typescript@5 is pinned deliberately: bare 'npm install typescript' now resolves to 7.x, whose npm package lacks the classic language service API. - prepare.py/tsintel.py + tsintel_server.mjs (Node helper), with injected launcher/transport seams so tests never spawn a real agent or process. - Verified live end to end: a prepared tree resolves a member (method) hover + go-to-def and an inferred variable type that tree-sitter cannot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The app runs in a sandboxed iframe without allow-modals, so window.confirm() was silently ignored and the 'Enable rich types' button did nothing. Switch to the existing confirmDialog() in-app modal, as the merge/close actions already do. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The prepare agent ran with --output-format json, so prepare.log only appeared at the end -- a multi-minute black box. Switch to stream-json and parse events incrementally, writing a readable line per shell command / output / narration to the log as it happens. The detail header now shows a live, auto-scrolling log panel while installing (and on failure, so the error is visible), and the pill pulses; polling tightened to 2.5s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Hang fix: the prepare agent ran in the extracted tree (no .git, carries the repo's .claude hooks), so a Stop hook blocked the headless agent from ever exiting -- it finished its work then hung. Set MNGR_CLAUDE_SUBAGENT_PROXY_CHILD so those hooks skip, as they do for proxied subagents. - Bar host-level changes: the agent had apt-installed Node to satisfy an engine-strict lockfile; the system prompt now forbids system/global installs and steers to --engine-strict=false instead. - Model picker: the Enable-rich-types dialog now offers Opus/Sonnet/Haiku (default Sonnet, remembered in localStorage); threaded through start_prepare. - Live log: tag each streamed line by kind and color agent narration / shell commands / tool calls / output distinctly in the panel. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tsserver's quickInfo documentation only includes JSDoc (/** */), so once a repo was prepared, functions documented with plain // comment blocks lost their descriptions on hover (a regression vs the tree-sitter engine). When tsserver returns no documentation, extract the leading comment block at the symbol's definition via ts.getLeadingCommentRanges (which covers // and /* */), clean the markers, and stop at a blank-line gap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-ai/forever-claude-template into mngr/pr-review-ux-actions
Two chats editing the same service produced four collision shapes: live edits interleaving in the shared working tree, duplicate harden workers (name/branch/runtime dir all derive from the service name), a hardened branch merging over a moved base, and merges landing mid-edit. Serialize the foreground with an advisory per-service editing lease (a cross-agent tk ticket taken by update-service before touching code, released each turn end; another agent's lease is surfaced to the user, never silently broken). Make harden passes single-flight per artifact with coalescing, specified in the new shared harden-contention.md reference: a lead finding a live pass leaves a note instead of dispatching a sibling; before merging, the lead waits out the foreground lease and runs a merge-base freshness check over the artifact's footprint; stale or conflicted passes are never hand-resolved but discarded and superseded by one pass covering everything since the last hardened merge. The foreground always wins: a live edit never waits on a pass, it just makes the pass stale.
…-ai/forever-claude-template into mngr/pr-review-ux-actions
Right-click a diff line -> "Ask an agent" launches a read-only headless claude -p inside the PR's cached checkout to investigate and answer inline, streaming its progress (reusing the type-install log renderer, factored into a shared agent_stream module). Questions + answers + raw investigation logs are persisted per PR and restored on reopen, each with a remove button. The investigator runs in a neutral throwaway working directory and reads the checkout by absolute path, so it does not inherit the reviewed repo's own CLAUDE.md / .claude hooks (which otherwise made it run git and commit). Also: render inline view zones on the diff's first onDidUpdateDiff (fixes zones being dropped before the diff computes), raise the view-zones layer so their buttons are clickable, and add PR_REVIEW_DATA_DIR / PR_REVIEW_PORT overrides for isolated verification. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both agents that spawn `claude -p` here (pr-review's rich-types + ask-an-agent) hit the same footgun: a run whose cwd is inside a repo loads that repo's .claude Stop hooks, and the mngr Stop hook fires `exit 2` on a headless child with no .git, so it hangs; or it follows the repo's CLAUDE.md and does git/tk work it was never asked to. claude_p.py now, on every claude_p_completion / claude_p_task call: - passes `--setting-sources user` so only user-level settings load and the repo's project/local hooks never load, regardless of cwd. This is the auth-safe hook switch (`--bare` also drops hooks but forces API-key auth, which the keyless path lacks -- verified `--setting-sources user` keeps keyless auth and suppresses the project Stop hook from a repo subdir). - sets MNGR_CLAUDE_SUBAGENT_PROXY_CHILD=1 in the child env (the marker mngr's own hooks honor) as defense in depth. claude_p_task gains a `work_dir` param for pointing at an untrusted checkout from a neutral cwd (CLAUDE.md is memory, not a setting source, so it is still discovered from cwd). SKILL.md documents the footgun and the work_dir pattern. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The shared module's docstring said both consumers spawn the agent inside a cached repo checkout, which is only true for prepare.py; the ask investigator runs in a neutral throwaway dir and reads the checkout by absolute path. Clarify that the caller chooses the working directory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A stray core dump appeared at the repo root during a session; ignore core dumps everywhere so they never get committed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
7 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.
Hardening pass for the "Ask an agent" per-line Q&A feature on the
pr-reviewservice (committed change onmngr/pr-review-ux-actions).What this branch contains
dea198cc— the live feature commit (carried forward from the base): right-click a diff line, ask a free-form question, and a read-only headlessclaude -pagent investigates the PR's cached checkout (from a neutral throwaway dir) and answers inline while streaming progress. Questions/answers/raw logs are persisted per PR and restored on reopen.c39d4276— docstring drift fix flagged by the architecture gate (agent_stream.pyclaimed both consumers run inside the checkout; only prepare does).fe70ae77— gitignore a stray core dump.Verification
cd libs/pr_review && uv run pytest— 164 passed, 0 failed (14 ratchets clean).🤖 Generated with Claude Code