Fix AskUserQuestion schema for options array - #3
Merged
agentforce314 merged 1 commit intoApr 26, 2026
Merged
Conversation
Define structured items for options and add multiSelect so tool parameters satisfy OpenAI-compatible JSON Schema validation. Made-with: Cursor
This was referenced May 10, 2026
ericleepi314
added a commit
that referenced
this pull request
May 11, 2026
ch03 state #3: SdkContext + run_with_sdk_context + Session migration
This was referenced May 24, 2026
peroxider
pushed a commit
to peroxider/clawcodex
that referenced
this pull request
Jun 15, 2026
…tegration test Engine/runner/task polish (port-plan §10 remaining items, non-TUI): - agentforce314#7 Result delivery: enqueue_workflow_notification delivers a run's terminal result to the model via the shared <task-notification> queue (notified guard). - agentforce314#8 Run-file location: journals live under ~/.clawcodex/transcripts/workflows/ (per-user session storage) via get_workflow_run_path, not the project tree. - agentforce314#3 retry_workflow_agent: engine retry loop re-spawns a running agent (bounded); also fixes a latent bug where a single-agent skip propagated and aborted the run. - agentforce314#4 ProgressTracker fed to finalize_agent_tool for accurate token totals. - agentforce314#5 isolation="worktree": per-agent wf_<runId>-<idx> git worktrees, best-effort. - agentforce314#6 Live integration test (fake provider) driving LiveAgentRunner -> run_agent -> the real query loop. It caught two real bugs, fixed here: tool DISPATCH resolves by name from the registry (so schema agents need a per-call registry where StructuredOutput is the validating tool), and the injected StructuredOutput was permission-blocked in the subagent (now explicitly allowed). Full workflow suite: 144 passing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
peroxider
pushed a commit
to peroxider/clawcodex
that referenced
this pull request
Jun 23, 2026
…+ P108-B) Layer 0 quick fix for the 3 highest-severity freeze risk points identified in F-108 §十八 audit: * agentforce314#2 TUI Permission modal hang (CRITICAL) * agentforce314#3 TUI AskUserQuestion modal hang (CRITICAL) * agentforce314#5 headless query future never completes (HIGH) - clawcodex_ext/tui/agent_bridge.py: bind _permission_handler / _ask_user_handler waits to _PERMISSION_TIMEOUT_S (30s). On timeout Permission auto-denies (False, False) and AskUser returns {}, both matching existing Esc-cancel semantics. timeout=0 preserves the legacy unbounded wait (F-108 design decision agentforce314#5). - extensions/api/query.py: move the wall-clock budget inside the polling loop. asyncio.wait_for cannot cancel executor futures, so the budget check fires from loop_started_at and surfaces SessionComplete(reason="exit_code=124") + a query_runner.timeout NDJSON event for postmortem. 0 src/ files modified (Decoupling Mandate golden rule agentforce314#1). 12 new unit tests; 483 orchestrator regression tests unaffected. Co-Authored-By: MiniMax-M3 <MiniMax-AI@claude-code-best.win>
ericleepi314
added a commit
that referenced
this pull request
Jul 2, 2026
All four of the book's remote systems (Bridge v1/v2, Direct Connect, Upstream Proxy) are already ported; only Direct Connect is wired/live, and wiring the rest needs claude.ai OAuth + CCR infrastructure the port can't exercise end-to-end. The one live, user-facing, cleanly-portable gap is the book's system #3 -- "tunnel API traffic through infrastructure that might inject credentials or terminate TLS" -- in its every-user form. Enterprise deployments route Anthropic traffic through a gateway / corporate MITM proxy that requires injected auth headers. TS parses curl-style Name: Value from ANTHROPIC_CUSTOM_HEADERS into defaultHeaders (services/api/client.ts:530-554). Python applied it NOWHERE -- the var appeared only in a SAFE_ENV_KEYS allowlist (trust_boundary.py:75), never parsed. Every Anthropic client was built with no default_headers, so a user behind such a gateway had no way to supply the header and every call failed auth. Fix: - src/services/api/custom_headers.py (new): parse_custom_headers (curl-style, split on the FIRST colon, trim name+value, skip blank/colon-less/empty-name lines, CRLF-safe, last-duplicate-wins -- mirrors TS getCustomHeaders; a differential Node-vs-Python run over 18 adversarial inputs showed 0 divergences) + get_anthropic_custom_headers. - Inject default_headers at every first-party Anthropic construction: the provider (anthropic_provider._client_kwargs -- the load-bearing live inference path via query.py -> _ensure_client), the streaming call_model (claude.py), session_title, token_estimation, rename_command. Behind a gateway they ALL need the header, else those calls fail auth. - Non-Anthropic providers (minimax/openai/etc., which target their own base_url) are EXCLUDED -- matches TS (custom headers on the first-party client only). The SDK MERGES default_headers with its own auth headers (x-api-key/anthropic-version retained; verified against anthropic 0.88.0); a gateway overriding x-api-key/Authorization is the intended enterprise BYO-auth use (matching TS -- no override blocklist). Header values can't contain newlines (parser splits on them; httpx validates) -- no header-injection vector; source is the user's own allowlisted env var. Docs: my-docs/ch16-remote-round4-gap-analysis.md + plan + facet + critic verdict (APPROVE) under my-docs/port-improvement-round-4/. Tests: tests/test_ch16_custom_headers_round4.py (15): parse semantics (first-colon-split, trim, skip blank/colonless/empty-name, CRLF, none/empty, last-dup-wins), env reader, provider _client_kwargs injection (+ none-when-unset), the non-Anthropic (Minimax) exclusion, and the live call_model construction (AsyncAnthropic built with default_headers; None when unset). Full suite at the pre-existing 9-failure baseline. Deferred: --permission-prompt-tool (headless MCP approval; only the type exists); explicit HTTPS_PROXY/NO_PROXY (httpx trust_env already honors it); Bedrock/Vertex routing (unwired scaffolding); Gemini base_url nit; wiring the dormant CCR Bridge v1/v2 (needs claude.ai OAuth infra). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 3, 2026
The round-4 LLM memory-relevance recall (#594) ships default-off because flipping it on is a latency+cost+correctness regression. This fixes the four blockers the round-4 critic flagged. Still default-off — zero behavior change on merge. - #2 selector cost (provider-gated pin): the recall side-query ran on the SESSION model (an Opus turn paid full price every turn). TS pins it to a small default (getDefaultSonnetModel). The port wires the small_fast_model setting, but its shipped default is an Anthropic id (claude-3-5-haiku-20241022) valid only on the first-party Anthropic endpoint. So _resolve_recall_model(provider) applies the pin ONLY when the session provider is a first-party AnthropicProvider; Minimax (Anthropic SDK, own endpoint), DeepSeek, OpenAI, OpenRouter fall back to the session model. Without the gate the Anthropic default id 400s on a non-Anthropic endpoint and — since recall swallows errors — silently kills recall every turn (critic M1). small_fast_model_provider pairing for non-Anthropic cheap-recall is deferred. - #3 de-dup reset on compaction: the recall reminder is ephemeral but _memory_surfaced was monotonic — once surfaced, a memory was never re-surfaced. After /compact drops the earlier context, the memory stayed suppressed. _do_compact now clears _memory_surfaced on success (idle-gated — no race with the turn worker). - #4 aggregate turn byte-cap: per-file was 4 KB but 5 selections could inject ~20 KB/turn. build_relevant_memory_reminder_with_paths caps at MAX_TOTAL_SURFACE_CHARS=12 KB and returns the paths ACTUALLY surfaced; get_relevant_memory_reminder de-dups only those, so a cap-trimmed memory stays eligible (the old mark-all was a latent suppress-forever bug the cap would have activated). Back-compat string wrapper retained. - #5 docstring: _maybe_recall_memories said "prepend"; the code appends after the user turn (correct — normalize_messages merges consecutive user messages). Fixed. Deferred #1 (synchronous side-query): the port has only this one prefetch, so there is no concurrent prefetch to overlap without a larger prompt-submit refactor — one small-model call per turn (fast Haiku on the default path). Documented. Docs: my-docs/port-improvement-round-5/r5-1-ch11-memory-enable-blockers.md + critic verdicts (REVISE->APPROVE). Tests: tests/test_r5_ch11_memory_enable_blockers.py (8) incl. the M1 regression guard (non-Anthropic never pins even when the Anthropic-default id is set). Existing memory suite (466) green via the back-compat wrapper. Full suite at the pre-existing baseline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 3, 2026
The round-4 LLM memory-relevance recall (#594) ships default-off because flipping it on is a latency+cost+correctness regression. This fixes the four blockers the round-4 critic flagged. Still default-off — zero behavior change on merge. - #2 selector cost (provider-gated pin): the recall side-query ran on the SESSION model (an Opus turn paid full price every turn). TS pins it to a small default (getDefaultSonnetModel). The port wires the small_fast_model setting, but its shipped default is an Anthropic id (claude-3-5-haiku-20241022) valid only on the first-party Anthropic endpoint. So _resolve_recall_model(provider) applies the pin ONLY when the session provider is a first-party AnthropicProvider; Minimax (Anthropic SDK, own endpoint), DeepSeek, OpenAI, OpenRouter fall back to the session model. Without the gate the Anthropic default id 400s on a non-Anthropic endpoint and — since recall swallows errors — silently kills recall every turn (critic M1). small_fast_model_provider pairing for non-Anthropic cheap-recall is deferred. - #3 de-dup reset on compaction: the recall reminder is ephemeral but _memory_surfaced was monotonic — once surfaced, a memory was never re-surfaced. After /compact drops the earlier context, the memory stayed suppressed. _do_compact now clears _memory_surfaced on success (idle-gated — no race with the turn worker). - #4 aggregate turn byte-cap: per-file was 4 KB but 5 selections could inject ~20 KB/turn. build_relevant_memory_reminder_with_paths caps at MAX_TOTAL_SURFACE_CHARS=12 KB and returns the paths ACTUALLY surfaced; get_relevant_memory_reminder de-dups only those, so a cap-trimmed memory stays eligible (the old mark-all was a latent suppress-forever bug the cap would have activated). Back-compat string wrapper retained. - #5 docstring: _maybe_recall_memories said "prepend"; the code appends after the user turn (correct — normalize_messages merges consecutive user messages). Fixed. Deferred #1 (synchronous side-query): the port has only this one prefetch, so there is no concurrent prefetch to overlap without a larger prompt-submit refactor — one small-model call per turn (fast Haiku on the default path). Documented. Docs: my-docs/port-improvement-round-5/r5-1-ch11-memory-enable-blockers.md + critic verdicts (REVISE->APPROVE). Tests: tests/test_r5_ch11_memory_enable_blockers.py (8) incl. the M1 regression guard (non-Anthropic never pins even when the Anthropic-default id is set). Existing memory suite (466) green via the back-compat wrapper. Full suite at the pre-existing baseline. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
peroxider
pushed a commit
to peroxider/clawcodex
that referenced
this pull request
Jul 3, 2026
…gressive summarization 修复 /goal 系统四个关键问题: agentforce314#1 — _pending_injection 排空(P0 阻断性 Bug) - repl/app.py: chat() finally 中 drain 注入并 enqueue 到 _queued_prompts - chat() 开始时 drain set_new_goal() 遗留的 objective_updated 注入 - TUI 路径 (tui/screens/repl.py): 在 on_agent_run_finished 中 drain 注入 并追加到 app_state.queued_prompts + 投递 QueuedPromptReady agentforce314#2 — <active-goal> 动态截断(原硬编码 240 字) - _dynamic_objective_chars() 根据 token 预算比例动态调优: >60% → 600, 30-60% → 360, 10-30% → 240, <10% → 160, 无预算 → 480 agentforce314#3 — 压缩管道 goal-aware 优先级 - PipelineConfig.goal_active: bool 字段 - goal_active 时 autocompact 阈值 +10%(上限 0.95) - _is_goal_steering_message() 保护含 <goal-steering 的消息不被压缩 - query/engine.py: _check_goal_active() 查询 GoalStateRegistry agentforce314#4 — 渐进式摘要(Progressive Summarization) - GoalState.milestones: list[dict] + MILESTONE_TURN_INTERVAL=15 - state_machine: add_milestone() 纯函数 + 各转换保留 milestones - controller: on_assistant_turn_complete() 每 15 轮自动记录里程碑 - prompts: continuation_prompt 在里程碑边界注入结构化进度要求 - <active-goal> 上下文块嵌入最近 5 个里程碑的 XML 摘要 Tests: 86/86 goal + 145/145 compact + 68/69 stage5 (1 pre-existing CI circular import) all pass. Co-Authored-By: AtomCode (deepseek-v4-flash) <noreply@atomgit.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 4, 2026
…S-2) (#645) * feat(services): autoFix config + hook + runner (SERVICES-2 W1-W3; wiring next) Port of typescript/src/services/autoFix/ — the settings-opt-in feature that runs the user's lint/test command after a file edit and injects <auto_fix_feedback> so the model self-fixes. Python had only the /autofix slash shell; the runtime was absent. - config.py (autoFixConfig.ts): get_auto_fix_config(raw) → AutoFixConfig|None — dict + enabled + at-least-one-of-lint/test (the zod .refine) + numeric bounds (maxRetries 0..10 default 3, timeout 1000..300000 default 30000); None (safeParse-null posture) on any failure. load_auto_fix_config reads settings.extra["autoFix"] (verified: no typed field — lands in extra, like the plugins round's enabledPlugins). - hook.py (autoFixHook.ts): AUTO_FIX_TOOLS = the port's file-mutation registry names (Write/Edit/MultiEdit/NotebookEdit, the file_edit/file_write analog); should_run_auto_fix; build_auto_fix_context (the <auto_fix_feedback> block VERBATIM) + build_max_retries_context (verbatim). - runner.py (autoFixRunner.ts): run_auto_fix_check — lint first, test only if lint passed; each an asyncio.create_subprocess_shell bounded by timeout, killing the process GROUP on timeout (start_new_session + os.killpg, the detached/killTree analog) and reaping to avoid zombies; _build_error_summary verbatim; never-raises (spawn failure → has_errors=False). Verified: config parse/refine/bounds, hook fire + verbatim context, runner lint-fail-skips-test / lint-pass-test-fail / both-clean / timeout-kill (sleep 30 @ 800ms → returns 0.81s, group killed, timed_out) / abort. REMAINING: W4 wiring — a NEW sibling run_auto_fix_step at the orchestrator (tool_execution.py, after run_post_tool_use_hooks), NOT inside that function (it early-returns when no user PostToolUse hooks configured — the teammate-hooks split-gate class; autoFix must run regardless). Retry cap by query_tracking.chain_id. Then tests + suite. Awaiting autofix-critic on the plan's W4 placement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(services): autoFix wiring + all critic findings (SERVICES-2 complete) The autofix-critic caught that TS autoFix is DEAD CODE — AUTO_FIX_TOOLS = {'file_edit','file_write'} never matches the real tool names (Edit/Write, FileEditTool/constants.ts:2 FILE_EDIT_TOOL_NAME='Edit'), verified first-hand. So this port activates the author's INTENT ({Edit,Write}) as a DOCUMENTED, opt-in-gated DIVERGENCE (fixes the reference's dead tool-name set; inert unless a user writes settings.autoFix.enabled). All findings: - B1 (split-gate): run_auto_fix_step is a NEW SIBLING at the orchestrator (tool_execution.py, after run_post_tool_use_hooks) — NOT inside that function, which early-returns when no user PostToolUse hook is configured (the common autoFix case). Pinned by test_fires_with_zero_posttool_hooks. - B2 (accessor): config via get_settings().extra.get("autoFix") (ToolContext has no settings; global read = the per-process equivalent). - M1 (reject-not-clamp): out-of-range/non-int → whole config None (zod .min/.max rejects); .default only when key absent. - M2 (camelCase): reads raw["maxRetries"]/raw["timeout"]. - M3 (reset-on-success): the retry counter is DELETED on a clean run (toolHooks.ts:247), not just incremented on error. - M4 (tool set): {"Edit","Write"} — the author's intent, not the 4-element _FILE_EDIT_TOOLS (NotebookEdit excluded, MultiEdit is a phantom). - M5 (None-guard): chain key handles query_tracking=None → "default". - M6 (10k cap): each stream sliced to 10 000 before combine. - minors: SIGTERM (not SIGKILL) + reap + ProcessLookupError race; mid-flight abort (kills the group, degrades to no-errors); cwd from tool_use_context; the post-hook attachment yield shape (list-wrapped content). tests/services/test_autofix.py (26): config parse/refine/reject/camelCase, hook fire + verbatim contexts + {Edit,Write} intent, runner (lint-skips-test / test-fail / clean / timeout-SIGTERM-kill / 10k cap / pre-set abort / mid-flight abort / no-commands), step (ZERO-hooks regression, non-file skip, retry cap, reset-on-clean, None-query-tracking). Full suite at the 6-failure baseline (7857 passed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(services): autoFix config zod-strictness (critic minor #1) enabled must be a strict bool (a string "false" is truthy in Python — must not turn autoFix ON against intent); a bad-type lint/test rejects the whole config → None, matching zod z.boolean()/z.string() safeParse (not the prior silent coerce/drop). 3 new tests; consistent with the already-strict int path. (Minors #2 retry-map-lifecycle and #3 proxy-coverage accepted as faithful-to-TS / adequate.) Full suite at the 6-failure baseline (7860). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 6, 2026
…C8) (#658) * feat(sandbox): honest unsandboxed guard — settings.sandbox no longer silent (C8) The port has NO sandbox enforcement (TS wraps the external @anthropic-ai/sandbox-runtime) AND SettingsSchema had no `sandbox` field — so a user's settings.sandbox was silently DROPPED at parse AND unenforced: a false sense of security. This is the utils/-round deferred "Sandbox chapter"'s security-meaningful MINIMUM (enforcement itself remains deferred). The guard maps the port's permanently-unavailable sandbox onto TS's OWN documented sandbox-unavailable path (entrypoints/sandboxTypes.ts:96-103): when sandbox.enabled is true but the sandbox cannot start, - failIfUnavailable=true → REFUSE (managed-settings hard gate — never silently run unsandboxed), - failIfUnavailable=false → "a warning is shown and commands run unsandboxed". - SettingsSchema.sandbox: new SandboxSettings dataclass (enabled, failIfUnavailable, autoAllowBashIfSandboxed, allowUnsandboxedCommands, excludedCommands); from_dict accepts camelCase + snake and IGNORES the enforcement-only passthrough keys (network/filesystem/ripgrep/…) so a valid settings.json never fails to load. - permissions/sandbox_guard.py: sandbox_hard_gate_error / sandbox_unsandboxed_ warning / warn_if_unsandboxed_once. - _bash_call: hard-gate → ToolPermissionError refusal; else warn-once + proceed (best-effort, never crashes bash). validate_settings surfaces the hard gate at load too. NON-goal (deferred): native sandbox-exec/bwrap enforcement — the sandbox-native-enforcement sub-chapter (needs critic scoping: macOS sandbox-exec is built-in/no-dep and may be in reach). tests/test_sandbox_guard.py (12): camelCase parse + unknown-key passthrough, the 3-way guard mapping, validate_settings hard-gate-errors vs warning-loads, warn-once, and _bash_call refuses under hard gate / runs+warns otherwise. 134 settings/bash tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs+test(sandbox): C8 scope — bash-tool-scoped (fg+bg), matches TS BashTool Pre-empting the critic's false-hard-gate probe: the guard is BashTool-scoped (both foreground + background bash share _bash_call, guard runs before the run_in_background branch), matching TS where shouldUseSandbox is BashTool-scoped (called only from bashPermissions.ts). Hook subprocesses + MCP stdio are user-authored config TS's sandbox doesn't wrap either → intentionally out of scope. The hard gate is an honest "no unsandboxed BASH", not a false "nothing runs unsandboxed". Added the scope note to the module + a background-bash hard-gate refusal test (13 total). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(sandbox): C8 — hard gate is a REFUSE-TO-START, not per-bash (critic MAJOR) The first cut refused per-_bash_call, but TS's failIfUnavailable is a refuse-to-START at the entrypoints (print.ts:600 / REPL.tsx:2362 "refusing to start without a working sandbox"). So under sandbox.enabled+failIfUnavailable the port STARTED and ran /bg (background/tasks.py Popen, OUTSIDE _bash_call), MCP servers, and hooks UNSANDBOXED while _bash_call refused the same command — a "hard gate" that /bg walked through, contradicting the chapter's no-false- assurance thesis. - Refuse-to-start in agent_server._build_runtime: sandbox_hard_gate_error → sess.init_error → the session refuses to start (the port's existing init-error mechanism), so /bg + MCP + hooks never run. The per-_bash_call guard stays as a CLI-path backstop. - enabledPlatforms (critic minor #3): now parsed + honored — on a platform not in the list, sandbox is disabled (no gate/warning), matching TS (sandboxTypes.ts:104); the port was more aggressive (would refuse where TS runs). - sandbox_guard docstring split into the two mechanisms (BashTool-scoped WARNING vs refuse-to-start HARD GATE). Tests +6 (→19): _build_runtime refuses under hard gate / warning-only starts; skill-slash !-shell fails closed (minor #4); enabledPlatforms off-platform disables / on-platform gates / empty=all. Enforcement scoping (critic-endorsed): guard + refuse-to-start is the MVP; native sandbox-exec/bwrap is a separate scoped chapter (a half-working sandbox is worse than an honest refusal — the chapter's own thesis). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(sandbox): C8 re-review 2 — close the /bg control-path leak The refuse-to-start (v2) closed MCP + hooks (both in _build_runtime), but the critic found /bg + /bg-agent are CONTROL requests, not turns: send_to_agent → _handle_control_request → _do_bgtask → subprocess.Popen(shell=True). That path bypasses _build_runtime AND the turn-path init_error checks, and the session stays fully alive after init_error (sess.start() + the returned handle are unconditional). So `/bg foo` still ran UNSANDBOXED under a hard-gate config the session was supposed to refuse — the same MAJOR, on the one path v2 missed. Fix: guard the top of _handle_control_request — a session with init_error set rejects every control request (replying the gate error) EXCEPT `interrupt` (a benign abort of a non-existent turn). Mirrors the "session not ready" pattern the permission-mode handlers already use. Now /bg + /bg-agent + MCP + hooks all refuse; the docstring's "nothing runs, not just Bash" is truthful. Test replaced (the old one asserted tool_registry is None — the intermediate, not the property): test_bg_run_control_request_refused_under_init_error drives a real bg_run control request against an init_error session and asserts NO subprocess (Popen + _do_bgtask both tripwired to raise) + the gate-error reply; test_interrupt_still_works_under_init_error pins the exemption. 21 sandbox tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 6, 2026
* feat(bash): notify on background-bash completion (C5 Part 1 — enqueueShellNotification) TS notifies when a run_in_background bash task reaches a terminal state (utils/task/framework.ts:289 enqueuePendingNotification; BashTool/prompt.ts: 285-287 "you will be notified when it completes — do not poll"). The port marked the terminal state notified=True WITHOUT sending — the documented forward-trap in background.py (ch10 WI-2), so a bg bash finishing while the model was away went unannounced (silently polled via TaskOutput instead). - task_notification.py: enqueue_shell_notification — mirrors enqueue_agent_notification (the atomic check-and-set duplicate-delivery guard + build_task_notification_xml + enqueue_pending_notification) but gated on LocalShellTaskState instead of LocalAgentTaskState. - background.py: REMOVED the forward-trap notified=True from the terminal replace() (per its own comment's instruction), and call enqueue_shell_notification after the terminal update. The eviction sweeper's notify-before-evict guard (eviction.py:97) now correctly keeps the entry until the notification is delivered, then reclaims it — instead of the old mark-notified-without-sending. Best-effort (never breaks reaping); duplicate-safe (a TaskStop that already notified makes it a no-op). tests/test_shell_completion_notification.py (3): enqueues + marks notified on terminal, duplicate-safe (already-notified → no second), wrong-task-type no-op. 44 background/eviction/notification tests green (the notified=False terminal state is compatible with the eviction guard). Part 2 (the Monitor tool + backpressure) follows separately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(bash): C5 Part 1 — fallback-mark-notified on enqueue failure (no leak) Pre-empting the eviction-leak risk: enqueue_shell_notification is best-effort (except: pass), but with the forward-trap removed the terminal state is notified=False — so if the enqueue raised, the sweeper (which keeps un-notified terminal tasks, eviction.py:97) would leak the task in /tasks forever. Added a fallback: on enqueue failure, force notified=True so the task stays evictable, degrading to the OLD evictable-but-unsent behavior rather than a leak (still strictly better than the pre-C5 silent-drop). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(bash): C5 Part 1 critic — shell-specific summary, XML escaping, kill path The c5p1-critic found the model-facing payload was wrong (the mechanism was right). Real TS analog: enqueueShellNotification (LocalShellTask.tsx:105-172). #1 BLOCKING — wrong summary: reused the AGENT builder, so every bg-bash completion was announced as `Agent "..." completed`, the exit code was dropped, and the failed case said "Unknown error". Added build_shell_notification_xml + _build_shell_summary + BACKGROUND_BASH_SUMMARY_PREFIX ("Background command ") — `Background command "<desc>" completed (exit code N)` / `... failed with exit code N` / `... was stopped`, exit code included only when known. Threaded exit_code + tool_use_id through enqueue_shell_notification ← background.py's rc. #2 MAJOR — no XML escaping: a bg command routinely has < > & (and _reap uses `description or command`), so a raw summary produced a MALFORMED envelope. The shell builder XML-escapes the summary (TS LocalShellTask.tsx:164 escapeXml); the agent builder stays raw (LocalAgentTask.tsx:246-256 is raw — that claim was only true for agents). #3 MAJOR — kill path: TS killTask (killShellTasks.ts:38-44) atomically sets status='killed' + notified=True, which SUPPRESSES the reaper notification (no notification on a user kill). The port sent SIGTERM only → the reaper saw rc≠0 → sent a spurious `failed`. stop_background_bash now sets status='killed' + notified=True; _patch preserves a 'killed' status (doesn't reclassify from the SIGTERM exit code). The duplicate-guard's "TaskStop did it" premise is now actually wired. #4 tests — the old tests never asserted summary CONTENT (passed with the wrong "Agent" text). Added: full-summary snapshots (prefix + exit code + not-"Agent"), metachar XML-escaping, a REAL spawn→reap integration (exit 3 → the "failed with exit code 3" notification delivered), and kill-suppression (kill → status=killed + notified + NO notification). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(bash): C5 Part 1 #3 — mark killed in LocalShellTask.kill before the signal The c5p1-critic reproduced the spurious kill notification 8/8 through the REAL path (TaskStop tool → stop_task → impl.kill = LocalShellTask.kill). My prior mark lived only in stop_background_bash, which stop_task doesn't call — so nothing marked killed/notified before the reaper, which sent "failed with exit code -15". Fix (the critic's exact direction): move the status='killed' + notified=True mutation into LocalShellTask.kill, BEFORE os.killpg, gated on the process still being alive (TS killTask's status==='running' gate, killShellTasks.ts:38-44). This is the production kill path (TaskStop → stop_task → this), so the mark lives here; marking BEFORE the signal closes the reaper-vs-killer race (the reaper is blocked in proc.wait() and can't wake until the signal lands, by which point notified=True is committed → the reaper's enqueue no-ops). The stop_background_bash mark stays as defense-in-depth, also moved before its killpg. _patch preserves a 'killed' status. Test rewritten to drive the REAL path: test_stop_task_kill_suppresses_notification runs stop_task (what TaskStop uses) on a real bg task → asserts status=killed + notified + NO notification (the critic's repro, now 0). + a direct stop_background_bash defense-in-depth test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 6, 2026
… (C5 Part 2) (#665) * feat(tools): the Monitor tool — stream shell output with backpressure (C5 Part 2) Port of MonitorTool.ts (MONITOR_TOOL=true, build.ts:43): run a shell command in the background and STREAM its stdout to the model as ~1s <monitor-output> notifications, instead of the single completion notification the Bash run_in_background path delivers (C5 Part 1). Completes the Monitor-tool chapter. - src/tool_system/tools/monitor.py: the tool (spawns via spawn_background_bash, same detached shell + reaper) + a per-monitor polling thread (_stream_output) that tails the output file, emits each new batch of WHOLE lines (a trailing partial is held for the next poll) as an XML-escaped <monitor-output> notification, and stops when the task leaves 'running' / the 30-min deadline passes. - BACKPRESSURE (the tools-round critic's requirement — the first attempt was DEFERRED because it lacked this; an unbounded poller floods the conversation): after _MONITOR_MAX_NOTIFICATIONS (100) streamed notifications the monitor AUTO-STOPS — kills the shell + sends a final "auto-stopped … too many events … re-run with a tighter filter" notice. TS: "Monitors that produce too many events are automatically stopped." Registered in ALL_STATIC_TOOLS; gated like Bash (check_permissions=_bash_check_permissions); Monitor added to the getAllBaseTools parity snapshot. tests/tools/test_monitor_tool.py (8): registration, streaming (new lines → notifications, partial-line held, metachars escaped), THE backpressure (auto-stops after the cap: kill + final notice / no auto-stop under cap), and lifecycle (stops when the task leaves running / is evicted). 161 tool tests green (only the 2 pre-existing tool_parity default-property baseline failures). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(tools): C5-P2 pre-critic — Monitor safety guard + per-notification size cap Pre-addressing the c5p2-critic's two highest-risk probes: #5 (Monitor bypasses bash safety) — Monitor spawns via spawn_background_bash DIRECTLY, so it was a way AROUND the hardcoded-dangerous-pattern block + the C8 sandbox hard-gate that live in _bash_call (check_permissions only covers the permission RULES, not those hard gates below the permission layer). Extracted bash_command_safety_guard(command) (the shared dangerous-pattern + sandbox hard-gate check) and call it from BOTH _bash_call and _monitor_call so they can't drift and Monitor can't skip them. Tests: `rm -rf /` refused through Monitor; a sandbox hard-gate refused through Monitor. #1 (backpressure bounds count, not bytes) — one poll batches all new lines into ONE notification, so the count cap (100) doesn't bound a FIREHOSE (few-but-huge notifications). Added _MONITOR_MAX_NOTIFICATION_BYTES (8KB): each notification keeps the TAIL + a "[N earlier bytes truncated]" marker, so a single notification can't blow the conversation. Test: a 5000-byte burst → truncated, tail kept. 332 bash/monitor/sandbox tests green (the bash_command_safety_guard extraction didn't regress _bash_call). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(tools): C5-P2 critic — render-compatible envelope, bounded read, delivery framing Addresses the c5p2-critic's remaining findings (it reviewed the pre-a523d37 commit; #5 guard-bypass + the #1 per-notification cap were already fixed there — this closes the rest). MAJOR (envelope/delivery incompatible with the render path): the invented <monitor-output task=".."> envelope broke the drain path — parse_task_id found no <task-id> child (→ None, correlation lost), render_banner mis-rendered ("Background task finished"), and build_notification_turn framed a RUNNING monitor as "finished" every drain. Fix: _monitor_notification_xml emits a real <task-notification> (<task-id>/<output-file>/<status>running</status>/<summary>) that parse_task_id + render_banner understand; build_notification_turn is now status-aware — a batch that is ENTIRELY status=running gets a streaming preamble ("STILL RUNNING — do not report as finished"), while any finished task keeps the completion preamble. So a live monitor surfaces as a streaming update, not a false completion turn. #1 (bounded read — the OOM vector the per-notification cap didn't close): _drain did read_bytes() on the WHOLE file, so a firehose loads everything into memory before truncating. Now seeks to the offset and reads at most _MONITOR_MAX_READ_BYTES (8 MiB, TS diskOutput DEFAULT_MAX_READ_BYTES), advancing pos by bytes read — bounds memory AND fixes the minor UTF-8 rewind drift (offset arithmetic is pure bytes now). Minors: is_concurrency_safe True (TS MonitorTool.ts:54, fire-and-forget) + max_result_size_chars 10_000 (TS:51). tests/tools/test_monitor_tool.py (15): render-path compat (parse_task_id correlates, banner surfaces the line, pure-running → streaming preamble, finished → completion preamble) + bounded-read/size/backpressure/safety-guard. 452 bash/monitor/sandbox/notification/server tests green (the shared build_notification_turn change is additive — finished tasks unaffected). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(tools): C5-P2 #3 minors — final-drain flush + long-line progress Closes the c5p2-critic's two remaining #3 MINORs (non-gating, but cheap): #3.ii — a command whose last output has no trailing newline (printf done) lost that line: the terminal drain now runs with final=True and emits the trailing partial. (The mid-stream drain still holds a partial for the next poll — a line-in-progress shouldn't be split.) #3.i — a single line LONGER than the 8MiB read window had no newline in any window, so _drain rewound and re-read the same bytes every poll forever (bounded, sleep-gated, but a silent stall). Now: when the read window is FULL and has no newline, emit what we have so the stream keeps moving. Tests: partial-held-while-running-then-flushed-on-completion; a 500-byte single line over a tiny read window still makes progress. 15 monitor tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * polish(tools): C5-P2 APPROVE nits — terminal auto-stop status + divergence note The c5p2-critic APPROVED; applying its cheap ask + tidiest minor (both non-gating): - ASK: added a comment at the poller noting the DELIBERATE divergence from TS — the port drives an internal turn per drain, whereas TS injects monitor deltas as passive attachments on the next natural turn. Accepted as port scope (building the attachment pipeline for one tool is disproportionate); the cost is bounded (≤ cap then auto-stop, zero for quiet monitors). Marked so a future maintainer doesn't "fix" it by accident. - minor #3: the auto-stop notice reused status="running", so a TERMINAL "monitor stopped" event got the "STILL RUNNING" streaming preamble. _monitor_notification_xml now takes a status param; the auto-stop notice passes status="killed" → it gets the completion framing. Test asserts the killed status + no STILL-RUNNING preamble. Deferred (critic's other non-gating minors): mixed running+finished batch framing and the production banner showing status-not-line — both cosmetic, self-correcting, follow-up-able. 15 monitor tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 6, 2026
…ult) (#666) * feat(proxy): wire the CCR upstream proxy (C9 — env-gated, no-op by default) The port ported upstream_proxy.py (init_upstream_proxy + get_upstream_proxy_env, gated on CLAUDE_CODE_REMOTE) but left it UNWIRED — zero live callers, and subprocess_env carried a TODO ("when it lands, merge get_upstream_proxy_env"). This wires it, mirroring TS's registerUpstreamProxyEnvFn indirection. - src/utils/subprocess_env.py: register_upstream_proxy_env_fn(fn) + a module provider slot. subprocess_env merges the provider's recipe AFTER the scrub (so an injected HTTPS_PROXY / *_CA_BUNDLE isn't stripped by the anti-exfiltration pass), best-effort (a proxy-env failure can't break child spawning). The indirection keeps this module from statically importing the upstreamproxy package (asyncio/ssl/relay). - src/entrypoints/agent_server_cli.py: _maybe_init_upstream_proxy() at _serve start — when CLAUDE_CODE_REMOTE is truthy, register the provider + await init_upstream_proxy(); FAIL-OPEN (a remote session degrades to direct rather than the server dying). No-op otherwise. DEFAULT BEHAVIOUR UNCHANGED: with CLAUDE_CODE_REMOTE unset (every local build, and the open TS build strips this path entirely), the provider is never registered and subprocess_env is byte-for-byte the base. Low blast radius — the value is that a CCR-remote deployment's ported proxy actually engages instead of being dead code. Scope: the WS mTLS/CA-context and relay.stop()-cleanup pieces from the plan live inside the dormant relay itself (no open-build reference, untestable without CCR infra) — deferred; this lands the reachable, testable wiring. tests/test_c9_upstream_proxy_wiring.py (6): default no-op (env unchanged / empty-provider no-op), proxy-var merge, survives-the-scrub, fail-open, and the entrypoint guard is a no-op without the env. 89 subprocess/proxy tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(proxy): C9 #3 — register the proxy provider only AFTER init succeeds Pre-addressing the c9-critic's highest-risk probe: _maybe_init_upstream_proxy registered the env provider BEFORE await init_upstream_proxy(), so a failed init (fail-open) could leave the provider registered. Reordered to init-first, register-only-on-success — a failed init now leaves ZERO registration, so subprocess_env stays a pure no-op. (Defense-in-depth: get_upstream_proxy_env already returns {} when the proxy state is DISABLED with a clean env, verified.) Tests: a failing init leaves _upstream_proxy_env_fn is None (no half-registered provider); the disabled state returns an empty recipe. 8 C9 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(proxy): C9 critic — scrub-authoritative order + register-first (TS parity) The c9-critic corrected two real errors (and disproved my #4 premise: I'd grepped typescript/ INSIDE the worktree where it doesn't exist — the open build HAS this wiring at init.ts:148-149, 30 files reference CLAUDE_CODE_REMOTE. So C9 is genuine restored parity, not overreach — ship it). MAJOR-A [subprocess_env.py] — scrub/merge order was INVERTED vs TS. TS (subprocessEnv.ts:91-97) merges the proxy recipe THEN runs the scrub loop, so the SCRUB is authoritative/last — no provider can re-introduce a scrubbed secret. The port scrubbed first then merged, so a provider returning a scrub-set key would SURVIVE into the child (a defense-in-depth hole in the exact anti-exfiltration control), and the comment falsely claimed "TS order". Fixed: merge proxy first, THEN scrub — exact TS order; proxy keys aren't in the scrub set so they still survive. New test proves a provider returning ANTHROPIC_API_KEY is scrubbed while HTTPS_PROXY survives. MAJOR-B [agent_server_cli.py] — my earlier #3 "fix" (init-first) diverged from TS register-first (init.ts:148-149) on a FALSE premise: the critic proved a provider over a disabled/failed init returns {} on a clean env (safe in EITHER order). Reverted to register-first for exact TS parity; dropped the false "injects bad vars" rationale. Test updated to assert the real safe property (a failed init injects nothing because the disabled state returns {}). Also (critic notes): wired _serve_stdio too (TS inits in shared init.ts for ALL sessions, not just HTTP — a CCR-remote stdio container would otherwise miss the proxy); fixed the upstream_proxy docstring (9-var→8-var, dropped the nonexistent lowercase `proxy` claim). 9 C9 tests green (default no-op, proxy-merge, scrub-authoritative, fail-open, disabled-returns-empty, entrypoint-guard). subprocess/proxy suite green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 7, 2026
…S-2) (#645) * feat(services): autoFix config + hook + runner (SERVICES-2 W1-W3; wiring next) Port of typescript/src/services/autoFix/ — the settings-opt-in feature that runs the user's lint/test command after a file edit and injects <auto_fix_feedback> so the model self-fixes. Python had only the /autofix slash shell; the runtime was absent. - config.py (autoFixConfig.ts): get_auto_fix_config(raw) → AutoFixConfig|None — dict + enabled + at-least-one-of-lint/test (the zod .refine) + numeric bounds (maxRetries 0..10 default 3, timeout 1000..300000 default 30000); None (safeParse-null posture) on any failure. load_auto_fix_config reads settings.extra["autoFix"] (verified: no typed field — lands in extra, like the plugins round's enabledPlugins). - hook.py (autoFixHook.ts): AUTO_FIX_TOOLS = the port's file-mutation registry names (Write/Edit/MultiEdit/NotebookEdit, the file_edit/file_write analog); should_run_auto_fix; build_auto_fix_context (the <auto_fix_feedback> block VERBATIM) + build_max_retries_context (verbatim). - runner.py (autoFixRunner.ts): run_auto_fix_check — lint first, test only if lint passed; each an asyncio.create_subprocess_shell bounded by timeout, killing the process GROUP on timeout (start_new_session + os.killpg, the detached/killTree analog) and reaping to avoid zombies; _build_error_summary verbatim; never-raises (spawn failure → has_errors=False). Verified: config parse/refine/bounds, hook fire + verbatim context, runner lint-fail-skips-test / lint-pass-test-fail / both-clean / timeout-kill (sleep 30 @ 800ms → returns 0.81s, group killed, timed_out) / abort. REMAINING: W4 wiring — a NEW sibling run_auto_fix_step at the orchestrator (tool_execution.py, after run_post_tool_use_hooks), NOT inside that function (it early-returns when no user PostToolUse hooks configured — the teammate-hooks split-gate class; autoFix must run regardless). Retry cap by query_tracking.chain_id. Then tests + suite. Awaiting autofix-critic on the plan's W4 placement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(services): autoFix wiring + all critic findings (SERVICES-2 complete) The autofix-critic caught that TS autoFix is DEAD CODE — AUTO_FIX_TOOLS = {'file_edit','file_write'} never matches the real tool names (Edit/Write, FileEditTool/constants.ts:2 FILE_EDIT_TOOL_NAME='Edit'), verified first-hand. So this port activates the author's INTENT ({Edit,Write}) as a DOCUMENTED, opt-in-gated DIVERGENCE (fixes the reference's dead tool-name set; inert unless a user writes settings.autoFix.enabled). All findings: - B1 (split-gate): run_auto_fix_step is a NEW SIBLING at the orchestrator (tool_execution.py, after run_post_tool_use_hooks) — NOT inside that function, which early-returns when no user PostToolUse hook is configured (the common autoFix case). Pinned by test_fires_with_zero_posttool_hooks. - B2 (accessor): config via get_settings().extra.get("autoFix") (ToolContext has no settings; global read = the per-process equivalent). - M1 (reject-not-clamp): out-of-range/non-int → whole config None (zod .min/.max rejects); .default only when key absent. - M2 (camelCase): reads raw["maxRetries"]/raw["timeout"]. - M3 (reset-on-success): the retry counter is DELETED on a clean run (toolHooks.ts:247), not just incremented on error. - M4 (tool set): {"Edit","Write"} — the author's intent, not the 4-element _FILE_EDIT_TOOLS (NotebookEdit excluded, MultiEdit is a phantom). - M5 (None-guard): chain key handles query_tracking=None → "default". - M6 (10k cap): each stream sliced to 10 000 before combine. - minors: SIGTERM (not SIGKILL) + reap + ProcessLookupError race; mid-flight abort (kills the group, degrades to no-errors); cwd from tool_use_context; the post-hook attachment yield shape (list-wrapped content). tests/services/test_autofix.py (26): config parse/refine/reject/camelCase, hook fire + verbatim contexts + {Edit,Write} intent, runner (lint-skips-test / test-fail / clean / timeout-SIGTERM-kill / 10k cap / pre-set abort / mid-flight abort / no-commands), step (ZERO-hooks regression, non-file skip, retry cap, reset-on-clean, None-query-tracking). Full suite at the 6-failure baseline (7857 passed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(services): autoFix config zod-strictness (critic minor #1) enabled must be a strict bool (a string "false" is truthy in Python — must not turn autoFix ON against intent); a bad-type lint/test rejects the whole config → None, matching zod z.boolean()/z.string() safeParse (not the prior silent coerce/drop). 3 new tests; consistent with the already-strict int path. (Minors #2 retry-map-lifecycle and #3 proxy-coverage accepted as faithful-to-TS / adequate.) Full suite at the 6-failure baseline (7860). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 7, 2026
…C8) (#658) * feat(sandbox): honest unsandboxed guard — settings.sandbox no longer silent (C8) The port has NO sandbox enforcement (TS wraps the external @anthropic-ai/sandbox-runtime) AND SettingsSchema had no `sandbox` field — so a user's settings.sandbox was silently DROPPED at parse AND unenforced: a false sense of security. This is the utils/-round deferred "Sandbox chapter"'s security-meaningful MINIMUM (enforcement itself remains deferred). The guard maps the port's permanently-unavailable sandbox onto TS's OWN documented sandbox-unavailable path (entrypoints/sandboxTypes.ts:96-103): when sandbox.enabled is true but the sandbox cannot start, - failIfUnavailable=true → REFUSE (managed-settings hard gate — never silently run unsandboxed), - failIfUnavailable=false → "a warning is shown and commands run unsandboxed". - SettingsSchema.sandbox: new SandboxSettings dataclass (enabled, failIfUnavailable, autoAllowBashIfSandboxed, allowUnsandboxedCommands, excludedCommands); from_dict accepts camelCase + snake and IGNORES the enforcement-only passthrough keys (network/filesystem/ripgrep/…) so a valid settings.json never fails to load. - permissions/sandbox_guard.py: sandbox_hard_gate_error / sandbox_unsandboxed_ warning / warn_if_unsandboxed_once. - _bash_call: hard-gate → ToolPermissionError refusal; else warn-once + proceed (best-effort, never crashes bash). validate_settings surfaces the hard gate at load too. NON-goal (deferred): native sandbox-exec/bwrap enforcement — the sandbox-native-enforcement sub-chapter (needs critic scoping: macOS sandbox-exec is built-in/no-dep and may be in reach). tests/test_sandbox_guard.py (12): camelCase parse + unknown-key passthrough, the 3-way guard mapping, validate_settings hard-gate-errors vs warning-loads, warn-once, and _bash_call refuses under hard gate / runs+warns otherwise. 134 settings/bash tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs+test(sandbox): C8 scope — bash-tool-scoped (fg+bg), matches TS BashTool Pre-empting the critic's false-hard-gate probe: the guard is BashTool-scoped (both foreground + background bash share _bash_call, guard runs before the run_in_background branch), matching TS where shouldUseSandbox is BashTool-scoped (called only from bashPermissions.ts). Hook subprocesses + MCP stdio are user-authored config TS's sandbox doesn't wrap either → intentionally out of scope. The hard gate is an honest "no unsandboxed BASH", not a false "nothing runs unsandboxed". Added the scope note to the module + a background-bash hard-gate refusal test (13 total). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(sandbox): C8 — hard gate is a REFUSE-TO-START, not per-bash (critic MAJOR) The first cut refused per-_bash_call, but TS's failIfUnavailable is a refuse-to-START at the entrypoints (print.ts:600 / REPL.tsx:2362 "refusing to start without a working sandbox"). So under sandbox.enabled+failIfUnavailable the port STARTED and ran /bg (background/tasks.py Popen, OUTSIDE _bash_call), MCP servers, and hooks UNSANDBOXED while _bash_call refused the same command — a "hard gate" that /bg walked through, contradicting the chapter's no-false- assurance thesis. - Refuse-to-start in agent_server._build_runtime: sandbox_hard_gate_error → sess.init_error → the session refuses to start (the port's existing init-error mechanism), so /bg + MCP + hooks never run. The per-_bash_call guard stays as a CLI-path backstop. - enabledPlatforms (critic minor #3): now parsed + honored — on a platform not in the list, sandbox is disabled (no gate/warning), matching TS (sandboxTypes.ts:104); the port was more aggressive (would refuse where TS runs). - sandbox_guard docstring split into the two mechanisms (BashTool-scoped WARNING vs refuse-to-start HARD GATE). Tests +6 (→19): _build_runtime refuses under hard gate / warning-only starts; skill-slash !-shell fails closed (minor #4); enabledPlatforms off-platform disables / on-platform gates / empty=all. Enforcement scoping (critic-endorsed): guard + refuse-to-start is the MVP; native sandbox-exec/bwrap is a separate scoped chapter (a half-working sandbox is worse than an honest refusal — the chapter's own thesis). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(sandbox): C8 re-review 2 — close the /bg control-path leak The refuse-to-start (v2) closed MCP + hooks (both in _build_runtime), but the critic found /bg + /bg-agent are CONTROL requests, not turns: send_to_agent → _handle_control_request → _do_bgtask → subprocess.Popen(shell=True). That path bypasses _build_runtime AND the turn-path init_error checks, and the session stays fully alive after init_error (sess.start() + the returned handle are unconditional). So `/bg foo` still ran UNSANDBOXED under a hard-gate config the session was supposed to refuse — the same MAJOR, on the one path v2 missed. Fix: guard the top of _handle_control_request — a session with init_error set rejects every control request (replying the gate error) EXCEPT `interrupt` (a benign abort of a non-existent turn). Mirrors the "session not ready" pattern the permission-mode handlers already use. Now /bg + /bg-agent + MCP + hooks all refuse; the docstring's "nothing runs, not just Bash" is truthful. Test replaced (the old one asserted tool_registry is None — the intermediate, not the property): test_bg_run_control_request_refused_under_init_error drives a real bg_run control request against an init_error session and asserts NO subprocess (Popen + _do_bgtask both tripwired to raise) + the gate-error reply; test_interrupt_still_works_under_init_error pins the exemption. 21 sandbox tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 7, 2026
* feat(bash): notify on background-bash completion (C5 Part 1 — enqueueShellNotification) TS notifies when a run_in_background bash task reaches a terminal state (utils/task/framework.ts:289 enqueuePendingNotification; BashTool/prompt.ts: 285-287 "you will be notified when it completes — do not poll"). The port marked the terminal state notified=True WITHOUT sending — the documented forward-trap in background.py (ch10 WI-2), so a bg bash finishing while the model was away went unannounced (silently polled via TaskOutput instead). - task_notification.py: enqueue_shell_notification — mirrors enqueue_agent_notification (the atomic check-and-set duplicate-delivery guard + build_task_notification_xml + enqueue_pending_notification) but gated on LocalShellTaskState instead of LocalAgentTaskState. - background.py: REMOVED the forward-trap notified=True from the terminal replace() (per its own comment's instruction), and call enqueue_shell_notification after the terminal update. The eviction sweeper's notify-before-evict guard (eviction.py:97) now correctly keeps the entry until the notification is delivered, then reclaims it — instead of the old mark-notified-without-sending. Best-effort (never breaks reaping); duplicate-safe (a TaskStop that already notified makes it a no-op). tests/test_shell_completion_notification.py (3): enqueues + marks notified on terminal, duplicate-safe (already-notified → no second), wrong-task-type no-op. 44 background/eviction/notification tests green (the notified=False terminal state is compatible with the eviction guard). Part 2 (the Monitor tool + backpressure) follows separately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(bash): C5 Part 1 — fallback-mark-notified on enqueue failure (no leak) Pre-empting the eviction-leak risk: enqueue_shell_notification is best-effort (except: pass), but with the forward-trap removed the terminal state is notified=False — so if the enqueue raised, the sweeper (which keeps un-notified terminal tasks, eviction.py:97) would leak the task in /tasks forever. Added a fallback: on enqueue failure, force notified=True so the task stays evictable, degrading to the OLD evictable-but-unsent behavior rather than a leak (still strictly better than the pre-C5 silent-drop). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(bash): C5 Part 1 critic — shell-specific summary, XML escaping, kill path The c5p1-critic found the model-facing payload was wrong (the mechanism was right). Real TS analog: enqueueShellNotification (LocalShellTask.tsx:105-172). #1 BLOCKING — wrong summary: reused the AGENT builder, so every bg-bash completion was announced as `Agent "..." completed`, the exit code was dropped, and the failed case said "Unknown error". Added build_shell_notification_xml + _build_shell_summary + BACKGROUND_BASH_SUMMARY_PREFIX ("Background command ") — `Background command "<desc>" completed (exit code N)` / `... failed with exit code N` / `... was stopped`, exit code included only when known. Threaded exit_code + tool_use_id through enqueue_shell_notification ← background.py's rc. #2 MAJOR — no XML escaping: a bg command routinely has < > & (and _reap uses `description or command`), so a raw summary produced a MALFORMED envelope. The shell builder XML-escapes the summary (TS LocalShellTask.tsx:164 escapeXml); the agent builder stays raw (LocalAgentTask.tsx:246-256 is raw — that claim was only true for agents). #3 MAJOR — kill path: TS killTask (killShellTasks.ts:38-44) atomically sets status='killed' + notified=True, which SUPPRESSES the reaper notification (no notification on a user kill). The port sent SIGTERM only → the reaper saw rc≠0 → sent a spurious `failed`. stop_background_bash now sets status='killed' + notified=True; _patch preserves a 'killed' status (doesn't reclassify from the SIGTERM exit code). The duplicate-guard's "TaskStop did it" premise is now actually wired. #4 tests — the old tests never asserted summary CONTENT (passed with the wrong "Agent" text). Added: full-summary snapshots (prefix + exit code + not-"Agent"), metachar XML-escaping, a REAL spawn→reap integration (exit 3 → the "failed with exit code 3" notification delivered), and kill-suppression (kill → status=killed + notified + NO notification). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(bash): C5 Part 1 #3 — mark killed in LocalShellTask.kill before the signal The c5p1-critic reproduced the spurious kill notification 8/8 through the REAL path (TaskStop tool → stop_task → impl.kill = LocalShellTask.kill). My prior mark lived only in stop_background_bash, which stop_task doesn't call — so nothing marked killed/notified before the reaper, which sent "failed with exit code -15". Fix (the critic's exact direction): move the status='killed' + notified=True mutation into LocalShellTask.kill, BEFORE os.killpg, gated on the process still being alive (TS killTask's status==='running' gate, killShellTasks.ts:38-44). This is the production kill path (TaskStop → stop_task → this), so the mark lives here; marking BEFORE the signal closes the reaper-vs-killer race (the reaper is blocked in proc.wait() and can't wake until the signal lands, by which point notified=True is committed → the reaper's enqueue no-ops). The stop_background_bash mark stays as defense-in-depth, also moved before its killpg. _patch preserves a 'killed' status. Test rewritten to drive the REAL path: test_stop_task_kill_suppresses_notification runs stop_task (what TaskStop uses) on a real bg task → asserts status=killed + notified + NO notification (the critic's repro, now 0). + a direct stop_background_bash defense-in-depth test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 7, 2026
… (C5 Part 2) (#665) * feat(tools): the Monitor tool — stream shell output with backpressure (C5 Part 2) Port of MonitorTool.ts (MONITOR_TOOL=true, build.ts:43): run a shell command in the background and STREAM its stdout to the model as ~1s <monitor-output> notifications, instead of the single completion notification the Bash run_in_background path delivers (C5 Part 1). Completes the Monitor-tool chapter. - src/tool_system/tools/monitor.py: the tool (spawns via spawn_background_bash, same detached shell + reaper) + a per-monitor polling thread (_stream_output) that tails the output file, emits each new batch of WHOLE lines (a trailing partial is held for the next poll) as an XML-escaped <monitor-output> notification, and stops when the task leaves 'running' / the 30-min deadline passes. - BACKPRESSURE (the tools-round critic's requirement — the first attempt was DEFERRED because it lacked this; an unbounded poller floods the conversation): after _MONITOR_MAX_NOTIFICATIONS (100) streamed notifications the monitor AUTO-STOPS — kills the shell + sends a final "auto-stopped … too many events … re-run with a tighter filter" notice. TS: "Monitors that produce too many events are automatically stopped." Registered in ALL_STATIC_TOOLS; gated like Bash (check_permissions=_bash_check_permissions); Monitor added to the getAllBaseTools parity snapshot. tests/tools/test_monitor_tool.py (8): registration, streaming (new lines → notifications, partial-line held, metachars escaped), THE backpressure (auto-stops after the cap: kill + final notice / no auto-stop under cap), and lifecycle (stops when the task leaves running / is evicted). 161 tool tests green (only the 2 pre-existing tool_parity default-property baseline failures). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(tools): C5-P2 pre-critic — Monitor safety guard + per-notification size cap Pre-addressing the c5p2-critic's two highest-risk probes: #5 (Monitor bypasses bash safety) — Monitor spawns via spawn_background_bash DIRECTLY, so it was a way AROUND the hardcoded-dangerous-pattern block + the C8 sandbox hard-gate that live in _bash_call (check_permissions only covers the permission RULES, not those hard gates below the permission layer). Extracted bash_command_safety_guard(command) (the shared dangerous-pattern + sandbox hard-gate check) and call it from BOTH _bash_call and _monitor_call so they can't drift and Monitor can't skip them. Tests: `rm -rf /` refused through Monitor; a sandbox hard-gate refused through Monitor. #1 (backpressure bounds count, not bytes) — one poll batches all new lines into ONE notification, so the count cap (100) doesn't bound a FIREHOSE (few-but-huge notifications). Added _MONITOR_MAX_NOTIFICATION_BYTES (8KB): each notification keeps the TAIL + a "[N earlier bytes truncated]" marker, so a single notification can't blow the conversation. Test: a 5000-byte burst → truncated, tail kept. 332 bash/monitor/sandbox tests green (the bash_command_safety_guard extraction didn't regress _bash_call). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(tools): C5-P2 critic — render-compatible envelope, bounded read, delivery framing Addresses the c5p2-critic's remaining findings (it reviewed the pre-67843e1 commit; #5 guard-bypass + the #1 per-notification cap were already fixed there — this closes the rest). MAJOR (envelope/delivery incompatible with the render path): the invented <monitor-output task=".."> envelope broke the drain path — parse_task_id found no <task-id> child (→ None, correlation lost), render_banner mis-rendered ("Background task finished"), and build_notification_turn framed a RUNNING monitor as "finished" every drain. Fix: _monitor_notification_xml emits a real <task-notification> (<task-id>/<output-file>/<status>running</status>/<summary>) that parse_task_id + render_banner understand; build_notification_turn is now status-aware — a batch that is ENTIRELY status=running gets a streaming preamble ("STILL RUNNING — do not report as finished"), while any finished task keeps the completion preamble. So a live monitor surfaces as a streaming update, not a false completion turn. #1 (bounded read — the OOM vector the per-notification cap didn't close): _drain did read_bytes() on the WHOLE file, so a firehose loads everything into memory before truncating. Now seeks to the offset and reads at most _MONITOR_MAX_READ_BYTES (8 MiB, TS diskOutput DEFAULT_MAX_READ_BYTES), advancing pos by bytes read — bounds memory AND fixes the minor UTF-8 rewind drift (offset arithmetic is pure bytes now). Minors: is_concurrency_safe True (TS MonitorTool.ts:54, fire-and-forget) + max_result_size_chars 10_000 (TS:51). tests/tools/test_monitor_tool.py (15): render-path compat (parse_task_id correlates, banner surfaces the line, pure-running → streaming preamble, finished → completion preamble) + bounded-read/size/backpressure/safety-guard. 452 bash/monitor/sandbox/notification/server tests green (the shared build_notification_turn change is additive — finished tasks unaffected). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(tools): C5-P2 #3 minors — final-drain flush + long-line progress Closes the c5p2-critic's two remaining #3 MINORs (non-gating, but cheap): #3.ii — a command whose last output has no trailing newline (printf done) lost that line: the terminal drain now runs with final=True and emits the trailing partial. (The mid-stream drain still holds a partial for the next poll — a line-in-progress shouldn't be split.) #3.i — a single line LONGER than the 8MiB read window had no newline in any window, so _drain rewound and re-read the same bytes every poll forever (bounded, sleep-gated, but a silent stall). Now: when the read window is FULL and has no newline, emit what we have so the stream keeps moving. Tests: partial-held-while-running-then-flushed-on-completion; a 500-byte single line over a tiny read window still makes progress. 15 monitor tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * polish(tools): C5-P2 APPROVE nits — terminal auto-stop status + divergence note The c5p2-critic APPROVED; applying its cheap ask + tidiest minor (both non-gating): - ASK: added a comment at the poller noting the DELIBERATE divergence from TS — the port drives an internal turn per drain, whereas TS injects monitor deltas as passive attachments on the next natural turn. Accepted as port scope (building the attachment pipeline for one tool is disproportionate); the cost is bounded (≤ cap then auto-stop, zero for quiet monitors). Marked so a future maintainer doesn't "fix" it by accident. - minor #3: the auto-stop notice reused status="running", so a TERMINAL "monitor stopped" event got the "STILL RUNNING" streaming preamble. _monitor_notification_xml now takes a status param; the auto-stop notice passes status="killed" → it gets the completion framing. Test asserts the killed status + no STILL-RUNNING preamble. Deferred (critic's other non-gating minors): mixed running+finished batch framing and the production banner showing status-not-line — both cosmetic, self-correcting, follow-up-able. 15 monitor tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 7, 2026
…ult) (#666) * feat(proxy): wire the CCR upstream proxy (C9 — env-gated, no-op by default) The port ported upstream_proxy.py (init_upstream_proxy + get_upstream_proxy_env, gated on CLAUDE_CODE_REMOTE) but left it UNWIRED — zero live callers, and subprocess_env carried a TODO ("when it lands, merge get_upstream_proxy_env"). This wires it, mirroring TS's registerUpstreamProxyEnvFn indirection. - src/utils/subprocess_env.py: register_upstream_proxy_env_fn(fn) + a module provider slot. subprocess_env merges the provider's recipe AFTER the scrub (so an injected HTTPS_PROXY / *_CA_BUNDLE isn't stripped by the anti-exfiltration pass), best-effort (a proxy-env failure can't break child spawning). The indirection keeps this module from statically importing the upstreamproxy package (asyncio/ssl/relay). - src/entrypoints/agent_server_cli.py: _maybe_init_upstream_proxy() at _serve start — when CLAUDE_CODE_REMOTE is truthy, register the provider + await init_upstream_proxy(); FAIL-OPEN (a remote session degrades to direct rather than the server dying). No-op otherwise. DEFAULT BEHAVIOUR UNCHANGED: with CLAUDE_CODE_REMOTE unset (every local build, and the open TS build strips this path entirely), the provider is never registered and subprocess_env is byte-for-byte the base. Low blast radius — the value is that a CCR-remote deployment's ported proxy actually engages instead of being dead code. Scope: the WS mTLS/CA-context and relay.stop()-cleanup pieces from the plan live inside the dormant relay itself (no open-build reference, untestable without CCR infra) — deferred; this lands the reachable, testable wiring. tests/test_c9_upstream_proxy_wiring.py (6): default no-op (env unchanged / empty-provider no-op), proxy-var merge, survives-the-scrub, fail-open, and the entrypoint guard is a no-op without the env. 89 subprocess/proxy tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(proxy): C9 #3 — register the proxy provider only AFTER init succeeds Pre-addressing the c9-critic's highest-risk probe: _maybe_init_upstream_proxy registered the env provider BEFORE await init_upstream_proxy(), so a failed init (fail-open) could leave the provider registered. Reordered to init-first, register-only-on-success — a failed init now leaves ZERO registration, so subprocess_env stays a pure no-op. (Defense-in-depth: get_upstream_proxy_env already returns {} when the proxy state is DISABLED with a clean env, verified.) Tests: a failing init leaves _upstream_proxy_env_fn is None (no half-registered provider); the disabled state returns an empty recipe. 8 C9 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(proxy): C9 critic — scrub-authoritative order + register-first (TS parity) The c9-critic corrected two real errors (and disproved my #4 premise: I'd grepped typescript/ INSIDE the worktree where it doesn't exist — the open build HAS this wiring at init.ts:148-149, 30 files reference CLAUDE_CODE_REMOTE. So C9 is genuine restored parity, not overreach — ship it). MAJOR-A [subprocess_env.py] — scrub/merge order was INVERTED vs TS. TS (subprocessEnv.ts:91-97) merges the proxy recipe THEN runs the scrub loop, so the SCRUB is authoritative/last — no provider can re-introduce a scrubbed secret. The port scrubbed first then merged, so a provider returning a scrub-set key would SURVIVE into the child (a defense-in-depth hole in the exact anti-exfiltration control), and the comment falsely claimed "TS order". Fixed: merge proxy first, THEN scrub — exact TS order; proxy keys aren't in the scrub set so they still survive. New test proves a provider returning ANTHROPIC_API_KEY is scrubbed while HTTPS_PROXY survives. MAJOR-B [agent_server_cli.py] — my earlier #3 "fix" (init-first) diverged from TS register-first (init.ts:148-149) on a FALSE premise: the critic proved a provider over a disabled/failed init returns {} on a clean env (safe in EITHER order). Reverted to register-first for exact TS parity; dropped the false "injects bad vars" rationale. Test updated to assert the real safe property (a failed init injects nothing because the disabled state returns {}). Also (critic notes): wired _serve_stdio too (TS inits in shared init.ts for ALL sessions, not just HTTP — a CCR-remote stdio container would otherwise miss the proxy); fixed the upstream_proxy docstring (9-var→8-var, dropped the nonexistent lowercase `proxy` claim). 9 C9 tests green (default no-op, proxy-merge, scrub-authoritative, fail-open, disabled-returns-empty, entrypoint-guard). subprocess/proxy suite green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
singlaamitesh
pushed a commit
to singlaamitesh/clawcodex
that referenced
this pull request
Jul 7, 2026
…tion-schema Fix AskUserQuestion schema for options array
singlaamitesh
pushed a commit
to singlaamitesh/clawcodex
that referenced
this pull request
Jul 7, 2026
…nd/background promotion
Phase 9 + Phase 10 of the chapter-10 orchestration port — the closing
chunk. Permission forwarding from worker to leader, mailbox poller with
receiver-side defense-in-depth gate, and foreground-to-background
promotion via asyncio.wait(FIRST_COMPLETED).
What lands (Phase 9 — permission forwarding):
- src/services/swarm/leader_permission_bridge.py: PermissionRequest
frozen dataclass + to_envelope. create_permission_request with
CSPRNG-backed request_id. register_permission_callback /
unregister_permission_callback / get_pending_request_ids — module-
level dict guarded by RLock. deliver_permission_decision fires the
registered callback and auto-unregisters via atomic pop; idempotent
on duplicate decisions; logged-but-not-crashing on callback
exceptions. The pop happens under the lock; the callback fires
AFTER the lock is released so callbacks may safely re-enter the
registry without deadlocking. send_permission_request_via_mailbox
writes the envelope to the leader's inbox.
- src/services/swarm/mailbox_poller.py: daemon thread with idempotent
start, ~1s tick (faster than eviction's 5s — permission-request UX
matters more than eviction lag). Per-recipient <inbox>.read_offset
cursor file so envelopes don't replay across daemon restarts.
Envelope dispatch covers 5 types:
- plain-text (envelope is None) → pending_user_messages queue
- shutdown_request → shutdown_requested=True
- plan_approval_response (lead-verified) → clears
awaiting_plan_approval + sets permission_mode
- permission_response → forwarded to deliver_permission_decision
- shutdown_response → logged (callback registry is run-loop
integration ticket, on Phase-11 backlog)
Receiver-side from=lead_agent_id defense-in-depth gate (the deferred
Chunk-F D1 + critic concern C3 from the refactoring-plan review).
When the poller sees a plan_approval_response envelope with
envelope["from"] mismatching expected_lead_agent_id, it logs a WARNING
and drops the envelope without crashing the poller or mutating
teammate state. Sender-side gate from Phase 7 + receiver-side gate
here = defense-in-depth against forged envelopes written through
any non-SendMessage path.
What lands (Phase 10 — foreground/background promotion):
- src/agent/foreground_promotion.py: register_agent_foreground /
register_agent_background / unregister_agent_foreground lifecycle
helpers — atomic mutators on the registry.
run_with_background_escape(iterator, *, background_signal,
on_background) is the direct translation of TS's Promise.race for
the chapter's foreground-to-background transition. Drives an async
iterator, races __anext__ against background_signal.wait() via
asyncio.wait(FIRST_COMPLETED), cancels the unfinished side cleanly.
Returns (messages_collected, was_backgrounded). Optional
on_background callback fires inside the bg-signal branch so the
caller can swap abort controllers / flip is_backgrounded while
state is well-defined.
Iterator exceptions on the normal path propagate to the caller
(re-raised, not silently dropped into was_backgrounded=False).
Tests (30 across 3 files):
- tests/services/swarm/test_leader_permission_bridge.py (9):
PermissionRequest dataclass shape, to_envelope serialization,
create_permission_request CSPRNG id generation, callback
registration round-trip, atomic-pop-then-fire-outside-lock,
idempotent delivery on duplicate request_id, callback exception
is logged-not-crashing, unregister cleanup, get_pending_request_ids
snapshot.
- tests/services/swarm/test_mailbox_poller.py (10): daemon lifecycle
start/stop idempotent, all 5 envelope dispatch types, per-recipient
offset cursor advances and survives restarts, 3 receiver-side
defense-in-depth gate tests (lead approves, non-lead from-claim
dropped, no-lead-configured drops).
- tests/test_foreground_promotion.py (11): chapter's 4 abort scenarios
(foreground completes naturally, bg signal during iteration promotes,
bg signal already set returns immediately, iterator exception
propagates without swallowing); register/unregister/transition state
helpers; on_background callback fires with well-defined state.
Phase 9+10 acceptance gates: permission request escalates worker →
leader mailbox; register_permission_callback allows leader's approval
to fire onAllow/onReject; mailbox poller verifies envelope from matches
lead_agent_id for plan_approval_response with log-and-drop on mismatch;
fg→bg promotion via asyncio.wait(FIRST_COMPLETED) with clean
cancellation drain on both branches; iterator exceptions propagate;
all Phase 0-8 tests still green.
This commit closes Task agentforce314#3: 17/17 gap-analysis items mapped to landed
WIs across 8 chunks (Phase 0 → Phase 10). The foundational Task state
machine that the gap analysis flagged as "essentially absent in Python"
is now the typed substrate ~14 modules depend on. ~349 chapter-10
tests added; full suite runs 3650 / 25 with the 25 being pre-existing
API-key/snapshot failures unrelated to this work.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
singlaamitesh
pushed a commit
to singlaamitesh/clawcodex
that referenced
this pull request
Jul 7, 2026
… hyperlinks + frame metrics + thinking widget + per-tool permission specialization + output styles frontmatter + placeholder Resume/Doctor screens Bundle of standalone module additions across ch13 phases 4-12 that share no integration surface with each other and don't touch ``app.py``, ``screens/repl.py``, ``agent_bridge.py``, or ``agent_loop.py``. Each module ships with unit tests; cross-cutting production wiring (where present) lives in a follow-up PR so this change merges cleanly into ``main`` regardless of order. Phase 4 vim foundation (gap agentforce314#4 wave-1+2): - ``src/tui/vim_buffer.py`` — multi-line ``VimBuffer`` + ``Cursor`` + ``Range`` - ``src/tui/vim_text_objects.py`` — ``find_text_object`` for ``iw``/``aw``/``ip``/``ap``/quotes/braces - ``src/tui/vim_operators.py`` — ``parse_operator_motion`` (``d2w``, ``ciw``, ``y$``); operator dispatch d/c/y/>/< - ``src/tui/vim_visual.py`` — Visual / Visual-Line / Visual-Block selection model - ``src/tui/vim_search.py`` — ``/`` / ``?`` regex search with ``n``/``N`` repeat - 70 tests (``test_vim_multiline.py`` + ``test_vim_wave2.py``) Phase 5 transcript search module (gap agentforce314#2): - ``src/tui/widgets/transcript_search.py`` — ``TranscriptSearch`` ModalScreen + ``find_matches`` helper - 16 tests (``test_transcript_search.py``) Phase 6 IME declared cursor module (gap agentforce314#3): - ``src/tui/declared_cursor.py`` — refcounted-set registry + CSI emission - 16 tests (``test_declared_cursor.py``) Phase 7 specialized permission dialogs (gap agentforce314#8): - ``src/tui/screens/permission_modal.py`` ``_TOOL_RENDERERS`` dispatcher + per-tool renderers (Bash, Edit, Write, Read) - 16 tests (``test_permission_modal_specialization.py``) Phase 8 placeholder Resume/Doctor screens (gap agentforce314#9): - ``src/tui/screens/resume_conversation.py`` — placeholder with empty-state until persistence wiring lands - ``src/tui/screens/doctor.py`` — read-only diagnostics (env/hyperlinks/frame-metrics/storage) - 6 tests (``test_resume_doctor_screens.py``) Phase 9 output styles frontmatter (gap agentforce314#7): - ``src/outputStyles/{loader,styles}.py`` — YAML frontmatter parsing via existing ``parse_frontmatter``; auto-discovery of ``~/.claude/outputStyles/`` - 11 tests (``test_output_styles_frontmatter.py``) + parity test (``test_output_styles_parity.py``) Phase 10 OSC 8 hyperlinks (gap agentforce314#15): - ``src/tui/hyperlinks.py`` — capability detection (FORCE_HYPERLINK / TERM_PROGRAM / VTE_VERSION / truecolor) - ``src/tui/widgets/messages/tool_result.py`` wraps file paths with the negative-lookbehind regex (avoids eating sentence punctuation) - 38 tests (``test_hyperlinks.py`` + ``test_tool_result_hyperlinks.py``) Phase 11 frame-event observability (gap agentforce314#10): - ``src/tui/frame_metrics.py`` — ``FrameEvent`` shape + ``register_frame_observer`` + no-op fast path when ``CLAWCODEX_DEBUG_REPAINTS`` unset - 17 tests (``test_frame_metrics.py``) Phase 12 assistant-thinking widget (gap agentforce314#16 sub-item): - ``src/tui/widgets/messages/assistant_thinking.py`` — italic-dim styled distinct row; redacted variant - ``src/tui/widgets/transcript_view.py`` adds ``append_thinking_chunk``/``append_thinking`` shells with symmetric guard for ``append_assistant_chunk`` (Critic-flagged: prevents future thinking↔assistant mis-routing once dispatch wiring lands) - 11 tests (``test_assistant_thinking.py`` + ``test_transcript_thinking_transitions.py``) Test surface: 521 cumulative tests pass (``tests/tui/`` + ``tests/parity/test_output_styles_parity.py``). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
singlaamitesh
pushed a commit
to singlaamitesh/clawcodex
that referenced
this pull request
Jul 7, 2026
…e-3-contextvars ch03 state agentforce314#3: SdkContext + run_with_sdk_context + Session migration
singlaamitesh
pushed a commit
to singlaamitesh/clawcodex
that referenced
this pull request
Jul 7, 2026
…tegration test Engine/runner/task polish (port-plan §10 remaining items, non-TUI): - agentforce314#7 Result delivery: enqueue_workflow_notification delivers a run's terminal result to the model via the shared <task-notification> queue (notified guard). - agentforce314#8 Run-file location: journals live under ~/.clawcodex/transcripts/workflows/ (per-user session storage) via get_workflow_run_path, not the project tree. - agentforce314#3 retry_workflow_agent: engine retry loop re-spawns a running agent (bounded); also fixes a latent bug where a single-agent skip propagated and aborted the run. - agentforce314#4 ProgressTracker fed to finalize_agent_tool for accurate token totals. - agentforce314#5 isolation="worktree": per-agent wf_<runId>-<idx> git worktrees, best-effort. - agentforce314#6 Live integration test (fake provider) driving LiveAgentRunner -> run_agent -> the real query loop. It caught two real bugs, fixed here: tool DISPATCH resolves by name from the registry (so schema agents need a per-call registry where StructuredOutput is the validating tool), and the injected StructuredOutput was permission-blocked in the subagent (now explicitly allowed). Full workflow suite: 144 passing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
singlaamitesh
pushed a commit
to singlaamitesh/clawcodex
that referenced
this pull request
Jul 7, 2026
agentforce314#599) All four of the book's remote systems (Bridge v1/v2, Direct Connect, Upstream Proxy) are already ported; only Direct Connect is wired/live, and wiring the rest needs claude.ai OAuth + CCR infrastructure the port can't exercise end-to-end. The one live, user-facing, cleanly-portable gap is the book's system agentforce314#3 -- "tunnel API traffic through infrastructure that might inject credentials or terminate TLS" -- in its every-user form. Enterprise deployments route Anthropic traffic through a gateway / corporate MITM proxy that requires injected auth headers. TS parses curl-style Name: Value from ANTHROPIC_CUSTOM_HEADERS into defaultHeaders (services/api/client.ts:530-554). Python applied it NOWHERE -- the var appeared only in a SAFE_ENV_KEYS allowlist (trust_boundary.py:75), never parsed. Every Anthropic client was built with no default_headers, so a user behind such a gateway had no way to supply the header and every call failed auth. Fix: - src/services/api/custom_headers.py (new): parse_custom_headers (curl-style, split on the FIRST colon, trim name+value, skip blank/colon-less/empty-name lines, CRLF-safe, last-duplicate-wins -- mirrors TS getCustomHeaders; a differential Node-vs-Python run over 18 adversarial inputs showed 0 divergences) + get_anthropic_custom_headers. - Inject default_headers at every first-party Anthropic construction: the provider (anthropic_provider._client_kwargs -- the load-bearing live inference path via query.py -> _ensure_client), the streaming call_model (claude.py), session_title, token_estimation, rename_command. Behind a gateway they ALL need the header, else those calls fail auth. - Non-Anthropic providers (minimax/openai/etc., which target their own base_url) are EXCLUDED -- matches TS (custom headers on the first-party client only). The SDK MERGES default_headers with its own auth headers (x-api-key/anthropic-version retained; verified against anthropic 0.88.0); a gateway overriding x-api-key/Authorization is the intended enterprise BYO-auth use (matching TS -- no override blocklist). Header values can't contain newlines (parser splits on them; httpx validates) -- no header-injection vector; source is the user's own allowlisted env var. Docs: my-docs/ch16-remote-round4-gap-analysis.md + plan + facet + critic verdict (APPROVE) under my-docs/port-improvement-round-4/. Tests: tests/test_ch16_custom_headers_round4.py (15): parse semantics (first-colon-split, trim, skip blank/colonless/empty-name, CRLF, none/empty, last-dup-wins), env reader, provider _client_kwargs injection (+ none-when-unset), the non-Anthropic (Minimax) exclusion, and the live call_model construction (AsyncAnthropic built with default_headers; None when unset). Full suite at the pre-existing 9-failure baseline. Deferred: --permission-prompt-tool (headless MCP approval; only the type exists); explicit HTTPS_PROXY/NO_PROXY (httpx trust_env already honors it); Bedrock/Vertex routing (unwired scaffolding); Gemini base_url nit; wiring the dormant CCR Bridge v1/v2 (needs claude.ai OAuth infra). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
singlaamitesh
pushed a commit
to singlaamitesh/clawcodex
that referenced
this pull request
Jul 7, 2026
…#601) The round-4 LLM memory-relevance recall (agentforce314#594) ships default-off because flipping it on is a latency+cost+correctness regression. This fixes the four blockers the round-4 critic flagged. Still default-off — zero behavior change on merge. - agentforce314#2 selector cost (provider-gated pin): the recall side-query ran on the SESSION model (an Opus turn paid full price every turn). TS pins it to a small default (getDefaultSonnetModel). The port wires the small_fast_model setting, but its shipped default is an Anthropic id (claude-3-5-haiku-20241022) valid only on the first-party Anthropic endpoint. So _resolve_recall_model(provider) applies the pin ONLY when the session provider is a first-party AnthropicProvider; Minimax (Anthropic SDK, own endpoint), DeepSeek, OpenAI, OpenRouter fall back to the session model. Without the gate the Anthropic default id 400s on a non-Anthropic endpoint and — since recall swallows errors — silently kills recall every turn (critic M1). small_fast_model_provider pairing for non-Anthropic cheap-recall is deferred. - agentforce314#3 de-dup reset on compaction: the recall reminder is ephemeral but _memory_surfaced was monotonic — once surfaced, a memory was never re-surfaced. After /compact drops the earlier context, the memory stayed suppressed. _do_compact now clears _memory_surfaced on success (idle-gated — no race with the turn worker). - agentforce314#4 aggregate turn byte-cap: per-file was 4 KB but 5 selections could inject ~20 KB/turn. build_relevant_memory_reminder_with_paths caps at MAX_TOTAL_SURFACE_CHARS=12 KB and returns the paths ACTUALLY surfaced; get_relevant_memory_reminder de-dups only those, so a cap-trimmed memory stays eligible (the old mark-all was a latent suppress-forever bug the cap would have activated). Back-compat string wrapper retained. - agentforce314#5 docstring: _maybe_recall_memories said "prepend"; the code appends after the user turn (correct — normalize_messages merges consecutive user messages). Fixed. Deferred agentforce314#1 (synchronous side-query): the port has only this one prefetch, so there is no concurrent prefetch to overlap without a larger prompt-submit refactor — one small-model call per turn (fast Haiku on the default path). Documented. Docs: my-docs/port-improvement-round-5/r5-1-ch11-memory-enable-blockers.md + critic verdicts (REVISE->APPROVE). Tests: tests/test_r5_ch11_memory_enable_blockers.py (8) incl. the M1 regression guard (non-Anthropic never pins even when the Anthropic-default id is set). Existing memory suite (466) green via the back-compat wrapper. Full suite at the pre-existing baseline. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 21, 2026
#599) All four of the book's remote systems (Bridge v1/v2, Direct Connect, Upstream Proxy) are already ported; only Direct Connect is wired/live, and wiring the rest needs claude.ai OAuth + CCR infrastructure the port can't exercise end-to-end. The one live, user-facing, cleanly-portable gap is the book's system #3 -- "tunnel API traffic through infrastructure that might inject credentials or terminate TLS" -- in its every-user form. Enterprise deployments route Anthropic traffic through a gateway / corporate MITM proxy that requires injected auth headers. TS parses curl-style Name: Value from ANTHROPIC_CUSTOM_HEADERS into defaultHeaders (services/api/client.ts:530-554). Python applied it NOWHERE -- the var appeared only in a SAFE_ENV_KEYS allowlist (trust_boundary.py:75), never parsed. Every Anthropic client was built with no default_headers, so a user behind such a gateway had no way to supply the header and every call failed auth. Fix: - src/services/api/custom_headers.py (new): parse_custom_headers (curl-style, split on the FIRST colon, trim name+value, skip blank/colon-less/empty-name lines, CRLF-safe, last-duplicate-wins -- mirrors TS getCustomHeaders; a differential Node-vs-Python run over 18 adversarial inputs showed 0 divergences) + get_anthropic_custom_headers. - Inject default_headers at every first-party Anthropic construction: the provider (anthropic_provider._client_kwargs -- the load-bearing live inference path via query.py -> _ensure_client), the streaming call_model (claude.py), session_title, token_estimation, rename_command. Behind a gateway they ALL need the header, else those calls fail auth. - Non-Anthropic providers (minimax/openai/etc., which target their own base_url) are EXCLUDED -- matches TS (custom headers on the first-party client only). The SDK MERGES default_headers with its own auth headers (x-api-key/anthropic-version retained; verified against anthropic 0.88.0); a gateway overriding x-api-key/Authorization is the intended enterprise BYO-auth use (matching TS -- no override blocklist). Header values can't contain newlines (parser splits on them; httpx validates) -- no header-injection vector; source is the user's own allowlisted env var. Docs: my-docs/ch16-remote-round4-gap-analysis.md + plan + facet + critic verdict (APPROVE) under my-docs/port-improvement-round-4/. Tests: tests/test_ch16_custom_headers_round4.py (15): parse semantics (first-colon-split, trim, skip blank/colonless/empty-name, CRLF, none/empty, last-dup-wins), env reader, provider _client_kwargs injection (+ none-when-unset), the non-Anthropic (Minimax) exclusion, and the live call_model construction (AsyncAnthropic built with default_headers; None when unset). Full suite at the pre-existing 9-failure baseline. Deferred: --permission-prompt-tool (headless MCP approval; only the type exists); explicit HTTPS_PROXY/NO_PROXY (httpx trust_env already honors it); Bedrock/Vertex routing (unwired scaffolding); Gemini base_url nit; wiring the dormant CCR Bridge v1/v2 (needs claude.ai OAuth infra). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 21, 2026
The round-4 LLM memory-relevance recall (#594) ships default-off because flipping it on is a latency+cost+correctness regression. This fixes the four blockers the round-4 critic flagged. Still default-off — zero behavior change on merge. - #2 selector cost (provider-gated pin): the recall side-query ran on the SESSION model (an Opus turn paid full price every turn). TS pins it to a small default (getDefaultSonnetModel). The port wires the small_fast_model setting, but its shipped default is an Anthropic id (claude-3-5-haiku-20241022) valid only on the first-party Anthropic endpoint. So _resolve_recall_model(provider) applies the pin ONLY when the session provider is a first-party AnthropicProvider; Minimax (Anthropic SDK, own endpoint), DeepSeek, OpenAI, OpenRouter fall back to the session model. Without the gate the Anthropic default id 400s on a non-Anthropic endpoint and — since recall swallows errors — silently kills recall every turn (critic M1). small_fast_model_provider pairing for non-Anthropic cheap-recall is deferred. - #3 de-dup reset on compaction: the recall reminder is ephemeral but _memory_surfaced was monotonic — once surfaced, a memory was never re-surfaced. After /compact drops the earlier context, the memory stayed suppressed. _do_compact now clears _memory_surfaced on success (idle-gated — no race with the turn worker). - #4 aggregate turn byte-cap: per-file was 4 KB but 5 selections could inject ~20 KB/turn. build_relevant_memory_reminder_with_paths caps at MAX_TOTAL_SURFACE_CHARS=12 KB and returns the paths ACTUALLY surfaced; get_relevant_memory_reminder de-dups only those, so a cap-trimmed memory stays eligible (the old mark-all was a latent suppress-forever bug the cap would have activated). Back-compat string wrapper retained. - #5 docstring: _maybe_recall_memories said "prepend"; the code appends after the user turn (correct — normalize_messages merges consecutive user messages). Fixed. Deferred #1 (synchronous side-query): the port has only this one prefetch, so there is no concurrent prefetch to overlap without a larger prompt-submit refactor — one small-model call per turn (fast Haiku on the default path). Documented. Docs: my-docs/port-improvement-round-5/r5-1-ch11-memory-enable-blockers.md + critic verdicts (REVISE->APPROVE). Tests: tests/test_r5_ch11_memory_enable_blockers.py (8) incl. the M1 regression guard (non-Anthropic never pins even when the Anthropic-default id is set). Existing memory suite (466) green via the back-compat wrapper. Full suite at the pre-existing baseline. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 21, 2026
…S-2) (#645) * feat(services): autoFix config + hook + runner (SERVICES-2 W1-W3; wiring next) Port of typescript/src/services/autoFix/ — the settings-opt-in feature that runs the user's lint/test command after a file edit and injects <auto_fix_feedback> so the model self-fixes. Python had only the /autofix slash shell; the runtime was absent. - config.py (autoFixConfig.ts): get_auto_fix_config(raw) → AutoFixConfig|None — dict + enabled + at-least-one-of-lint/test (the zod .refine) + numeric bounds (maxRetries 0..10 default 3, timeout 1000..300000 default 30000); None (safeParse-null posture) on any failure. load_auto_fix_config reads settings.extra["autoFix"] (verified: no typed field — lands in extra, like the plugins round's enabledPlugins). - hook.py (autoFixHook.ts): AUTO_FIX_TOOLS = the port's file-mutation registry names (Write/Edit/MultiEdit/NotebookEdit, the file_edit/file_write analog); should_run_auto_fix; build_auto_fix_context (the <auto_fix_feedback> block VERBATIM) + build_max_retries_context (verbatim). - runner.py (autoFixRunner.ts): run_auto_fix_check — lint first, test only if lint passed; each an asyncio.create_subprocess_shell bounded by timeout, killing the process GROUP on timeout (start_new_session + os.killpg, the detached/killTree analog) and reaping to avoid zombies; _build_error_summary verbatim; never-raises (spawn failure → has_errors=False). Verified: config parse/refine/bounds, hook fire + verbatim context, runner lint-fail-skips-test / lint-pass-test-fail / both-clean / timeout-kill (sleep 30 @ 800ms → returns 0.81s, group killed, timed_out) / abort. REMAINING: W4 wiring — a NEW sibling run_auto_fix_step at the orchestrator (tool_execution.py, after run_post_tool_use_hooks), NOT inside that function (it early-returns when no user PostToolUse hooks configured — the teammate-hooks split-gate class; autoFix must run regardless). Retry cap by query_tracking.chain_id. Then tests + suite. Awaiting autofix-critic on the plan's W4 placement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(services): autoFix wiring + all critic findings (SERVICES-2 complete) The autofix-critic caught that TS autoFix is DEAD CODE — AUTO_FIX_TOOLS = {'file_edit','file_write'} never matches the real tool names (Edit/Write, FileEditTool/constants.ts:2 FILE_EDIT_TOOL_NAME='Edit'), verified first-hand. So this port activates the author's INTENT ({Edit,Write}) as a DOCUMENTED, opt-in-gated DIVERGENCE (fixes the reference's dead tool-name set; inert unless a user writes settings.autoFix.enabled). All findings: - B1 (split-gate): run_auto_fix_step is a NEW SIBLING at the orchestrator (tool_execution.py, after run_post_tool_use_hooks) — NOT inside that function, which early-returns when no user PostToolUse hook is configured (the common autoFix case). Pinned by test_fires_with_zero_posttool_hooks. - B2 (accessor): config via get_settings().extra.get("autoFix") (ToolContext has no settings; global read = the per-process equivalent). - M1 (reject-not-clamp): out-of-range/non-int → whole config None (zod .min/.max rejects); .default only when key absent. - M2 (camelCase): reads raw["maxRetries"]/raw["timeout"]. - M3 (reset-on-success): the retry counter is DELETED on a clean run (toolHooks.ts:247), not just incremented on error. - M4 (tool set): {"Edit","Write"} — the author's intent, not the 4-element _FILE_EDIT_TOOLS (NotebookEdit excluded, MultiEdit is a phantom). - M5 (None-guard): chain key handles query_tracking=None → "default". - M6 (10k cap): each stream sliced to 10 000 before combine. - minors: SIGTERM (not SIGKILL) + reap + ProcessLookupError race; mid-flight abort (kills the group, degrades to no-errors); cwd from tool_use_context; the post-hook attachment yield shape (list-wrapped content). tests/services/test_autofix.py (26): config parse/refine/reject/camelCase, hook fire + verbatim contexts + {Edit,Write} intent, runner (lint-skips-test / test-fail / clean / timeout-SIGTERM-kill / 10k cap / pre-set abort / mid-flight abort / no-commands), step (ZERO-hooks regression, non-file skip, retry cap, reset-on-clean, None-query-tracking). Full suite at the 6-failure baseline (7857 passed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(services): autoFix config zod-strictness (critic minor #1) enabled must be a strict bool (a string "false" is truthy in Python — must not turn autoFix ON against intent); a bad-type lint/test rejects the whole config → None, matching zod z.boolean()/z.string() safeParse (not the prior silent coerce/drop). 3 new tests; consistent with the already-strict int path. (Minors #2 retry-map-lifecycle and #3 proxy-coverage accepted as faithful-to-TS / adequate.) Full suite at the 6-failure baseline (7860). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 21, 2026
…C8) (#658) * feat(sandbox): honest unsandboxed guard — settings.sandbox no longer silent (C8) The port has NO sandbox enforcement (TS wraps the external @anthropic-ai/sandbox-runtime) AND SettingsSchema had no `sandbox` field — so a user's settings.sandbox was silently DROPPED at parse AND unenforced: a false sense of security. This is the utils/-round deferred "Sandbox chapter"'s security-meaningful MINIMUM (enforcement itself remains deferred). The guard maps the port's permanently-unavailable sandbox onto TS's OWN documented sandbox-unavailable path (entrypoints/sandboxTypes.ts:96-103): when sandbox.enabled is true but the sandbox cannot start, - failIfUnavailable=true → REFUSE (managed-settings hard gate — never silently run unsandboxed), - failIfUnavailable=false → "a warning is shown and commands run unsandboxed". - SettingsSchema.sandbox: new SandboxSettings dataclass (enabled, failIfUnavailable, autoAllowBashIfSandboxed, allowUnsandboxedCommands, excludedCommands); from_dict accepts camelCase + snake and IGNORES the enforcement-only passthrough keys (network/filesystem/ripgrep/…) so a valid settings.json never fails to load. - permissions/sandbox_guard.py: sandbox_hard_gate_error / sandbox_unsandboxed_ warning / warn_if_unsandboxed_once. - _bash_call: hard-gate → ToolPermissionError refusal; else warn-once + proceed (best-effort, never crashes bash). validate_settings surfaces the hard gate at load too. NON-goal (deferred): native sandbox-exec/bwrap enforcement — the sandbox-native-enforcement sub-chapter (needs critic scoping: macOS sandbox-exec is built-in/no-dep and may be in reach). tests/test_sandbox_guard.py (12): camelCase parse + unknown-key passthrough, the 3-way guard mapping, validate_settings hard-gate-errors vs warning-loads, warn-once, and _bash_call refuses under hard gate / runs+warns otherwise. 134 settings/bash tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs+test(sandbox): C8 scope — bash-tool-scoped (fg+bg), matches TS BashTool Pre-empting the critic's false-hard-gate probe: the guard is BashTool-scoped (both foreground + background bash share _bash_call, guard runs before the run_in_background branch), matching TS where shouldUseSandbox is BashTool-scoped (called only from bashPermissions.ts). Hook subprocesses + MCP stdio are user-authored config TS's sandbox doesn't wrap either → intentionally out of scope. The hard gate is an honest "no unsandboxed BASH", not a false "nothing runs unsandboxed". Added the scope note to the module + a background-bash hard-gate refusal test (13 total). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(sandbox): C8 — hard gate is a REFUSE-TO-START, not per-bash (critic MAJOR) The first cut refused per-_bash_call, but TS's failIfUnavailable is a refuse-to-START at the entrypoints (print.ts:600 / REPL.tsx:2362 "refusing to start without a working sandbox"). So under sandbox.enabled+failIfUnavailable the port STARTED and ran /bg (background/tasks.py Popen, OUTSIDE _bash_call), MCP servers, and hooks UNSANDBOXED while _bash_call refused the same command — a "hard gate" that /bg walked through, contradicting the chapter's no-false- assurance thesis. - Refuse-to-start in agent_server._build_runtime: sandbox_hard_gate_error → sess.init_error → the session refuses to start (the port's existing init-error mechanism), so /bg + MCP + hooks never run. The per-_bash_call guard stays as a CLI-path backstop. - enabledPlatforms (critic minor #3): now parsed + honored — on a platform not in the list, sandbox is disabled (no gate/warning), matching TS (sandboxTypes.ts:104); the port was more aggressive (would refuse where TS runs). - sandbox_guard docstring split into the two mechanisms (BashTool-scoped WARNING vs refuse-to-start HARD GATE). Tests +6 (→19): _build_runtime refuses under hard gate / warning-only starts; skill-slash !-shell fails closed (minor #4); enabledPlatforms off-platform disables / on-platform gates / empty=all. Enforcement scoping (critic-endorsed): guard + refuse-to-start is the MVP; native sandbox-exec/bwrap is a separate scoped chapter (a half-working sandbox is worse than an honest refusal — the chapter's own thesis). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(sandbox): C8 re-review 2 — close the /bg control-path leak The refuse-to-start (v2) closed MCP + hooks (both in _build_runtime), but the critic found /bg + /bg-agent are CONTROL requests, not turns: send_to_agent → _handle_control_request → _do_bgtask → subprocess.Popen(shell=True). That path bypasses _build_runtime AND the turn-path init_error checks, and the session stays fully alive after init_error (sess.start() + the returned handle are unconditional). So `/bg foo` still ran UNSANDBOXED under a hard-gate config the session was supposed to refuse — the same MAJOR, on the one path v2 missed. Fix: guard the top of _handle_control_request — a session with init_error set rejects every control request (replying the gate error) EXCEPT `interrupt` (a benign abort of a non-existent turn). Mirrors the "session not ready" pattern the permission-mode handlers already use. Now /bg + /bg-agent + MCP + hooks all refuse; the docstring's "nothing runs, not just Bash" is truthful. Test replaced (the old one asserted tool_registry is None — the intermediate, not the property): test_bg_run_control_request_refused_under_init_error drives a real bg_run control request against an init_error session and asserts NO subprocess (Popen + _do_bgtask both tripwired to raise) + the gate-error reply; test_interrupt_still_works_under_init_error pins the exemption. 21 sandbox tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 21, 2026
* feat(bash): notify on background-bash completion (C5 Part 1 — enqueueShellNotification) TS notifies when a run_in_background bash task reaches a terminal state (utils/task/framework.ts:289 enqueuePendingNotification; BashTool/prompt.ts: 285-287 "you will be notified when it completes — do not poll"). The port marked the terminal state notified=True WITHOUT sending — the documented forward-trap in background.py (ch10 WI-2), so a bg bash finishing while the model was away went unannounced (silently polled via TaskOutput instead). - task_notification.py: enqueue_shell_notification — mirrors enqueue_agent_notification (the atomic check-and-set duplicate-delivery guard + build_task_notification_xml + enqueue_pending_notification) but gated on LocalShellTaskState instead of LocalAgentTaskState. - background.py: REMOVED the forward-trap notified=True from the terminal replace() (per its own comment's instruction), and call enqueue_shell_notification after the terminal update. The eviction sweeper's notify-before-evict guard (eviction.py:97) now correctly keeps the entry until the notification is delivered, then reclaims it — instead of the old mark-notified-without-sending. Best-effort (never breaks reaping); duplicate-safe (a TaskStop that already notified makes it a no-op). tests/test_shell_completion_notification.py (3): enqueues + marks notified on terminal, duplicate-safe (already-notified → no second), wrong-task-type no-op. 44 background/eviction/notification tests green (the notified=False terminal state is compatible with the eviction guard). Part 2 (the Monitor tool + backpressure) follows separately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(bash): C5 Part 1 — fallback-mark-notified on enqueue failure (no leak) Pre-empting the eviction-leak risk: enqueue_shell_notification is best-effort (except: pass), but with the forward-trap removed the terminal state is notified=False — so if the enqueue raised, the sweeper (which keeps un-notified terminal tasks, eviction.py:97) would leak the task in /tasks forever. Added a fallback: on enqueue failure, force notified=True so the task stays evictable, degrading to the OLD evictable-but-unsent behavior rather than a leak (still strictly better than the pre-C5 silent-drop). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(bash): C5 Part 1 critic — shell-specific summary, XML escaping, kill path The c5p1-critic found the model-facing payload was wrong (the mechanism was right). Real TS analog: enqueueShellNotification (LocalShellTask.tsx:105-172). #1 BLOCKING — wrong summary: reused the AGENT builder, so every bg-bash completion was announced as `Agent "..." completed`, the exit code was dropped, and the failed case said "Unknown error". Added build_shell_notification_xml + _build_shell_summary + BACKGROUND_BASH_SUMMARY_PREFIX ("Background command ") — `Background command "<desc>" completed (exit code N)` / `... failed with exit code N` / `... was stopped`, exit code included only when known. Threaded exit_code + tool_use_id through enqueue_shell_notification ← background.py's rc. #2 MAJOR — no XML escaping: a bg command routinely has < > & (and _reap uses `description or command`), so a raw summary produced a MALFORMED envelope. The shell builder XML-escapes the summary (TS LocalShellTask.tsx:164 escapeXml); the agent builder stays raw (LocalAgentTask.tsx:246-256 is raw — that claim was only true for agents). #3 MAJOR — kill path: TS killTask (killShellTasks.ts:38-44) atomically sets status='killed' + notified=True, which SUPPRESSES the reaper notification (no notification on a user kill). The port sent SIGTERM only → the reaper saw rc≠0 → sent a spurious `failed`. stop_background_bash now sets status='killed' + notified=True; _patch preserves a 'killed' status (doesn't reclassify from the SIGTERM exit code). The duplicate-guard's "TaskStop did it" premise is now actually wired. #4 tests — the old tests never asserted summary CONTENT (passed with the wrong "Agent" text). Added: full-summary snapshots (prefix + exit code + not-"Agent"), metachar XML-escaping, a REAL spawn→reap integration (exit 3 → the "failed with exit code 3" notification delivered), and kill-suppression (kill → status=killed + notified + NO notification). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(bash): C5 Part 1 #3 — mark killed in LocalShellTask.kill before the signal The c5p1-critic reproduced the spurious kill notification 8/8 through the REAL path (TaskStop tool → stop_task → impl.kill = LocalShellTask.kill). My prior mark lived only in stop_background_bash, which stop_task doesn't call — so nothing marked killed/notified before the reaper, which sent "failed with exit code -15". Fix (the critic's exact direction): move the status='killed' + notified=True mutation into LocalShellTask.kill, BEFORE os.killpg, gated on the process still being alive (TS killTask's status==='running' gate, killShellTasks.ts:38-44). This is the production kill path (TaskStop → stop_task → this), so the mark lives here; marking BEFORE the signal closes the reaper-vs-killer race (the reaper is blocked in proc.wait() and can't wake until the signal lands, by which point notified=True is committed → the reaper's enqueue no-ops). The stop_background_bash mark stays as defense-in-depth, also moved before its killpg. _patch preserves a 'killed' status. Test rewritten to drive the REAL path: test_stop_task_kill_suppresses_notification runs stop_task (what TaskStop uses) on a real bg task → asserts status=killed + notified + NO notification (the critic's repro, now 0). + a direct stop_background_bash defense-in-depth test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 21, 2026
… (C5 Part 2) (#665) * feat(tools): the Monitor tool — stream shell output with backpressure (C5 Part 2) Port of MonitorTool.ts (MONITOR_TOOL=true, build.ts:43): run a shell command in the background and STREAM its stdout to the model as ~1s <monitor-output> notifications, instead of the single completion notification the Bash run_in_background path delivers (C5 Part 1). Completes the Monitor-tool chapter. - src/tool_system/tools/monitor.py: the tool (spawns via spawn_background_bash, same detached shell + reaper) + a per-monitor polling thread (_stream_output) that tails the output file, emits each new batch of WHOLE lines (a trailing partial is held for the next poll) as an XML-escaped <monitor-output> notification, and stops when the task leaves 'running' / the 30-min deadline passes. - BACKPRESSURE (the tools-round critic's requirement — the first attempt was DEFERRED because it lacked this; an unbounded poller floods the conversation): after _MONITOR_MAX_NOTIFICATIONS (100) streamed notifications the monitor AUTO-STOPS — kills the shell + sends a final "auto-stopped … too many events … re-run with a tighter filter" notice. TS: "Monitors that produce too many events are automatically stopped." Registered in ALL_STATIC_TOOLS; gated like Bash (check_permissions=_bash_check_permissions); Monitor added to the getAllBaseTools parity snapshot. tests/tools/test_monitor_tool.py (8): registration, streaming (new lines → notifications, partial-line held, metachars escaped), THE backpressure (auto-stops after the cap: kill + final notice / no auto-stop under cap), and lifecycle (stops when the task leaves running / is evicted). 161 tool tests green (only the 2 pre-existing tool_parity default-property baseline failures). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(tools): C5-P2 pre-critic — Monitor safety guard + per-notification size cap Pre-addressing the c5p2-critic's two highest-risk probes: #5 (Monitor bypasses bash safety) — Monitor spawns via spawn_background_bash DIRECTLY, so it was a way AROUND the hardcoded-dangerous-pattern block + the C8 sandbox hard-gate that live in _bash_call (check_permissions only covers the permission RULES, not those hard gates below the permission layer). Extracted bash_command_safety_guard(command) (the shared dangerous-pattern + sandbox hard-gate check) and call it from BOTH _bash_call and _monitor_call so they can't drift and Monitor can't skip them. Tests: `rm -rf /` refused through Monitor; a sandbox hard-gate refused through Monitor. #1 (backpressure bounds count, not bytes) — one poll batches all new lines into ONE notification, so the count cap (100) doesn't bound a FIREHOSE (few-but-huge notifications). Added _MONITOR_MAX_NOTIFICATION_BYTES (8KB): each notification keeps the TAIL + a "[N earlier bytes truncated]" marker, so a single notification can't blow the conversation. Test: a 5000-byte burst → truncated, tail kept. 332 bash/monitor/sandbox tests green (the bash_command_safety_guard extraction didn't regress _bash_call). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(tools): C5-P2 critic — render-compatible envelope, bounded read, delivery framing Addresses the c5p2-critic's remaining findings (it reviewed the pre-67843e1 commit; #5 guard-bypass + the #1 per-notification cap were already fixed there — this closes the rest). MAJOR (envelope/delivery incompatible with the render path): the invented <monitor-output task=".."> envelope broke the drain path — parse_task_id found no <task-id> child (→ None, correlation lost), render_banner mis-rendered ("Background task finished"), and build_notification_turn framed a RUNNING monitor as "finished" every drain. Fix: _monitor_notification_xml emits a real <task-notification> (<task-id>/<output-file>/<status>running</status>/<summary>) that parse_task_id + render_banner understand; build_notification_turn is now status-aware — a batch that is ENTIRELY status=running gets a streaming preamble ("STILL RUNNING — do not report as finished"), while any finished task keeps the completion preamble. So a live monitor surfaces as a streaming update, not a false completion turn. #1 (bounded read — the OOM vector the per-notification cap didn't close): _drain did read_bytes() on the WHOLE file, so a firehose loads everything into memory before truncating. Now seeks to the offset and reads at most _MONITOR_MAX_READ_BYTES (8 MiB, TS diskOutput DEFAULT_MAX_READ_BYTES), advancing pos by bytes read — bounds memory AND fixes the minor UTF-8 rewind drift (offset arithmetic is pure bytes now). Minors: is_concurrency_safe True (TS MonitorTool.ts:54, fire-and-forget) + max_result_size_chars 10_000 (TS:51). tests/tools/test_monitor_tool.py (15): render-path compat (parse_task_id correlates, banner surfaces the line, pure-running → streaming preamble, finished → completion preamble) + bounded-read/size/backpressure/safety-guard. 452 bash/monitor/sandbox/notification/server tests green (the shared build_notification_turn change is additive — finished tasks unaffected). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(tools): C5-P2 #3 minors — final-drain flush + long-line progress Closes the c5p2-critic's two remaining #3 MINORs (non-gating, but cheap): #3.ii — a command whose last output has no trailing newline (printf done) lost that line: the terminal drain now runs with final=True and emits the trailing partial. (The mid-stream drain still holds a partial for the next poll — a line-in-progress shouldn't be split.) #3.i — a single line LONGER than the 8MiB read window had no newline in any window, so _drain rewound and re-read the same bytes every poll forever (bounded, sleep-gated, but a silent stall). Now: when the read window is FULL and has no newline, emit what we have so the stream keeps moving. Tests: partial-held-while-running-then-flushed-on-completion; a 500-byte single line over a tiny read window still makes progress. 15 monitor tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * polish(tools): C5-P2 APPROVE nits — terminal auto-stop status + divergence note The c5p2-critic APPROVED; applying its cheap ask + tidiest minor (both non-gating): - ASK: added a comment at the poller noting the DELIBERATE divergence from TS — the port drives an internal turn per drain, whereas TS injects monitor deltas as passive attachments on the next natural turn. Accepted as port scope (building the attachment pipeline for one tool is disproportionate); the cost is bounded (≤ cap then auto-stop, zero for quiet monitors). Marked so a future maintainer doesn't "fix" it by accident. - minor #3: the auto-stop notice reused status="running", so a TERMINAL "monitor stopped" event got the "STILL RUNNING" streaming preamble. _monitor_notification_xml now takes a status param; the auto-stop notice passes status="killed" → it gets the completion framing. Test asserts the killed status + no STILL-RUNNING preamble. Deferred (critic's other non-gating minors): mixed running+finished batch framing and the production banner showing status-not-line — both cosmetic, self-correcting, follow-up-able. 15 monitor tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Jul 21, 2026
…ult) (#666) * feat(proxy): wire the CCR upstream proxy (C9 — env-gated, no-op by default) The port ported upstream_proxy.py (init_upstream_proxy + get_upstream_proxy_env, gated on CLAUDE_CODE_REMOTE) but left it UNWIRED — zero live callers, and subprocess_env carried a TODO ("when it lands, merge get_upstream_proxy_env"). This wires it, mirroring TS's registerUpstreamProxyEnvFn indirection. - src/utils/subprocess_env.py: register_upstream_proxy_env_fn(fn) + a module provider slot. subprocess_env merges the provider's recipe AFTER the scrub (so an injected HTTPS_PROXY / *_CA_BUNDLE isn't stripped by the anti-exfiltration pass), best-effort (a proxy-env failure can't break child spawning). The indirection keeps this module from statically importing the upstreamproxy package (asyncio/ssl/relay). - src/entrypoints/agent_server_cli.py: _maybe_init_upstream_proxy() at _serve start — when CLAUDE_CODE_REMOTE is truthy, register the provider + await init_upstream_proxy(); FAIL-OPEN (a remote session degrades to direct rather than the server dying). No-op otherwise. DEFAULT BEHAVIOUR UNCHANGED: with CLAUDE_CODE_REMOTE unset (every local build, and the open TS build strips this path entirely), the provider is never registered and subprocess_env is byte-for-byte the base. Low blast radius — the value is that a CCR-remote deployment's ported proxy actually engages instead of being dead code. Scope: the WS mTLS/CA-context and relay.stop()-cleanup pieces from the plan live inside the dormant relay itself (no open-build reference, untestable without CCR infra) — deferred; this lands the reachable, testable wiring. tests/test_c9_upstream_proxy_wiring.py (6): default no-op (env unchanged / empty-provider no-op), proxy-var merge, survives-the-scrub, fail-open, and the entrypoint guard is a no-op without the env. 89 subprocess/proxy tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(proxy): C9 #3 — register the proxy provider only AFTER init succeeds Pre-addressing the c9-critic's highest-risk probe: _maybe_init_upstream_proxy registered the env provider BEFORE await init_upstream_proxy(), so a failed init (fail-open) could leave the provider registered. Reordered to init-first, register-only-on-success — a failed init now leaves ZERO registration, so subprocess_env stays a pure no-op. (Defense-in-depth: get_upstream_proxy_env already returns {} when the proxy state is DISABLED with a clean env, verified.) Tests: a failing init leaves _upstream_proxy_env_fn is None (no half-registered provider); the disabled state returns an empty recipe. 8 C9 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(proxy): C9 critic — scrub-authoritative order + register-first (TS parity) The c9-critic corrected two real errors (and disproved my #4 premise: I'd grepped typescript/ INSIDE the worktree where it doesn't exist — the open build HAS this wiring at init.ts:148-149, 30 files reference CLAUDE_CODE_REMOTE. So C9 is genuine restored parity, not overreach — ship it). MAJOR-A [subprocess_env.py] — scrub/merge order was INVERTED vs TS. TS (subprocessEnv.ts:91-97) merges the proxy recipe THEN runs the scrub loop, so the SCRUB is authoritative/last — no provider can re-introduce a scrubbed secret. The port scrubbed first then merged, so a provider returning a scrub-set key would SURVIVE into the child (a defense-in-depth hole in the exact anti-exfiltration control), and the comment falsely claimed "TS order". Fixed: merge proxy first, THEN scrub — exact TS order; proxy keys aren't in the scrub set so they still survive. New test proves a provider returning ANTHROPIC_API_KEY is scrubbed while HTTPS_PROXY survives. MAJOR-B [agent_server_cli.py] — my earlier #3 "fix" (init-first) diverged from TS register-first (init.ts:148-149) on a FALSE premise: the critic proved a provider over a disabled/failed init returns {} on a clean env (safe in EITHER order). Reverted to register-first for exact TS parity; dropped the false "injects bad vars" rationale. Test updated to assert the real safe property (a failed init injects nothing because the disabled state returns {}). Also (critic notes): wired _serve_stdio too (TS inits in shared init.ts for ALL sessions, not just HTTP — a CCR-remote stdio container would otherwise miss the proxy); fixed the upstream_proxy docstring (9-var→8-var, dropped the nonexistent lowercase `proxy` claim). 9 C9 tests green (default no-op, proxy-merge, scrub-authoritative, fail-open, disabled-returns-empty, entrypoint-guard). subprocess/proxy suite green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <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
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.
Summary
OpenAI (and compatible APIs) reject tool definitions when a JSON Schema array omits items. The AskUserQuestion tool declared options as
{ "type": "array" }without items, which produced400 invalid_function_parameterswith a message like "array schema missing items" undertools[0].function.parameters.Changes
label(required) anddescription(optional), aligned with how the handler normalizes option objects.options: array of objects withlabel(required),descriptionand optionalpreviewmultiSelect: optional boolean on each question (used bysrc/repl/core.py_ask_user_questions)Testing
AskUserQuestion.Prompt to reproduce
The prompt used to reproduce the error was:
Please generate a to-do app with the above requirements(Using the provided Task CLI requirements block.)