integration: the prime-adoption batch + engine slice 1c (merges #433 #434 #435 #436 #437 #438) - #446
Merged
Merged
Conversation
Slice 1c needs the notices out of agent_tools.zig, and every one of them went through say(), whose routing is not incidental: --json swallows a root's line, a pool-thread child has no writer and goes through the tick gate with a "[label] " prefix and a repair for the cut newline, and a format ending in \n releases held child ticks. A sink holds bytes, not a comptime format, so it cannot call say(). This is the same function with the line-ending test moved from the format to the last byte, and errors swallowed because an emit path has nowhere to return them. say() is left alone: its worker branch formats prefix and payload in ONE print into a fixed slot, and re-deriving that through a temporary would change where an over-long line gets cut. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
…inks (#422) Eleven variants for the moments agent_tools.zig has been drawing inline: the call bracket (announced/started) and its closing bracket (result/ finished), a pre-run refusal, the parallel-batch tallies, and the meta-tool notices. The pairs exist because one moment is TWO wire lines and each line owns a sequence id — collapsing them would reserve one id and write two. durable() now reads the payload, not just the tag, and it has to: whether a tool moment reaches the wire depends on which tool it is (ask_user's bracket goes out as the `ask_user` event; a meta tool's result never had a wire shape). JsonSink is now defined as "write exactly the durable events" with a panic on the else arm, so a future durable variant cannot reach the wire without a shape and burn its id on nothing (#330). The terminal half lives in agent_tool_render.zig — the one file in the cluster that reaches the palette, like agent_stream_render.zig for the stream. Every function there is the old code path gate for gate, with the two byte-preserving caps (160 arg bytes, 100 preview bytes) named rather than inlined. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
#422) The engine half keeps only what it owns: which moments happened, and the bookkeeping around them (the batch tally, argStreamedFully's dedup state, isMetaName). Every json/tty fork it used to carry is now a sink's choice, including the two suppressions that used to look symmetric but are not — the wire skips ask_user's bracket, the terminal skips prose that already streamed live. The `!self.sub` guards move into the renderer with the drawing they gated; goal_completed deliberately keeps none, matching the old call site (goalActive() is false for a subagent, so it is root-only anyway). sayToolResult keeps its out == null early-out at the emit site: that one is not presentation, it is "there is no frontend attached". One accepted behavior delta: sayToolUse's writes used to propagate and abort the batch on a failed stdout; a sink's emit returns void, so they are swallowed now, like every other converted emission since slice 1a. term.zig stays imported. Its use here is raw/nonblocking stdin for the Esc watcher, which is frontend INPUT and belongs to #430 — the same carve-out #429 already makes for agent_ws.zig. Proven by the eval harness: all 9 golden files byte identical, including the permission-prompt PTY golden whose ⚙/prompt/✓ interleaving is exactly what this touches. exec.zig, tools.zig and edit_verify.zig needed no change (every write there builds the tool RESULT, never the terminal), and agent_tool_gate.zig's lines are all prompt-block text, held for #430. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
#330) EngineSink.emit stamps BEFORE dispatch, so a durable sink handed an event it will drop burns a #330 sequence id on nothing. jsonEmit drops everything when out == null, and slice 1c moved two emitters (sayToolUse, emitToolRejected) ahead of any writer check — so in `graff --json` every subagent tool call advanced the SHARED counter by two with no wire line, and in `graff acp` (json_mode on, root.out nulled) the root did the same for a whole session, inflating the persisted event_seq. A supervisor reads that jump as lost data, which is the one thing protocol_seq promises cannot happen. Fixed where the rule can be stated once instead of re-derived per call site: jsonSink hands a NON-durable vtable to an agent with no writer. Same emitter, same (absent) output, no reservation — and no g_gui_mu round trip per pool-thread tool call either. Not fixed by guarding the two emitters, which was the obvious move and is wrong: on main sayToolUse's TUI branch went through say(), whose out == null path is the pool-thread WORKER LINE, so an early return there would have deleted every subagent's ⚙ line from the terminal. sayToolResult's own guard stays: main had it too. Also, per review of the slice: the four `!self.sub` gates go back to their emit sites. Who may announce a fan-out or a meta notice is engine policy about who owns the terminal, not a drawing decision; keeping it in agent_tool_render meant the renderer back-read Agent state (widening the debt engine_sink.zig's header says to shrink) and, worse, meant a future serve/attach sink would start receiving subagent tallies the engine never used to produce, with no field in the payload to tell them apart. Two test tightenings from the same review: the cap assertions now pin the literals 160/100 the pre-#422 inline path spelled out rather than the new constants (a test that compares the code to itself cannot guard a conversion), and the 100-byte result cap gets its first exercise. Proven: new engine_sink test asserts protocol_seq.current() == 0 after a full tool bracket + rejection + text delta on a writerless agent (it read 6 before). 1003/1003 tests pass and all 9 eval goldens are byte identical. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
The per-output cap (#193/#196) destroyed the elided bytes: the model's only recovery was to re-run the tool and guess a better slice. Now, when the agent has a durable session, the FULL output is written to `.graff/sessions/<session>/artifacts/tool-<n>.txt` before the cap shrinks it, and the marker cites the absolute path and the byte count, so the next turn can read or grep exactly what it needs. Same call site and same safety class as #196 (no WS-close bracketing): the cap still only shrinks strings in place. A subagent has no persisted history, so it has no durable session and keeps the plain truncation, as does any process that never wired a sink (every unit test). Growth is bounded twice: `session_cap_bytes` (64 MiB, all-or-nothing per artifact so a marker can never lie about its byte count), and reclamation of the artifact dirs whose `<session>.session.json` is gone — the session file is the ground truth for "this session was deleted", so an rm, the AI-title rename and /new all reclaim what they left behind. The sweep runs once, at the first spill, so a run that never spills does no extra I/O. The cap's truncation primitives move to the new module with it: the spill has to happen inside truncateStrField, the one place still holding the pre-truncation string, and agent_compact.zig sits at the 600-line cap. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
A /goal (or /loop) run is plan-act-VERIFY and re-verifies on every continuation, so the obvious move for a model that has just been told "completion is blocked, run eval" is to run eval again. When it has edited nothing since the last RED, that re-run costs the whole --eval command, a --judge model call, and 1500 bytes of output tail to re-derive a verdict that could not have changed. verify_fingerprint.zig fingerprints the verification before running it, and a fingerprint identical to the one the last verification FAILED on means the verifier is not run at all: - the fold covers every input to "could this produce a different result": the eval command text, `git status --porcelain -z -uall`, `git diff --binary HEAD`, and the CONTENTS of every untracked file the status listed. The last one is load-bearing - editing an untracked file leaves the porcelain line and the diff byte-identical, so without it the guard would skip a verify over real work. Parts are length-prefixed, so a byte moved across a file boundary is a change, not a collision. - the attempt still counts (eval_iter advances). A model that keeps calling eval without editing has to converge on the iteration cap, not spin for free. - the steer names the actual blocker instead of manufacturing a verdict: the workspace has not changed since the last failed verification, edit source files or tests first. Completion stays blocked either way, since a skipped run verifies nothing. FAIL-OPEN everywhere: no repo, no git, a timed-out probe, a truncated stream, an unreadable file, an absurd untracked count - each yields an unknown fingerprint, which never matches, so the verifier runs. The guard can cost a skipped re-run only when it is certain nothing moved; it can never invent a pass. A command that could not RUN disarms it too - "edit source files" is not the fix for that, and a stale fingerprint must not suppress the retry. The fold and the decision are pure and unit-tested without a repo (identical / tracked change / untracked-only change / boundary collision / fail-open matrix). The end-to-end test drives the real runEval against a real single-commit fixture repo and counts VERIFIER INVOCATIONS: six attempts, four spawns, the two no-progress ones free, and a tracked edit, a new untracked file and an edit to that untracked file each re-arm it. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
scripts/test-spill-artifact.py drives a real graff against the repo's scripted model (scripts/eval/mock_model.py on the lmstudio port) with a seeded session whose last tool output is over the per-output cap, and asserts all three halves of the claim: the request the harness sent carries the marker (absolute path + byte count) and NOT the elided needle, the artifact on disk holds the original bytes byte for byte, and a follow-up `bash` call against THE PATH THE MARKER CITED brings the needle back. Negative control: with the sink unwired the test fails with "no #409 marker" and no read-back. The marker's path is now resolved with realPathFile through the same dir handle the artifact was written with. The declared base (g_cwd_display) falls back to $PWD, which a caller that changed directory without exporting it gets wrong — and that produced a path that pointed at nothing, exactly what the e2e caught. The user-facing cap note now distinguishes the two outcomes: bytes elided but kept as an artifact, versus the pre-#409 destructive truncation (#202). Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
CHANGELOG entry for v0.0.240 and the README's context-management paragraph, which described the tool-result preview pointer but stopped at the send-time cap, where the bytes used to simply go. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
A bare pid is not an owner. Pids are a small recycled namespace, so a crashed holder's number is handed to something unrelated and a lock keyed on the pid alone reads as held forever - or, keyed on a timeout instead, gets stolen from a holder that is very much alive. proc_identity.zig reads the START identity of a pid: /proc/<pid>/stat field 22 on Linux (counted from the LAST ')', since comm may contain both spaces and parentheses), proc_pidinfo(PROC_PIDTBSDINFO) on macOS - a libSystem call rather than the `ps -o lstart=` subprocess the issue suggested, so there is no fork in a lock path, no locale-dependent date parsing and microsecond rather than one-second resolution - GetProcessTimes on Windows, and pid-only liveness anywhere else. Liveness is now "the pid is alive AND it is still the same process", and only a provable mismatch makes a lock reclaimable. Two rules keep the upgrade safe: a record with no start identity (an older graff's, or an identity-less platform's) keeps the pre-#413 pid-only contract, so an in-flight lock is never bricked; and a probe that FAILS is `.unknown`, which means held - wrongly reclaiming a live lock corrupts, wrongly honouring a dead one only waits. Both lock modules take it up. worktree_lease.Owner.start_ns becomes start_id and gains the producer it never had (selfOwner/probeOwners), with a new live_unverified verdict for a pid we cannot identify. The #289 degraded session write - a filesystem whose advisory locks do not work - stops racing unguarded and brackets itself with an owner record, via the reusable claimOwnerFile/releaseOwnerFile the credential store and any future daemon lease can share. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
…ename (#409) Moving a renamed session's artifacts (or dropping them at the rename) would strand every path already handed to the model in that transcript. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
…vioral detectors (#414) The #193 classifier was six case-sensitive substrings and a two-code list. Three gaps, all reported against real providers: 1. Nothing was checked BEFORE the overflow patterns. Bedrock formats throttling as "ThrottlingException: Too many tokens, please wait before trying again." — wording that collides head-on with the kind of generic "too many tokens" fallback a broad table needs. Classifying that as overflow discards real conversation to fix a problem compaction cannot fix, and shadows the Retry-After ladder that would have worked. A guard list now wins outright, scoped to throttling/quota: the family whose remedy is "wait", not "send less". Broader guards (a bare "overloaded", a generic "server_error") were deliberately rejected — a guard that swallows a REAL overflow wedges the session, which is worse. 2. Some providers accept an over-window input and never send an error. z.ai returns HTTP 200 with an empty completion; the only evidence is usage.input at or over the window. Classified from the usage now, and recovered through the existing pin-trim-retry path. 3. Others truncate the input to fit and report finish_reason=length with zero output — the model answered a conversation we did not send. Named distinctly (a notice, a trace note, telemetry, and last_api_error when recovery is unavailable) instead of reaching the user as an inexplicably short answer. Conservative by construction: output_tokens must be exactly 0, no content, and the input must fill >= 99% of the window, so an ordinary max-tokens completion cannot trip it. The pattern table also grew to the phrasings twelve more providers actually send, and matches case-insensitively — a Title-Cased rejection used to miss and wedge the session. The guards are what make that breadth safe. Classification moved to agent_overflow.zig; agent_request_policy.zig was at the 600-line cap and re-exports it, keeping its two original test names so the behavioral eval harness does not read the move as deleted coverage. Tests: table-driven fixtures for every guard, every added provider phrasing, and regression rows for all six original patterns proving no reclassification (37 rows), plus 18 completion fixtures covering both detectors and the shapes that must NOT trip them. scripts/test-pty-overflow.py gains three real-PTY scenarios (Bedrock throttle stays on the retry path; the silent 200 compacts and retries; truncate-then-length is reported). Suite 994 -> 1000; golden eval harness byte-identical to a pristine build of the base commit. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
…ranscript line (#421, #410) ## What changed The root system prompt is no longer one frozen string. `src/prompt_text.zig` holds it as 16 capability-scoped segments; `prompts.zig` owns the `segments` table that gives each one a `Gate`, the comptime `main_system_prompt` those segments still concatenate to (an `Agent` struct default, so it has to stay comptime), and `composeBase()`, which drops every segment whose capability this process does not have. `startup.buildSystemPrompt` now starts from `prompts.baseForSession(arena)` instead of the constant. Gates are read from the two predicates dispatch already refuses a hallucinated call with (`no_local_tools.blocks` + `tool_gates.blocks`), so the prompt cannot disagree with the catalog. What the gates remove today: - `--no-local-tools` (#330) drops the read_file/edit_file/write_file/codedb/bash paragraph, the `.graff/traces` paragraph (its instruction is "read and analyze it", and the trace is a host file), the `gh issue create` paragraph, and the batching note's three tool examples: 1930 of 6777 chars, 28.5%; - `subagent`/`workflow` absent drops the fan-out paragraph (413 chars); - `todo_write` absent drops both todo paragraphs (197); - `note_constraint` absent drops the record-a-rejection paragraph (388). Two prompt-doctrine lines from the prime-agent analysis are adopted where they had a home: "never invent a tool, a parameter, or a wrapper API" joins the always-on intro, and "run the target project through its OWN environment" joins the verify-your-work paragraph. #410 adds one line naming this session's durable transcript. It composes inside `setSystemPrompts`, not at the call site, so a persona swap or a `set_system_prompt` cannot drop it; `buildRootAgent` settles `session_name` before the funnel runs. `prompt_snapshot_tests.zig` pins all of it: the full-capability prompt against an inline golden, and a capability matrix asserting that an absent capability contributes ZERO text (exact length AND the dropped segment's bytes being unfindable), plus the already-gated MCP/skill/optional-tool/goal paths. ## Why - Problem/failure mode: every session paid for every instruction. An embedder running `--no-local-tools` was told to `read_file` before editing and to prefer `codedb` over `bash grep` for tools the provider was never told exist - tokens on every turn, and an invitation to call something that cannot work. - Reason for this approach: the segment table is the single source of both the comptime constant and the runtime composition, so the two cannot drift or reorder; and `composeBase` returns the constant itself at full capability, so the common path allocates nothing and is byte-identical to what shipped. - Constraints or trade-offs: the transcript line says JSON, not JSONL as the issue assumed - `.graff/sessions/<name>.session.json` is one object, the JSONL files are the traces - and it does not claim to hold what compaction discarded, because compaction rewrites the retained history in place. A wrong format or provenance claim costs the model a turn discovering otherwise. - Rejected alternatives: gating the git/PR/commit guidance too. An embedder still reaches a sandbox where git may run, and "never discard the user's work" is the wrong instruction to make optional. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
…an forge The marker embeds the artifact's REAL absolute path, and std.testing.tmpDir names its directory with 16 random base64-url characters. 'N' is in that alphabet, so "the needle is gone from the transcript" failed whenever the random name happened to contain one: 4 failures in 15 runs, measured identically on this integration AND on feat/prime-409-spill alone. Test-only; the assertion's intent is unchanged. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
…an forge The marker embeds the artifact's REAL absolute path, and std.testing.tmpDir names its directory with 16 random base64-url characters. 'N' is in that alphabet, so "the needle is gone from the transcript" failed whenever the random name happened to contain one: 4 failures in 15 runs, measured identically on this integration AND on feat/prime-409-spill alone. Test-only; the assertion's intent is unchanged. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
The fingerprint folds untracked file contents, and runEval appends to .graff/eval-log.tsv on every call — in a repo that does not gitignore .graff/, the tree provably moved each iteration and the guard failed open (measured: strictly worse than no guard, +prompt growth, zero savings). graff's own state dir is now excluded from the untracked scan. Found by the integration batch comparison. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
…hs are not /-rooted (#409) The test matched 'are at /' and posix separators in the marker; on windows the resolved path is drive-rooted with backslashes and the windows CI job failed on exactly this test. The path is now extracted from the marker and asserted absolute via std.fs.path.isAbsolute, with the artifact name matched separator-free. Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
…ert) Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
…ed scan) Co-Authored-By: Codegraff <blackfloofie@codegraff.com>
This was referenced Aug 6, 2026
Spill, don't truncate: oversized tool outputs go to a session artifact, cap note cites the path
#409
Closed
This was referenced Aug 6, 2026
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.
The pre-validated integration of the six feature PRs, merged serially with conflicts resolved (one
v0.0.241changelog section, alltest_hookswirings kept,agent_request.zigcarrying both #436's and #437's changes), including both post-comparison fixes (windows-portable spill assert,.graff/untracked-scan exclusion).This exact tree is what every quality gate ran against:
Merging this makes #433/#434/#435/#436/#437/#438 reachable from main (GitHub auto-marks them merged). #442 and #443 follow separately.