Small train: scratchpad, MCP decision tools, and lock custody fixes - #740
Merged
Conversation
…budget Closes ibl-2b09abdadd25. context.py::_warn_if_over_budget only logged when the scratchpad section exceeded SCRATCHPAD_SECTION_BUDGET_CHARS (90_000); the raw file was still shipped whole into every context build. - ouroboros/context.py: new _render_scratchpad_for_context degrades to the newest whole blocks that fit (block-boundary cuts only), always retains at least the single newest block, and appends an in-band gap-marker (BIBLE P1: no silent truncation) whenever older blocks were dropped OR the retained block(s) alone still exceed budget. The raw WARNING log is preserved (fires on the pre-trim value) so the rot class stays visible. - ouroboros/context_budget.py: SCRATCHPAD_MAX_CONTENT_CHARS=60_000 — source-side content cap for Memory.append_scratchpad_block, ANDed with the existing _SCRATCHPAD_MAX_BLOCKS count cap (single-pass eviction). - ouroboros/memory.py: append_scratchpad_block now evicts on whichever cap (count or content) is violated first; pinned blocks stay exempt from both, same FIFO+journal invariants as before. - tests/test_scratchpad_byte_cap.py: new coverage for both caps and the render-side degrade path (legacy fallback, gap marker on drop, gap marker on the always-retained-newest-alone case). Follow-up: closes ibl-d1ade41fe54d as well (that item tracked exactly this gap-marker-not-firing bug, filed by a prior task attempt that ran out of budget before landing the fix). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AZGh7G7f2g7w8KkQW74Z4V
The fork-local block-pinning feature (ibl-3d7b7b7d5dc9, Memory.pin_scratchpad_block) does not exist on this branch, so the eviction loop in append_scratchpad_block is adapted to plain FIFO (oldest-first, pop(0)) instead of skipping pinned blocks, and test_content_cap_respects_pinning is dropped along with it. Everything else (byte-size cap alongside the block-count cap, and the render-side block-boundary degrade in context.py) is unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AZGh7G7f2g7w8KkQW74Z4V
…ad of crashing
Closes ibl-329e1741d5f9. `_handle_task_acceptance_review` did
`agent_evidence = dict(evidence or {})` with no type guard, unlike every
other consumer of an `evidence` param in this codebase (review_dispatch.py,
subagents.py, mutation_attribution.py, delegate_evidence.py all guard with
`evidence if isinstance(evidence, dict) else {}`).
When a provider emits `evidence` as a JSON-encoded string instead of a
nested object (or any other non-dict type), `dict(some_str)` iterates the
string char-by-char — each single character isn't a length-2 (key, value)
pair, so it raises "dictionary update sequence element #0 has length 1;
2 is required" and the tool call fails outright instead of degrading
(observed live in task cbf79f0994274f90's close-out run).
Now: a string is first tried as JSON (recovering the likely-intended
object); anything else that isn't a dict is wrapped under "raw_evidence"
rather than raised. Added tests/test_task_acceptance_review_evidence_coercion.py
covering JSON-string, plain-string, and list inputs — all previously
crashed the handler, all now degrade cleanly (verified against the
unpatched handler too).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AZGh7G7f2g7w8KkQW74Z4V
- ouroboros/context.py: SCRATCHPAD_MAX_CONTENT_CHARS is only consumed by memory.py's eviction logic now, not referenced in context.py itself. - tests/test_scratchpad_byte_cap.py: unused `import pytest` (no pytest.mark/pytest.raises usage in this file). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AZGh7G7f2g7w8KkQW74Z4V
…chpad content-cap growth The scratchpad content-size cap (ibl-2b09abdadd25) added to append_scratchpad_block's eviction loop pushes ouroboros/memory.py from under 1000 lines into the 1001-1500 band, which this repo's size-ratchet CI lane blocks without an explicit rationale. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AZGh7G7f2g7w8KkQW74Z4V
… renderer _render_scratchpad_for_context re-implemented Memory._write_scratchpad_markdown and diverged from it: the writer renders blocks newest-first and joins the parts with "\n", while the context helper rendered the kept slice in storage order (oldest-first) and joined with "". Whenever the scratchpad section exceeded its context budget, the model therefore read its own working memory in the reverse of the order scratchpad.md presents it -- silently, and only on the degraded path, which is exactly when the recent blocks matter most. Extract the rendering body of _write_scratchpad_markdown (everything after its empty/corrupt guard) into a pure render_scratchpad_markdown(blocks) in ouroboros/memory.py and call it from both sides. The writer passes journal_path().exists() for the retired-blocks pointer line; the context helper passes the same flag and renders the kept newest slice. The private _SCRATCHPAD_MAX_BLOCKS import in ouroboros/context.py goes away with the duplicate renderer. Tests: two new cases pin the order fix by measuring label offsets in the persisted scratchpad.md against the degraded body (the first rendered block must be the NEWEST), and assert the degraded body minus the in-band gap marker is byte-identical to the writer's rendering of the kept slice. Both fail on the previous helper and pass on this one. Docs: docs/ARCHITECTURE.md now states that scratchpad.md is rendered newest-first and that eviction is FIFO until BOTH the block-count cap and the content cap hold. The test module docstring no longer mentions "non-pinned" blocks -- the scratchpad has no pinning concept.
…pping it
The string branch of the evidence coercion in _handle_task_acceptance_review
preserves what the agent supplied: unparseable text is wrapped as
{"raw_evidence": ...} rather than discarded. The non-dict tail did not follow
that rule -- a list, number, or bool collapsed to {} and the reviewer saw an
empty evidence packet with no trace that the agent had supplied anything. That
is the same class of silent omission the string branch was written to avoid.
Make the tail symmetric: only a genuinely absent (None) evidence argument
yields {}; any other non-dict payload is wrapped as
{"raw_evidence": repr(evidence)[:2000]}, bounded the same way the rest of the
agent-supplied section is.
Tests now assert the coercion where it is actually observable -- the kwargs the
handler forwards to build_task_acceptance_evidence, which the returned tool text
does not echo. The JSON-string case must arrive as {"tests": "green"} (decoded,
not wrapped), the list case as the raw_evidence wrapper, and an omitted evidence
argument must not invent a raw_evidence entry.
A Main or project chat message on an install with a Project runs as an ephemeral decision turn. That lane withheld the whole configured MCP surface (`mcp: ephemeral_turn`) and blocked MCP dispatch, so a healthy owner-configured MCP server was invisible from ordinary conversation while the same tools rode every managed task (#722). Capability change (a widening, owner-approved 2026-09-08): the decision turn now offers the owner's configured MCP tools behind the same network resource gate as a managed task. Extension tools stay lane-restricted, and get_schema_by_name now withholds them on that lane exactly as schemas() and execute() do, so discovery and dispatch agree for both dynamic surfaces; the surviving extensions omission row names promote_chat_to_task as the working next step instead of "(no detail)". The decision-turn rule text states the lane truthfully. The built-in allowlist, the network gate, contract disabled_tools, presence ceilings and acting-child grants are unchanged; policy_hidden_reason keeps its disclosed residual for unregistered dynamic names. Residual: the decision turn is cancelable=False, so an in-flight MCP call there cannot be stopped from the UI — the same class as the web_search/browse_page tools already admitted to that lane.
Owner decision (2026-09-08, batch 2 Q10 = A): the ephemeral decision turn behind Main and project chat admits the owner's dynamic surfaces as a managed task does — configured MCP tools (previous commit) AND enabled, granted, reviewed extension tools. The model decides inline vs promote_chat_to_task (P5); the code invariant "durable side effects" is no longer a lane filter for either surface. - registry_guards._ephemeral_block_result gates BUILT-INS only: a resolved extension tool, a dead extension name (new keyword extension_unavailable, so it keeps its EXTENSION_UNAVAILABLE answer instead of the allowlist text) and an MCP name all pass; the built-in default-deny allowlist is untouched. - schemas(): the extensions/ephemeral_turn omission row is gone; the lane's extension filters are liveness (enable + grants + review), acting-child grants and the network resource gate, as on a managed task. - get_schema_by_name: the ephemeral refusal on the extensions branch added by the previous commit is removed together with the execute deny, so discovery and dispatch agree for both dynamic surfaces. - tool_policy.format_capability_omissions: the row `detail` fallback from the previous commit is reverted (its only producer was the removed row). - Decision-turn rule text, the registry.py rationale comment and the ARCHITECTURE MCP sentence describe the lane in present tense. Capability widening, not a restriction. Unchanged: presence ceilings, acting-child external_tool_grants, contract disabled_tools, the network gate, the skill review/grant/enable lifecycle. Residual: the decision turn is cancelable=False, so an extension or MCP call in flight there cannot be stopped from the UI — the same class as the web_search/browse_page tools that lane already had (owner chose functionality, Q2 = A). Tests: ws5 CW3 tests re-pinned (guard admits both surfaces, denies a built-in mutator; schemas expose a live extension and MCP tool, hide a dead extension, no ephemeral_turn row; discovery/dispatch agreement with the extension handler actually running on the lane and EXTENSION_UNAVAILABLE for a dead one on both lanes); MCP integration test also pins a disabled server absent and undispatched on the lane; test_tool_result retargets the guard-ordering extension case to the acting-child grant guard. Fixes #722.
test_render_scratchpad_block_boundary_invariant expected a seconds field in the rendered block heading, but render_scratchpad_markdown prints the timestamp to the minute (ts[:16]), so re.finditer matched nothing and the assertion loop never ran — the test passed while asserting nothing. Drop the seconds class and bind the matches with a non-empty assertion so the loop can never silently run zero times again. The property itself already held (test_render_scratchpad_no_mid_string_truncation and the byte-identity test cover it); this only makes the pin real.
The background-observation inbox took its sidecar lock with age-only staleness (owner_aware_stale defaulted to False), so a contender that judged the lock file older than stale_sec=10.0 unlinked the lock of a LIVE holder. Both writers then entered the same append-only inbox, both read it as still empty, and both claimed the same stable ID -- two durable rows under one id, which is exactly the contract the stable-ID dedupe exists to hold. Owner-aware staleness matches what append_jsonl already does on this very same lock file (ouroboros/utils.py), whose own comment records that the non-owner-aware form "could delete the lock of a LIVE holder": this store was the outlier, not the platform. Both takers move together on purpose -- the GC/fold pass rewrites the whole inbox under the same lock and can outlive stale_sec, so leaving it age-only would only move the eviction to the other side. Dead or stampless owners still recover by age; only a provably live pid is now waited out. Evidence: CI run 34213029366, job full-test (windows-latest) -- test_cross_instance_enqueue_same_id_is_atomically_deduplicated failed with results=[True, True] / assert 2 == 1 and zero gap reasons, i.e. two complete reads of one empty inbox. The regression pin added here reproduces that symptom on the name tier (the tier with no kernel backstop) with the product change reverted, and is green with it.
34d8208 fixed the observation inbox: elapsed time alone must never evict a LIVE holder of a lock file another writer also takes, or two writers enter one authority. That was one instance of a class -- a lock file whose takers disagree about what "stale" means. This closes the class: every taker of a lock file that a second call site also takes now judges staleness the same way. Dead or stampless owners still recover through the stale-age path, so no crash recovery is lost; only a live, pid-stamped owner is now waited out. Fixed (one flag per call, no other behaviour change): * ouroboros/process_custody.py:379 -- _rewrite_ledger destructively rewrites the custody ledger that append_jsonl (process_custody.py:152) appends to under the same sidecar, owner-aware. * ouroboros/project_dialogue.py:339 -- the chat-annotation append holds the lock across a read-modify claim check (require_latest_status/token); a stolen lock hands two routing attempts the same card. * ouroboros/skill_review_history.py:613 -- append_history_once shares its sidecar with the owner-aware append_jsonl writer of the same history file (skill_review_history.py:580). * ouroboros/skill_review_history.py:679 -- the root-task projection append is nested INSIDE that hold, so its own hold is structurally long-lived. * supervisor/state.py:944 -- log rotation renames a live log out from under append_jsonl writers that hold this exact lock owner-aware. * ouroboros/utils.py:454 -- update_json_locked owns the <file>.lock sidecar that cancel-intent projection and task-result quarantine take on the very same files; its mutator is arbitrary caller code and may outlive stale_sec. * ouroboros/cancel_intents.py:119 -- cancellation_projection_lock takes the same cancel_intents.json.lock as update_json_locked (cancel_intents.py:574 and five siblings). * ouroboros/task_result_schema.py:116 -- the quarantine mover documents that it takes "the update_json_locked lock" of the row it renames. * supervisor/state.py:77 -- acquire_file_lock guards locks/state.lock, taken from supervisor/state.py, supervisor/evolution_lifecycle.py and ouroboros/agent_startup_checks.py. Availability is unchanged: an unavailable lock still proceeds with the loud _warn_state_unlocked trail instead of silently believing a stolen lock is held. Left age-only, deliberately: * ouroboros/subagent_worktrees.py:182 -- metadata=str(os.getpid()) writes no "pid=" field, so the flag would be a no-op without changing the stamp. * ouroboros/tools/git_plumbing.py:111 -- metadata carries no pid either, and the fd is closed right after acquisition: a name lease whose only recovery IS age. * ouroboros/gateway/ui_preferences.py:51, ouroboros/projects_registry.py:68, ouroboros/delegate_custody.py:299, ouroboros/delegate_start_claims.py:34, ouroboros/review_state.py:390 -- sole taker of their own lock file, short read-modify-write holds. * ouroboros/claudexor_runtime.py:1225 -- sole acquirer of install.lock; the only other reader (claudexor_runtime.py:1446) reads its age to report "install in progress" and never evicts. tests/test_lockfile_helpers.py gains one parametrized regression covering four of the fixed takers on the NAME tier (a back-dated lock file stamped with a live pid): the lock must survive with its exact identity and the target file must be byte-unchanged. All four fail on the pre-fix tree.
… the journal When the rendered scratchpad exceeds SCRATCHPAD_SECTION_BUDGET_CHARS the context build keeps the newest whole blocks and drops the oldest, but the in-band gap marker sent the actor to scratchpad_journal.jsonl for blocks that were never retired (they are still live in scratchpad_blocks.json / scratchpad.md), and the section header forbade re-reading scratchpad.md. The BIBLE P1 omission pointer could not resolve. - _render_scratchpad_for_context: the gap marker now names the omitted count and timestamp range and points at memory/scratchpad.md via read_file; the newest-block-overflow case says so explicitly. The writer's own journal-pointer line (retired/replaced blocks) is untouched: different population, different pointer. - build_memory_sections: a trimmed body gets a PARTIAL header that invites the re-read; the non-degraded header is byte-identical. - docs/ARCHITECTURE.md: one sentence on the degradation contract. - tests: marker points at the live store and never at the journal; degraded header allows re-read; non-degraded header unchanged. Co-authored-by: Ouroboros <311266734+ouroboros-agent@users.noreply.github.com>
Non-dict, non-string acceptance evidence was hard-sliced to 2000 chars with no omission note, asymmetric with the string branch that forwards the whole payload. Route the repr through truncate_review_artifact so a cut carries the OMISSION NOTE and original length; short payloads still pass through whole because of the marker floor. Co-authored-by: Ouroboros <311266734+ouroboros-agent@users.noreply.github.com>
Closed
2 tasks
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This train lands the reviewed follow-ups for community PRs #735 and #736, the MCP decision-lane capability from #722, and the shared owner-aware lock custody fix.
It preserves the contributors’ original commits and adds the Ouroboros follow-up commits. The train includes the live scratchpad budget-gap disclosure and header fix, omission-aware non-dict review evidence, MCP/extension discovery-execution parity, and the class-level owner-aware stale-lock fix.
Local validation on the exact candidate 1ed6256: full Python parallel 16,829 passed / 6 skipped; full serial 1,190 passed / 99 skipped; Node 1,032 passed; ruff F and size-ratchet checks passed. GitHub CI is running on this exact SHA; merge waits for its result.