Skip to content
This repository was archived by the owner on May 30, 2026. It is now read-only.

v5.7.x: four reliability fixes from a multi-agent audit (frontend leak + 3 boundary hints) - #46

Open
RobLe3 wants to merge 4 commits into
joi-lab:ouroborosfrom
RobLe3:fix/v5.7.x-reliability-bundle
Open

v5.7.x: four reliability fixes from a multi-agent audit (frontend leak + 3 boundary hints)#46
RobLe3 wants to merge 4 commits into
joi-lab:ouroborosfrom
RobLe3:fix/v5.7.x-reliability-bundle

Conversation

@RobLe3

@RobLe3 RobLe3 commented May 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Four independent reliability/UX fixes surfaced during a multi-agent audit running v5.7.1 against local OpenAI-compatible model backends. Each is small, single-concern, has direct pin tests, and does not introduce new abstractions or change semantics. Bundled in one PR because every fix is small and orthogonal — same shape as the accepted #29 bundle.

The fixes

1. web/modules/chat.js — bound chat live-card memory on long sessions

Three accumulation sites grew monotonically: record.items per task (unbounded push, dedupe-against-last only), the matching DOM nodes under record.timelineEl, and finished-task records in liveCardRecords (intentionally retained for chat history).

The dedupe-against-last fails on interleaved retry storms (e.g. alternating provider_incomplete_response + llm_usage events during a stuck-round LLM loop) — a single wedged round can push hundreds of timeline rows. Added MAX_LIVE_TIMELINE_ITEMS=200 and MAX_LIVE_CARD_RECORDS=100 caps with trimRecordItems(record) (shifts array + DOM + expandedLineKeys in lockstep) and evictOldestFinishedRecordIfFull() (only evicts finished records, removes the DOM node). Cards beyond the cap are reconstructed from progress.jsonl on page reload.

2. ouroboros/tools/registry.py — TOOL_ARG_ERROR signature hint

When the agent emits a tool call with an unexpected kwarg, missing required arg, or wrong arity, the dispatcher catches the TypeError and surfaces it as TOOL_ARG_ERROR. The bare exception message says no but doesn't say how — observed: data_write(force=True) looped without a recovery cue.

Added _format_handler_signature_hint(name, handler, err_msg) that enriches the catch site with a one-line "Valid args for : ()" reminder built from inspect.signature(handler). Triggers only on known kwarg/arity error shapes; any other TypeError raised inside a handler body gets the original message unchanged. Filters out ctx, *args, and **kwargs from the surfaced hint.

3. ouroboros/tools/git.py_repo_write_commit dirty-tree resilience (port of #36)

PR #36 fixed the checkout-on-dirty-tree class in _repo_commit_push: when checkout fails, check if we're already on branch_dev; if yes, the failure is incidental (no-op-but-git-complained on a dirty tree where the dirty files ARE what's being committed) and we proceed to staging. The parallel checkout site at _repo_write_commit:1171 (the legacy "write one file + commit" path) still aborts on any checkout failure — same trigger, same shape, not covered by #36.

Ports the already_on_target check from _repo_commit_push to _repo_write_commit. Mechanically identical pattern; only the follow-up step changes (write+stage instead of stage cycle).

4. ouroboros/tools/shell.pyrun_shell argv[0] tool-name hint (extends #37)

Smaller coder models emit malformed calls of the shape cmd=["run_shell", "cd /x && y"] — putting the literal tool name as argv[0]. subprocess returns ENOENT on the literal "run_shell" with the opaque message [Errno 2] No such file or directory: 'run_shell'. Observed: 3 identical malformed calls in one task before the agent gave up.

Same shape of fix as the merged SHELL_REGEX_HINT (#37) — boundary-level structured hint before subprocess returns its opaque error. Detects cmd[0] in {"run_shell", "shell"} and surfaces a SHELL_ARG_ERROR with a concrete suggestion: if cmd[1:] looks like a bash-string with shell metachars, suggest ["bash", "-c", "..."]; otherwise suggest dropping the tool name from argv.

Tests

  • tests/test_chat_js_contracts.py (extended) — 4 new contract tests pin the constants, the trimRecordItems shape (array+DOM+expandedLineKeys lockstep), the eviction logic (only finished records, DOM removed), and the call-site wiring.
  • tests/test_tool_arg_signature_hint.py (new) — 6 tests cover unexpected-kwarg, missing-required, multiple-values, suppression on unrelated TypeErrors, ctx-only handler, and **opts filtering.
  • tests/test_repo_write_commit_dirty_tree.py (new) — 2 tests pin the resilience path and the legitimate-failure preservation.
  • tests/test_shell_run_shell_argv0.py (new) — 5 tests cover the exact production shape, plain-argv shape, the "shell" alias, the existing builtin-rejection path (regression guard), and the lone-tool-name edge case.
  • All 17 new tests + adjacent existing tests pass on this branch.

No behavior change

No new public APIs. No new env vars. No new dependencies. No semantic changes — every fix is recovery enrichment or boundary hardening at an existing site:

Test plan

  • CI green on this PR
  • pytest tests/test_chat_js_contracts.py tests/test_tool_arg_signature_hint.py tests/test_shell_run_shell_argv0.py tests/test_repo_write_commit_dirty_tree.py -v → all green
  • Existing test suite remains green
  • Manual: noisy chat session does not balloon timeline DOM (verify via DevTools Memory tab)
  • Manual: a run_shell(cmd=["run_shell", "..."]) call surfaces SHELL_ARG_ERROR with the bash-c suggestion
  • Manual: a tool called with an unexpected kwarg surfaces the "Valid args for " hint
  • Manual: a repo_write_commit on a dirty tree where the agent is already on branch_dev proceeds to write+stage (does not abort at checkout)

🤖 Generated with Claude Code

RobLe3 and others added 4 commits May 5, 2026 11:37
Three accumulation sites in `web/modules/chat.js` made browser memory
grow monotonically over long sessions:

1. `record.items` per task — unbounded array push at the timeline
   append site. Dedupe-against-last only (not against any prior entry),
   so interleaved retry storms (e.g. alternating
   `provider_incomplete_response` + `llm_usage` events during a
   stuck-round LLM loop) defeat the dedupe and balloon `record.items`.
2. The matching DOM nodes under `record.timelineEl`, appended in
   lockstep with `record.items`.
3. Finished-task records in `liveCardRecords` Map — intentionally
   retained today (so cards stay expandable in chat history), but
   never bounded.

Fix:
- `MAX_LIVE_TIMELINE_ITEMS = 200` and `MAX_LIVE_CARD_RECORDS = 100`
  constants at the top of the IIFE.
- `trimRecordItems(record)` shifts the oldest item AND removes the
  matching first DOM child in lockstep, plus deletes the shifted
  item's `lineKey` from `record.expandedLineKeys`.
- `evictOldestFinishedRecordIfFull()` evicts the oldest *finished*
  record when the Map cap is reached (running cards untouched);
  removes the evicted DOM node so it doesn't linger detached.
- Wired into the only push site in `applyLiveCardState` and the only
  insertion site in `createLiveCardRecord`.

Tests: 4 contract tests in `tests/test_chat_js_contracts.py` pin the
constants, the helpers' shape (array+DOM+expandedLineKeys all in
lockstep), and the call sites. The trim-helper test verifies the
existence of all three cleanup steps; the evict-helper test verifies
that only finished records are evicted and the DOM node is removed.

No public API changes. No new env vars. No semantic changes. Cards
beyond the cap are reconstructed from `progress.jsonl` on page reload.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… malformations

When the agent emits a tool call with an unexpected kwarg, missing
required arg, or wrong arity, the dispatcher in
`ouroboros/tools/registry.py` catches the resulting `TypeError` and
surfaces it as `TOOL_ARG_ERROR`. The bare exception message tells the
agent what *broke* but not what would have *worked*, so smaller local
coder models loop on the same malformation without a recovery cue.

Observed shape: a consolidation task called
`data_write(path=..., content=..., force=True)` and the runtime
surfaced `_data_write() got an unexpected keyword argument 'force'`
with no list of valid kwargs. Same recovery problem as the merged
`SHELL_REGEX_HINT` (joi-lab#37) — error message says no, doesn't say how.

Fix: `_format_handler_signature_hint(name, handler, err_msg)` enriches
the catch site with a one-line "Valid args for <tool>: (<params>)"
reminder built from `inspect.signature(handler)`. Triggers only on
known kwarg/arity error shapes:
  - "unexpected keyword argument"
  - "got an unexpected keyword"
  - "missing 1 required positional argument"
  - "missing required argument"
  - "got multiple values for"
  - "takes no keyword arguments"
Any other `TypeError` raised inside the handler body (e.g. `int + str`)
gets the original message unchanged — the hint suppresses to avoid
misleading noise.

The `ctx` parameter is filtered from the hint (it's the runtime-injected
first arg the model never sees). `*args` and `**kwargs` are also skipped
so variadic params don't appear as literal arg names.

Tests: 6 in `tests/test_tool_arg_signature_hint.py` cover unexpected-kwarg,
missing-required, multiple-values, suppression on unrelated TypeErrors,
ctx-only handler suppression, and `**opts` filtering.

Universal benefit: every tool's malformed-call error gets a recovery
signpost without per-tool changes. No public API changes.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
joi-lab#36)

PR joi-lab#36 fixed the same checkout-on-dirty-tree class in `_repo_commit_push`
(now at `git.py:1303`): when checkout fails, check if we're already on
`branch_dev`; if yes, the failure is incidental (no-op-but-git-complained
on a dirty tree, where the dirty files ARE what's being committed) and
we proceed to staging. Only abort when on a different branch, where
the checkout was actually needed.

The parallel checkout site at `_repo_write_commit:1171` (the legacy
"write one file + commit" path used by the `repo_write_commit` tool)
still aborts on ANY checkout failure. Same trigger, same shape, but
not covered by the original fix.

Fix: ports the `already_on_target` check from `_repo_commit_push` into
`_repo_write_commit`. The patch is mechanically identical; only the
follow-up step changes (write+stage instead of stage cycle).

Tests: 2 in `tests/test_repo_write_commit_dirty_tree.py`:
  - `test_write_commit_proceeds_when_already_on_branch_after_checkout_failure`
    verifies the resilience path engages (write_text reached) when the
    agent is on `branch_dev` and checkout fails.
  - `test_write_commit_aborts_when_on_different_branch_with_failure`
    verifies the legitimate failure path is preserved (write_text NOT
    reached, error returned) when the agent is on a different branch.

No public API changes. No new env vars. No behavior change for the
happy path (clean checkout). Frame as "extends merged joi-lab#36 to the
sibling site."

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Smaller coder models (qwen3-coder-30b in particular) emit malformed
run_shell calls of the shape:

  {"name": "run_shell",
   "arguments": {"cmd": ["run_shell", "cd /x && y"]}}

instead of the correct:

  {"name": "run_shell",
   "arguments": {"cmd": ["bash", "-c", "cd /x && y"]}}

subprocess returns ``ENOENT`` on the literal "run_shell" with the
opaque message ``[Errno 2] No such file or directory: 'run_shell'``.
The model has no recovery cue and loops on the same shape — observed
3 identical malformed calls in a single task.

Same shape of fix as the merged `SHELL_REGEX_HINT` (joi-lab#37) — boundary-
level structured hint with a concrete fix suggestion, before
subprocess returns its opaque error.

Fix: detects `cmd[0] in {"run_shell", "shell"}` and surfaces a
`SHELL_ARG_ERROR` that:
  - Names the bad argv[0]
  - If `cmd[1:]` is one shell-string with metachars (`&&`, `|`, `>`,
    `;`, `$(`, etc.), suggests `["bash", "-c", "..."]`
  - Otherwise suggests dropping the tool name from argv
  - Adds notes about cwd= for cd, repo_read for files, code_search
    for symbol lookup

Tests: 5 in `tests/test_shell_run_shell_argv0.py`:
  - The exact production failure shape (tool name + bash-string)
  - Plain argv with the tool name accidentally prepended
  - The "shell" alias also caught
  - Real builtins (e.g. cd) still hit the existing builtin-rejection
    path (regression guard)
  - Lone tool name with no rest still caught

Fits the merged-joi-lab#37 pattern: defensive boundary at one site, narrow
trigger, no behavior change on the happy path. No public API changes.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
razzant pushed a commit that referenced this pull request May 28, 2026
The repository has no contribution guide today and the README only
mentions forks in the context of signed CI builds, so a first-time
human contributor has to reverse-engineer the contribution flow from
git history and from docs/DEVELOPMENT.md.

This change adds a deliberately short CONTRIBUTING.md that:

- Sets expectations about what makes Ouroboros unusual (self-modifying
  agent runtime, constitution-first design, enforced size budgets,
  the LLMClient SSOT, the platform_layer.py guard).
- Routes contributors to the canonical documents — BIBLE.md,
  README.md, docs/ARCHITECTURE.md, docs/DEVELOPMENT.md, and
  docs/CHECKLISTS.md — rather than restating them, per P7 (DRY).
- Documents the contributor PR flow: branch naming, Conventional
  Commits, smoke-gate expectations, CI tier behaviour for fork PRs,
  and the explicit fact that the agent's repo_commit review machinery
  (advisory + triad + scope) does NOT run on PRs.
- Lists realistic starter targets (already-filed bugs, the
  DEVELOPMENT.md-advertised module split debts, tool-description
  polish, cross-platform fixes) since the repo currently has no
  "good first issue" label.
- Cites recent merged human-authored PRs (#46, #48, #51) and the
  Cloud.ru issue series (#39#45) as concrete shape references for
  PRs and issues.

The README gains a small "Contributing" section between Philosophy
and Version History that points at CONTRIBUTING.md without
duplicating its content.

No code changes. No new tests required.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant