Skip to content

fix: copy conversation_history to prevent caller mutation - #235

Closed
atian8179 wants to merge 1 commit into
NousResearch:mainfrom
atian8179:fix/conversation-history-mutation
Closed

fix: copy conversation_history to prevent caller mutation#235
atian8179 wants to merge 1 commit into
NousResearch:mainfrom
atian8179:fix/conversation-history-mutation

Conversation

@atian8179

Copy link
Copy Markdown

Problem

run_conversation() in run_agent.py creates an alias instead of a copy of the conversation_history parameter:

messages = conversation_history or []

Since messages is the same object as the caller's list, every messages.append() during the conversation also modifies the caller's original list. This is a classic Python mutable argument pitfall.

Reported in #228.

Solution

messages = list(conversation_history) if conversation_history else []

This creates a shallow copy when a non-empty list is passed, preserving the caller's original list. When conversation_history is None or empty, a new list is created as before.

Changes

  • run_agent.py line 2572: 1 line changed

Testing

  • Shallow copy preserves message objects (no deep copy overhead)
  • None and [] inputs still produce an empty list
  • No behavioral change for the internal conversation flow

Closes #228

Replace `messages = conversation_history or []` with
`messages = list(conversation_history) if conversation_history else []`
to avoid mutating the caller's list. The previous code created an alias
to the caller's list, so every append during the conversation also
modified the original list unexpectedly.

Closes NousResearch#228
@teknium1

teknium1 commented Mar 2, 2026

Copy link
Copy Markdown
Contributor

Closing in favor of PR #229 which has the same fix plus tests. Thanks for the contribution!

@teknium1 teknium1 closed this Mar 2, 2026
rhCat added a commit to rhCat/Maria that referenced this pull request Jul 26, 2026
…th not rediscovering

A resume-here note spanning both repos (Maria + cyberware), written because most of what was learned this
session is not recoverable from the diffs: it is measurements against the live fleet and a list of things
that silently do nothing.

Records what shipped (cyberware NousResearch#232-NousResearch#235 merged, 5 commits unreleased since v1.8.0; four commits on
feat/ed25519-assertions here), what is deployed and proven (Maria authenticating with a fresh Ed25519
assertion per claim, verified offline, a 5-turn LLM round passing 4/4), and exactly where it stopped —
v1.9.0 untagged and this branch without a PR.

Blockers that need a decision or a machine unreachable from here: the alembic worker re-serving cached
evidence under the REQUESTED commit hash; cibatio rendering no judgment because INTEL_* never reaches the
engine; /run-conservation-stack exiting 4; whether selfmod should be approvable at all; and the fact that a
caged agent has no reachable approval channel — 45 push_backs, 9 approvals, all synthetic.

Also the four bugs found and fixed (the 403/409 transport collapse that had made Maria read-only; a config
key that registered nothing; .strip() truncating ~5% of binary keys; a None mirror_dir crash), and the traps
worth not paying for twice — GOVD_AUTH_VERIFIER must be env because the entrypoint regenerates govd.json;
docker-compose.maria.yml is UNTRACKED; the documented launch line has two wrong values; the worker is on
11080 and needs a commit SHA not a branch; commits need --no-gpg-sign; and `docker exec … python3 - "$ARG"
<<PY` runs nothing and exits 0.

Ends with the pattern that generated most of the work: seven times something reported success while
achieving nothing. Exit codes were honest about crashes and silent about doing nothing, so every green
result here was earned by checking content — and guard tests are mutation-verified, because a guard test
never seen failing is not yet evidence.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rhCat added a commit to rhCat/Maria that referenced this pull request Aug 1, 2026
…roved nothing

BRANCHING.md covers how to sync. This records what happened on the first one:
upstream refactored the block-decision logic out of both executors into a
single _run_agent_tool_execution_middleware, so our two gate hooks collapsed
into one call site — a tighter waist (11 dispatch paths funnel through it) that
also runs on final_args, after Relay rewrites, so ARGS_DIGEST finally covers
what actually executes.

The part worth keeping: tests/test_govern_gate.py passed 38/38 before any
conflict was resolved, and would pass with the gate deleted — it never
references tool_executor. That is cyberware NousResearch#235's shape exactly. Records the
direct wiring proof and its mutation check, and makes 'the gate is still
reached' step 3 of the next sync rather than an assumption.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rhCat added a commit to rhCat/Maria that referenced this pull request Aug 1, 2026
One govd on the VPS, many agents. Each agent authenticates as its own named
principal with its own key and capability scope; all write to one shared
ledger. The board shows one node — agents are the `principal` column on run
rows, not board entries. This is the shape governance-pm drives at
(M3-T03-retire-peragent); the per-agent govd is what that milestone retires.

Covers the govd node, turning on the ed25519 identity scheme, an identity per
agent, the container (Hermes' own provider config rather than a proxy, TUI/CLI
as the surface), and verification.

The section worth reading first is the failure mode: FIVE distinct faults all
present as "agent healthy, every tool call denied", and they are
indistinguishable from inside the container and from the agent's logs. The
node's ledger is the only discriminator — rows-all-rejected is an
AUTHORISATION problem (subject mismatch, narrow ACL, expiry); no-rows-at-all is
a WIRING problem (stale chip, auth_verifier unset, image pre-NousResearch#235, or govd
unreachable). Every one of these was hit at least once during this deployment.

Records the derivation that makes the identity step portable: the subject is
"ed25519:" + sha256(raw_pub).hexdigest()[:16], verified byte-identical against
infra.cwp.sign.keyid — so a node whose cyberware clone is older than the image
can still register a principal.

And the traps, each of which cost real time once: services bind the tailnet IP
rather than loopback (three separate services, three separate false "it's
down"); a failed credential lookup makes compose no-op while looking like a
successful deploy; deploy-node.sh overwrites run/ from etc/ and rewrites
govd.json, silently reverting auth_verifier; no agent key is ever created for
you; the agent-principal and monitor tokens are distinct and easily swapped.

The govd half has been run against a live node. The container launch and
first-claim verification have not been completed on a VPS yet, and the doc says
so.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

run_conversation() mutates the caller's conversation_history list

2 participants