Skip to content

fix(pi-shell): unblock bash heredocs >4 KiB on Windows and >64 KiB on macOS - #2

Closed
oldschoola wants to merge 1451 commits into
mainfrom
fix/heredoc-deadlock-windows-macos
Closed

fix(pi-shell): unblock bash heredocs >4 KiB on Windows and >64 KiB on macOS#2
oldschoola wants to merge 1451 commits into
mainfrom
fix/heredoc-deadlock-windows-macos

Conversation

@oldschoola

Copy link
Copy Markdown
Owner

Symptom

The bash tool times out after 305 s when the agent sends a heredoc-style command such as:

mkdir -p .agent-context
cat > .agent-context/merge-context.md << 'CONTEXT'
... ~4 KB of literal body ...
CONTEXT
ls -la .agent-context/

The hard timeout text is the one emitted by bash_executor.ts:214: Command exceeded hard timeout after <N> seconds. Nothing about the user's command is malformed — the brush interpreter deadlocks before cat/ls ever runs.

Root cause

brush_core::interp::setup_open_file_with_contents wires heredocs and here-strings to a process by:

  1. creating an anonymous pipe via std::io::pipe(),
  2. synchronously writing the whole body into the writer half on the calling thread,
  3. handing the reader half to the next command as its stdin.

The reader is not consumed until after the function returns and the command spawns, so step (2) is bounded by the OS pipe buffer:

Platform Pipe buffer Workaround in current code
Linux 64 KiB default, growable via F_SETPIPE_SZ up to /proc/sys/fs/pipe-max-size (1 MiB) F_SETPIPE_SZ(len) before the write
macOS 16–64 KiB, no F_SETPIPE_SZ equivalent none
Windows CreatePipe(0) ≈ 4 KiB none

Any heredoc bigger than the platform buffer (~4 KiB on Windows, 16–64 KiB on macOS) fills the buffer mid-write, writer.write_all blocks forever, and the 305 s hard-timeout in pi-shell finally trips. Confirmed present in upstream reubeno/brush@main.

Fix

In the vendored setup_open_file_with_contents, keep the Linux F_SETPIPE_SZ fast path for the common in-process case, but fall through to a detached writer thread on every other platform (and on Linux when the kernel rejects the requested size — body > pipe-max-size). The thread owns the writer and terminates naturally on drain or BrokenPipe. No JoinHandle is kept — it's fire-and-forget, same shape dash uses for large heredocs.

A LOCAL DIVERGENCE (vs upstream reubeno/brush@main) comment block documents the fork.

Tests

  • New cross-platform regression large_heredoc_does_not_deadlock in pi-shell::shell::tests writes a 256 KiB body through the : builtin (never drains stdin) and asserts execute_shell finishes in ≤10 s with exit code 0. Pre-fix, the test panics with execute_shell hung past 10 s — heredoc writer deadlocked (verified locally by stashing the fix).
  • Full cargo test -p pi-shell (184 tests) green.
  • End-to-end smoke through the rebuilt napi binding: the user's exact heredoc shape (mkdir + cat > .agent-context/merge-context.md << 'CONTEXT' … CONTEXT + ls) with a 12,889-byte body completes in 178 ms, exit 0, file written intact. Pre-fix the same command hangs to 305 s.
  • cargo clippy -p brush-core --lib / cargo clippy -p pi-shell --all-targets — no new warnings (5 pre-existing in process.rs).

Files

  • crates/brush-core-vendored/src/interp.rs — rewrite setup_open_file_with_contents; document the divergence.
  • crates/pi-shell/src/shell.rs — add large_heredoc_does_not_deadlock.
  • packages/natives/CHANGELOG.md[Unreleased] → Fixed entry.
  • packages/coding-agent/CHANGELOG.md[Unreleased] → Fixed entry pointing at the natives release.

Plan artifact: local://fix-heredoc-deadlock-windows-macos.md (surfaced from a session report; no issue tracked).

can1357 and others added 30 commits May 15, 2026 05:02
…ning-content-rejection

fix: exclude OpenCode providers from synthetic reasoning_content injection for Kimi models
…uto-retry

fix(ai): recover stalled lazy provider streams
…-plugin-manifest-commands-key-

fix(discovery): honor Claude plugin commands manifest key
…-doesn-t-exit

fix(coding-agent/commit): force clean exit after omp commit finishes
feat(coding-agent/acp): bridge ExtensionUIContext to ACP unstable_createElicitation
…nup-fresh

fix(coding-agent): wait for ACP prompt idle cleanup
…ud-models-in-retry-fallbackcha

fix(providers): load cached standard model discoveries
…ADME

- Rewrote `README.md` with a concise operator-first walkthrough of bot behavior and setup flow.
- Replaced large architecture and security narratives with a compact summary of proxy trust boundaries and run modes.
- Updated CLI and operational examples in `README.md` to match current `bun run`-based workflows and checks.
…lassified issues

- Added an inbound thread flag to tool bindings and set it from PR task context.
- Updated classify_issue and set_issue_labels to return no-op responses when the inbound thread is a PR or the issue is already classified, avoiding GitHub label updates.
- Expanded tests for no-op behavior and prompt text to reflect that triage tools are only used on fresh unclassified issues.
- Replaced direct hard-timeout `stop()` calls with a shared cancel hook that invokes `client.stop()` then `client._mark_closed()` using `RpcProcessExitError`.
- Added a cancellation-path workaround for an upstream `omp_rpc` issue where `stop()` alone left `_closed_error` unset and kept `_wait_for_agent_end` blocked until timeout.
- Updated worker tests to verify both hard-timeout and cancel-hook flows call `_mark_closed()` with `RpcProcessExitError`.
…ck prompt waits

- Stop now marks the client as closed via `_mark_closed` with `RpcProcessExitError`, so `_wait_for_agent_end` wakes immediately instead of waiting for a timeout.
- Added a regression test using a hanging server subprocess to verify `stop()` unblocks `prompt_and_wait` promptly and raises `RpcProcessExitError`.
- Updated Kimi compatibility tests to distinguish Moonshot-hosted models from OpenCode-hosted ones for `reasoning_content` and assistant-content tool-call behavior.
Brush-core applied POSIX parameter expansion to $env before dispatching a command, mangling PowerShell references like Write-Host $env:SystemRoot to :SystemRoot. Move the fix down to env-var application: every brush session now defines env=$env as an internal (non-exported) shell variable, so the bash expansion of $env yields the literal $env and PowerShell tokens reach the child intact. User assignments (env=prod; echo "$env:8080") still shadow the fallback in their command scope, so the POSIX bash contract is preserved.

Fixes can1357#1079
- Exempted the `fmt:rs` task from the early-exit skip logic so formatting always runs.
- Reformatted `apply_command_env` signature in shell.rs to satisfy the formatter.
…checks

- Added skip_checks option to gh_push_branch and gh_open_pr to bypass bun pre-publish checks.
- Short-circuited bun fix/check helpers when skip_checks=true and propagated the flag through gh tool calls.
- Changed rename_workspace_branch to accept pr_number and leave branch unchanged during an open-PR rename path.
- Documented skip_checks escape-hatch rules and two-strike gh_push_branch retry restrictions.
- Added tests for skip-check bypass behavior, failed-check suppression, and no-op branch rename with open PR.
- Updated classify_issue validation to audit and include command errors for invalid primary, rationale, and branch_slug inputs.
- Allowed non-bug classifications to ignore unsupported fields (priority, functional, provider, platform) without raising hard errors.
- Aligned prompt guidance and tests to match the new classify_issue tolerance for optional fields.
…runtime commands

- Added `scripts/with-pi-root.sh` to resolve `PI_ROOT` from an explicit checkout, `/work/pi`, or an auto-cloned cache directory, with env knobs for repo, ref, cache path, and auto-update behavior.
- Updated `pi-artifacts`, `rebuild`, and `up` to execute through the new wrapper so Docker build and compose startup share the resolved oh-my-pi root.
- Documented the new resolution flow in `.env.example` and `README.md`, added cache ignore entries, and kept build/runtime config aligned with the resolved root.
- Standardized triage flow to read issue context, classify before side effects, and separate bug, doc, and non-bug paths.
- Tightened bug-path execution to run repro_record, bun run fix, bun check, commit, then push and open PR.
- Reworked ambiguity handling so agents post one clarifying comment, never guess, and wait for explicit direction.
- Refined follow-up, review, and resume rules to reuse session state, preserve thread context, and amend-push existing PR branches.
- Updated host-tool docs to clarify gh_post_comment fields, bug-label requirements, skip_checks verification, and issue-closure behavior.
…e operations

- Updated hashline splitting to skip emitting a section when a path header has no following operations.
- Added a trimmed non-empty check so only sections with diff content are returned.
- Added tests for duplicate and trailing headers to ensure empty header-only chunks are silently dropped.
… request paths

- Introduced a `fetch` option on `StreamOptions` and threaded it through providers to let callers supply a custom request transport.
- Updated provider clients and direct HTTP calls across Anthropic, OpenAI, Azure, Google, GitLab Duo, Gemini CLI, Ollama, and Codex flows to use the injected fetch implementation.
- Extended retry helper options to accept a fetch override and preserved preconnect support from the selected fetch function.
- Introduced `FetchImpl` type with optional `preconnect` to accept non-Bun fetch implementations without type errors.
- Applied the new type across all providers and `StreamOptions.fetch`.
- Added tests verifying fetch override routing for openai-completions, openai-responses, and fetchWithRetry.
Mirrors the pi-mono API surface so apps can preflight tool execution
(block or mutate validated args) and post-process tool results
(override content/details/isError) without wrapping tools.

- `AgentLoopConfig.beforeToolCall` runs after argument validation. Return
  `{ block: true, reason }` to short-circuit with a tool-error result.
  Mutations to `context.args` are forwarded to `tool.execute` without
  revalidation, matching pi-mono semantics.
- `AgentLoopConfig.afterToolCall` runs after execution and before
  `tool_execution_end` / tool-result message emission. Returned fields
  override the executed result; omitted fields fall through. Hook
  exceptions surface as tool errors and do not abort the batch.
- `Agent` exposes both hooks as public, reassignable fields so extension
  reloads can swap implementations mid-session.

Compatibility: fully additive. Both hooks default to undefined and the
loop behaves identically when neither is set. The internal
`executeToolCalls` signature was collapsed to `(context, message,
signal, stream, config)` -- it is not exported, so this is not a public
API change. Pi-mono's `terminate` field on `AfterToolCallResult` is
omitted because our `AgentToolResult` has no batch-level early-stop
contract.
…hline bodies

Commit 6d05a23 made splitHashlineInputs silently drop sections whose
body has no operations, so '@<path>\n' alone now yields zero sections
and trips the 'exactly one hashline section' guard before reaching the
no-op or local-URL checks the tests intended to exercise.

- 'returns no-op error for unchanged content' now uses '= 1<hash>..1<hash>'
  with the existing line as payload (true no-op via formatLineHash).
- 'returns a handled error when the source path is a local URL' appends
  '+ EOF\n' so the section is emitted and resolveToCwd surfaces the
  internal-scheme rejection.
- Implemented question auto-close scheduling with configurable enablement, delay, and scan interval.
- Added reaction lookup and issue-close operations across backend, proxy, and client APIs.
- Extended host tools and app server wiring to append auto-close suffixes, run scheduler, and cancel closures.
- Updated documentation and env examples with question-autoclose settings and behavior notes.
- Added pending_closures persistence and lifecycle handling to claim, finalize, requeue, and cancel rows.
- Added tests covering scheduler, DB, proxy, server, and host-tool cancellation scenarios.
can1357 and others added 22 commits May 17, 2026 11:20
…tial-resolver-does-not-support

fix(ai): support credential_process profiles in AWS credential resolver
…l-path

fix(coding-agent): correct install.sh fallback URL in omp update warning
…ee-horizontally-overflows-view

fix(tui): compress deep session tree gutters
fix(coding-agent): defer ACP session startup
Normalized OpenAI Responses schema sanitization so object schema nodes always include properties while only traversing schema-valued positions. Wired the same sanitizer into the OpenAI Codex Responses tool conversion path and added regression coverage for no-argument MCP-style schemas and literal payload preservation.

Fixes can1357#1147
…utput

- Collapsed a multi-line reinstall-warning `console.log` statement into a single line.
- Preserved the existing warning message text and only simplified its formatting.
Added dependencies and contentSchema to the OpenAI Responses schema-position sets so draft-04..07 dependencies maps and draft 2019-09 contentSchema nodes also get the properties-on-object normalization. Recognized array-form type declarations that include object. Preserved malformed non-array oneOf payloads instead of dropping them. Documented the cycle-safe cache seed and added a self-referential regression test.

Refs can1357#1147
…s-pr-d

ACP: keep command details stable across updates and replay
…on-gate

fix(coding-agent): repair ACP permission flow for file edits
…nuation

fix(coding-agent): keep ACP async continuations owned
…ex-handoff-fails-on-no-argumen

fix(ai): normalize OpenAI object tool schemas
…r 401

- Buffered `start` events until after the first replay-unsafe event and replayed them on auth failure.
- Retried stream requests with refreshed credentials when `onAuthError` returned a new key for gateway and pi-native.
- Added 401 error-status parsing for assistant errors and updated stream-auth tests for start+401 retry behavior.
- Added `AuthRetryFailure` helpers and status extraction types to propagate auth-retry metadata through stream attempts.
- Tracked ACP tool-call inputs per session and replayed them via `toolArgsById`/`getToolArgs` plumbing.
- Merged ACP tool execution end content from start and result events so command output replay preserves original args.
- Scoped ACP async-job draining by session `ownerId` and `agentId` with in-flight tracking and permission-gated deferred turns.
- Refactored compaction telemetry and async tests with per-test telemetry setup and asynchronous teardown resets.
- Set GIT_CONFIG_* and GIT_TERMINAL_PROMPT environment variables to prevent test interference from user gitconfig, LFS filters, signing, and credential helpers.
- Enhanced git error messages to include stdout when stderr is empty, providing better diagnostics for test failures.
- Replaced symlink-resolving pathIsWithin with lexical path containment check to prevent test isolation bypass via symlinked extensions.
The Bun.env scrub and filterProcessEnv used isValidEnvName (strict shell
identifier shape), which deleted standard Windows variables like
ProgramFiles(x86) and CommonProgramFiles(x86). procmgr.ts imports this
module before resolving the shell and reads Bun.env['ProgramFiles(x86)']
to find Git Bash under 32-bit Program Files, so installations that only
had Git there were no longer discovered and failed with 'No bash shell
found'.

The unsafe cases for native execve are '=' or NUL in names and NUL in
values, not parentheses. Introduce isSafeEnvName covering exactly those
cases and use it for the in-place Bun.env scrub and the spawn-env
filter. Keep isValidEnvName (strict) for dotenv parsing, where strict
shell-identifier shape is the right contract.
fix(utils): ignore unsafe dotenv entries
… macOS

brush_core::interp::setup_open_file_with_contents wrote the entire heredoc/here-string body into an anonymous pipe synchronously before handing the reader to the downstream command. Bodies that exceed the OS pipe buffer (~4 KiB on Windows, 16-64 KiB on macOS) deadlocked the writer forever, and the bash tool tripped its 305 s hard timeout without ever launching the consumer. The Linux fast path still uses F_SETPIPE_SZ to grow the pipe inline; every other platform (and Linux bodies that overflow pipe-max-size) now decouples the write onto a fire-and-forget thread that terminates on drain or BrokenPipe.

Adds a 256 KiB regression test that exercises the worst-case shape (: builtin, which never drains stdin), guarded by tokio::time::timeout(10s) so a regression fails CI fast instead of hanging.
@oldschoola

Copy link
Copy Markdown
Owner Author

Superseded by upstream PR can1357#1325.

@oldschoola oldschoola closed this May 24, 2026
can1357 pushed a commit that referenced this pull request May 27, 2026
…s unsupported-effort models

Triple-stacked failure on the same axis (thinking effort) produced the
user-visible

    Error: Compaction failed: Thinking effort high is not supported by
           xai-oauth/grok-build.
    Supported efforts:

(empty list after the colon) whenever the active model was a curated
xAI catalog entry with compat.supportsReasoningEffort: false.

Three defects lined up. (1) Behavior: compaction at four call sites
in packages/agent/src/compaction/compaction.ts hardcoded
reasoning: Effort.High and never threaded session.thinkingLevel —
the user's /model :off selection (and any explicit low/medium) was
silently overridden. On every other model this was invisible.
(2) Validation: requireSupportedEffort threw at the openai-flavored
mapper layer before the wire-side omitReasoningEffort gate in
providers/xai-responses.ts ever ran; two contradictory guards on the
same wire param. (3) Message: when getSupportedEfforts returned [],
the rendered error tail was 'Supported efforts: ' with nothing after
the colon — disappears as a side-effect of fix #2.

Fix #1 — thread ThinkingLevel | undefined end-to-end. Add
SummaryOptions.thinkingLevel and HandoffOptions.thinkingLevel.
Convert via a single exhaustive switch (effortFromThinkingLevel) in
the new resolveCompactionEffort helper:
  - Off            → undefined  (omit reasoning entirely)
  - undefined/Inherit → Effort.High → clamp per model (preserves the
                                       historical default for users
                                       who never touched the dial)
  - explicit Effort → respect user → clamp per model

resolveCompactionEffort lives in compaction.ts; all four call sites
(generateSummary, generateHandoff, generateShortSummary,
generateTurnPrefixSummary) route through it. agent-session.ts threads
this.thinkingLevel into all three production compaction entry points
(manual /compact at L6201, auto-compaction at L6458 — the most-fired
path, originally missed in plan review — and direct generateHandoff
at L5465). The audit-gate test
(test/agent-session-compaction-thinking-threading.test.ts) scans the
file with a brace-balanced extractor and refuses any unthreaded site.

Fix #2 — silent-clamp at the openai-flavored mapper layer. Extract
exported modelOmitsReasoningEffort(model) in model-thinking.ts as the
single source of truth for compat.supportsReasoningEffort: false on
openai-responses* APIs. getSupportedEfforts now calls it instead of
inlining the check (pure refactor — observable behavior preserved).
resolveOpenAiReasoningEffort in stream.ts early-returns undefined
when the predicate is true, so the wire-side omitReasoningEffort
gate (providers/xai-responses.ts:78) becomes the single source of
truth for the actual strip — no redundant throw.

Three regression tests pin the contract:
  - packages/ai/test/xai-oauth-effort-strip.test.ts (5 tests):
    modelOmitsReasoningEffort returns true for grok-build and
    grok-4.20-0309-reasoning, false for grok-4.3 / Anthropic /
    openai-completions.
  - packages/agent/test/compaction-thinking-level.test.ts (5 tests):
    every ThinkingLevel outcome through generateHandoff — Off stays
    undefined (not coerced to High), Low stays Low, Inherit / undefined
    default to High, grok-build clamps to undefined regardless of
    requested level. Covers the Codex-caught Off-vs-not-provided
    distinction.
  - packages/coding-agent/test/agent-session-compaction-thinking-threading.test.ts
    (2 tests): brace-balanced source scan asserts every direct
    compact() / generateHandoff() in agent-session.ts threads
    'thinkingLevel: this.thinkingLevel'; floor of 3 threaded sites.

TDD red-green verified for fix #1: temporarily reverted the handoff
call-site back to hardcoded Effort.High → compaction-thinking-level
went 2 pass / 3 fail (Off coerced, Low overridden, grok-build throws);
restored → 5 pass / 0 fail.

Verified:
  - packages/agent:  127 pass / 0 fail
  - packages/ai:     1061 pass / 337 skip / 0 fail
  - packages/coding-agent (focused): 179 pass / 5 skip / 0 fail
  - biome + tsgo --noEmit clean across all three packages

Out of scope (follow-ups):
  - branch-summarization.ts:307 already passes no reasoning — no edit.
  - The empty-list error message at model-thinking.ts:296 is now
    structurally unreachable from the openai-responses path.
  - modelOmitsReasoningEffort and grokSupportsReasoningEffort
    (xai-responses.ts:22) overlap; collapse into a single predicate
    in a future commit.

Op: correct
Restores: spec:compaction-honors-session-thinking-level
Restores: spec:xai-oauth-grok-build-compaction-no-throw
(cherry picked from commit e07b47e)
can1357 pushed a commit that referenced this pull request May 27, 2026
The thinking-level fix (e07b47e) added SummaryOptions.thinkingLevel
and threaded it from agent-session.ts into compact(), but the
field-by-field rebuild of summaryOptions inside compact() (and a
second inline rebuild for generateShortSummary) silently dropped it.

Effect on every call site that fans through compact():
  generateSummary, generateTurnPrefixSummary, generateShortSummary all
  see options?.thinkingLevel === undefined => resolveCompactionEffort
  falls back to Effort.High => user's /model :off selection is
  silently overridden, and xai-oauth/grok-build still trips on the
  unsupported-effort path even though fix #2 strips it at the wire
  layer of the openai-responses mapper.

Add `thinkingLevel: options?.thinkingLevel` at both rebuild sites in
compact(): the summaryOptions literal feeding generateSummary and
generateTurnPrefixSummary, and the inline options literal feeding
generateShortSummary.

Extend compaction-thinking-level.test.ts with four compact()-level
cases driving isSplitTurn:true so all three summarizers fire:
  - Off                  -> every fan-out call gets reasoning=undefined
  - Low                  -> every fan-out call gets reasoning="low"
  - <unset>              -> every fan-out call gets reasoning="high" (default)
  - grok-build + High    -> every fan-out call gets reasoning=undefined (clamp)

TDD red-green verified: stashing the source fix flips the Off and Low
cases to fail with received="high" (exactly the reviewer's prediction);
restoring the fix returns all four to green. Suite: 131 pass / 0 fail
(baseline 127 + 4 new). biome + tsgo --noEmit clean.

Op: correct
Restores: spec:compaction-honors-session-thinking-level
(cherry picked from commit 9b501e3)
oldschoola pushed a commit that referenced this pull request May 28, 2026
…s unsupported-effort models

Triple-stacked failure on the same axis (thinking effort) produced the
user-visible

    Error: Compaction failed: Thinking effort high is not supported by
           xai-oauth/grok-build.
    Supported efforts:

(empty list after the colon) whenever the active model was a curated
xAI catalog entry with compat.supportsReasoningEffort: false.

Three defects lined up. (1) Behavior: compaction at four call sites
in packages/agent/src/compaction/compaction.ts hardcoded
reasoning: Effort.High and never threaded session.thinkingLevel —
the user's /model :off selection (and any explicit low/medium) was
silently overridden. On every other model this was invisible.
(2) Validation: requireSupportedEffort threw at the openai-flavored
mapper layer before the wire-side omitReasoningEffort gate in
providers/xai-responses.ts ever ran; two contradictory guards on the
same wire param. (3) Message: when getSupportedEfforts returned [],
the rendered error tail was 'Supported efforts: ' with nothing after
the colon — disappears as a side-effect of fix #2.

Fix #1 — thread ThinkingLevel | undefined end-to-end. Add
SummaryOptions.thinkingLevel and HandoffOptions.thinkingLevel.
Convert via a single exhaustive switch (effortFromThinkingLevel) in
the new resolveCompactionEffort helper:
  - Off            → undefined  (omit reasoning entirely)
  - undefined/Inherit → Effort.High → clamp per model (preserves the
                                       historical default for users
                                       who never touched the dial)
  - explicit Effort → respect user → clamp per model

resolveCompactionEffort lives in compaction.ts; all four call sites
(generateSummary, generateHandoff, generateShortSummary,
generateTurnPrefixSummary) route through it. agent-session.ts threads
this.thinkingLevel into all three production compaction entry points
(manual /compact at L6201, auto-compaction at L6458 — the most-fired
path, originally missed in plan review — and direct generateHandoff
at L5465). The audit-gate test
(test/agent-session-compaction-thinking-threading.test.ts) scans the
file with a brace-balanced extractor and refuses any unthreaded site.

Fix #2 — silent-clamp at the openai-flavored mapper layer. Extract
exported modelOmitsReasoningEffort(model) in model-thinking.ts as the
single source of truth for compat.supportsReasoningEffort: false on
openai-responses* APIs. getSupportedEfforts now calls it instead of
inlining the check (pure refactor — observable behavior preserved).
resolveOpenAiReasoningEffort in stream.ts early-returns undefined
when the predicate is true, so the wire-side omitReasoningEffort
gate (providers/xai-responses.ts:78) becomes the single source of
truth for the actual strip — no redundant throw.

Three regression tests pin the contract:
  - packages/ai/test/xai-oauth-effort-strip.test.ts (5 tests):
    modelOmitsReasoningEffort returns true for grok-build and
    grok-4.20-0309-reasoning, false for grok-4.3 / Anthropic /
    openai-completions.
  - packages/agent/test/compaction-thinking-level.test.ts (5 tests):
    every ThinkingLevel outcome through generateHandoff — Off stays
    undefined (not coerced to High), Low stays Low, Inherit / undefined
    default to High, grok-build clamps to undefined regardless of
    requested level. Covers the Codex-caught Off-vs-not-provided
    distinction.
  - packages/coding-agent/test/agent-session-compaction-thinking-threading.test.ts
    (2 tests): brace-balanced source scan asserts every direct
    compact() / generateHandoff() in agent-session.ts threads
    'thinkingLevel: this.thinkingLevel'; floor of 3 threaded sites.

TDD red-green verified for fix #1: temporarily reverted the handoff
call-site back to hardcoded Effort.High → compaction-thinking-level
went 2 pass / 3 fail (Off coerced, Low overridden, grok-build throws);
restored → 5 pass / 0 fail.

Verified:
  - packages/agent:  127 pass / 0 fail
  - packages/ai:     1061 pass / 337 skip / 0 fail
  - packages/coding-agent (focused): 179 pass / 5 skip / 0 fail
  - biome + tsgo --noEmit clean across all three packages

Out of scope (follow-ups):
  - branch-summarization.ts:307 already passes no reasoning — no edit.
  - The empty-list error message at model-thinking.ts:296 is now
    structurally unreachable from the openai-responses path.
  - modelOmitsReasoningEffort and grokSupportsReasoningEffort
    (xai-responses.ts:22) overlap; collapse into a single predicate
    in a future commit.

Op: correct
Restores: spec:compaction-honors-session-thinking-level
Restores: spec:xai-oauth-grok-build-compaction-no-throw
(cherry picked from commit e07b47e)
oldschoola pushed a commit that referenced this pull request May 28, 2026
The thinking-level fix (e07b47e) added SummaryOptions.thinkingLevel
and threaded it from agent-session.ts into compact(), but the
field-by-field rebuild of summaryOptions inside compact() (and a
second inline rebuild for generateShortSummary) silently dropped it.

Effect on every call site that fans through compact():
  generateSummary, generateTurnPrefixSummary, generateShortSummary all
  see options?.thinkingLevel === undefined => resolveCompactionEffort
  falls back to Effort.High => user's /model :off selection is
  silently overridden, and xai-oauth/grok-build still trips on the
  unsupported-effort path even though fix #2 strips it at the wire
  layer of the openai-responses mapper.

Add `thinkingLevel: options?.thinkingLevel` at both rebuild sites in
compact(): the summaryOptions literal feeding generateSummary and
generateTurnPrefixSummary, and the inline options literal feeding
generateShortSummary.

Extend compaction-thinking-level.test.ts with four compact()-level
cases driving isSplitTurn:true so all three summarizers fire:
  - Off                  -> every fan-out call gets reasoning=undefined
  - Low                  -> every fan-out call gets reasoning="low"
  - <unset>              -> every fan-out call gets reasoning="high" (default)
  - grok-build + High    -> every fan-out call gets reasoning=undefined (clamp)

TDD red-green verified: stashing the source fix flips the Off and Low
cases to fail with received="high" (exactly the reviewer's prediction);
restoring the fix returns all four to green. Suite: 131 pass / 0 fail
(baseline 127 + 4 new). biome + tsgo --noEmit clean.

Op: correct
Restores: spec:compaction-honors-session-thinking-level
(cherry picked from commit 9b501e3)
oldschoola added a commit that referenced this pull request Jun 23, 2026
…ved directory globs, and live GLM plan evaluation script

Multi-signal convergence boost (fast-context.ts):
- Track 3 independent signal categories: planGlobMatchedSet, planGrepFileSet, suppGlobMatchedSet
- +3 per signal beyond the first, added to contentScore BEFORE type multiplier
- Rewards files matching plan-glob + plan-grep + supp-glob (convergent evidence)
- Fixes conversation-context can1357#7->#1, MCP #2->#1, git-status #2->#1, worker-host #2->#1
- MRR 0.9444->0.9475 (deterministic, 27 queries). 25/27 at #1 (was 21/22)

Config/data file penalty (fast-context.ts):
- .json/.yaml/.toml/.csv/.svg files get 0.7x type multiplier
- Prevents porcelain.json from outranking git.ts via convergence boost

Keyword-derived directory globs (fast-context.ts):
- Query keywords >=6 chars generate **/${kw}/**/* directory globs
- Catches GT files in named directories (identity/classify.ts, session/session-context.ts)

Hint-system prompt refinement (fast-context-hint-system.md):
- Added glob specificity guidance with GOOD/BAD examples
- Trimmed to reduce token overhead (avoids GLM parse failures from longer prompt)

Live GLM plan evaluation script (bench-fast-context-live-glm.ts):
- Calls real GLM (zai/glm-5-turbo) to generate search plans
- Scores plan quality: parse rate, glob hit rate, grep hit rate, keyword coverage
- Feeds real plans through ranking pipeline for end-to-end MRR with real model plans
- Key finding: MRR drops 0.94->0.80 with real GLM plans — plan quality is the gap
- This is the ONLY benchmark that exercises the hint-system prompt (deterministic benchmark mocks the plan)
- Separated from autoresearch.sh to preserve deterministic contract
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.