Skip to content

DO NOT MERGE — merge train: 3333,3334,3338,3345,3347 - #3349

Closed
trinity-ability wants to merge 22 commits into
devfrom
train/20261007-1609
Closed

trinity-ability wants to merge 22 commits into
devfrom
train/20261007-1609

Conversation

@trinity-ability

Copy link
Copy Markdown
Contributor

Integration surface for #3333, #3334, #3306, #3338, #3345, #3347. Never merged; members merge individually once green.

Train-only commits on top of the members: sibling tests/registry.json unions; #3345↔#3347 doc + SQLite migration-list resolutions; #3347's Alembic revision re-parented to 0094_agent_skill_gates ← 0093_platform_alert_responded_heal (#3338) — the same re-parent goes onto #3347's own branch once #3338 is on dev.

🤖 Generated with Claude Code

alex-furman and others added 21 commits October 6, 2026 20:27
…ery to its agent (#3313)

A list/object `id` reached the unguarded `in open_by_rid` (and the write-back's
`in response_map`) and raised out of `_sync_agent`; a list/object `priority`
raised in `_clamp_ingested_item` (breaking its never-raises contract) and in
`_comparable_priority` via `changed_fields`. `_poll_cycle`'s
gather(return_exceptions=True) then dropped the error unlogged, so the agent's
answers stopped being written back every 5 s with nothing in the logs.

- Non-string id: held as invalid_id when pending, skipped otherwise, before any
  lookup; skipped in the write-back loop (it names no row).
- Priority: isinstance-guarded in the clamp and _comparable_priority (-> medium).
- Per-agent sync exceptions from the gather are logged with the agent name.

Tests: the three TestUnhashableEntryValues strict-xfails are unmarked, and new
test_3313_operator_queue_sync_error_logged drives the real _poll_cycle. With the
fix reverted, all five went red (unhashable id, priority rewrite, clamp
list/object, gather logging).

Fixes #3313

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… fault (#3316)

advance_on_terminal claims run N (CAS runs_completed N-1 -> N) and then closes
run N's row in a second write. A fault between the two (database is locked, a
process kill) left the loop claimed with the run still `running`, and every
redelivered terminal and reconcile_after_restart then lost the claim CAS, so
the loop showed "running" forever.

A delivery that loses the claim now recognises that state (loop non-terminal,
runs_completed == N, run N still running) and closes the run itself.
finalize_loop_run becomes a second CAS on status='running' and returns bool;
only the caller that wins the close runs the tail, so a repair racing a live
advance still dispatches run N+1 exactly once. No schema change.

Red before the fix: test_m56_fault_after_claim_is_recoverable_on_restart
(xfail marker removed). Red with the close CAS removed:
test_m56_repair_that_loses_the_close_does_not_dispatch.

Fixes #3316

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A platform-minted alert (a request_id with a reserved platform prefix) has
no agent audience, so the operator's answer is its last event. The answer
sink now passes terminal=is_platform_minted(item) and the respond
compare-and-set writes status='acknowledged' + acknowledged_at for those
rows. An agent's own ask still lands 'responded'. The endings ledger is
unchanged (answered / person), so the skill gate's resolver and the
Workspace views read the same ending.

Agent files written before ent#499 can still hold such an alert as a
'responded' entry, and nothing removes it. Ingestion now skips a
reserved-prefix entry that is no longer pending without the #1631
WARNING. Only a pending entry can pre-create a platform row, so that case
keeps the WARNING.

Rows already stuck in 'responded' move to 'acknowledged' once, through the
dual-track heal platform_alert_responded_heal (SQLite) and Alembic 0093.
Ids are matched the way is_platform_minted matches them, in Python,
because SQL LIKE reads the '_' in 'val_' as a wildcard.

Mutation check: with terminal=False the three answer-sink tests in
test_2372 go red; with the guard skip removed the two leftover-entry tests
go red.

Fixes #2372

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lands (#3274)

The gated-skill check (#3208) is inert until the gate map supplies a row.
The #3208 merge-train validation listed seven gaps that a row makes
reachable; this closes them, plus #3233 and the UI's handling of the 202.
It lands before or together with the gate map, never after it.

- Context scan: a schedule's name, an MCP key's name and the requester's
  email reach the executor's system prompt beside the request. The gate
  now scans them (raw and as the prompt renders them), apart from the
  request, shows them on the card when they alone matched, and never
  replays them. A parity test classifies every ExecutionContext string
  field.
- Telegram: a group reply is scanned with the quote line it carries.
  Group history, sender labels and the group title stay unscanned
  (decided; stated limit, naming the runtimes without the hook).
- Wiring: the event-loopback flag, the ending-observer registration and
  the sweep call are each pinned by a test that runs the line.
- Replay: an approved /task or fan-out run is sent the caller's message
  and system prompt apart, never their concatenation.
- MCP: the inline connector tier reads the gate result like the other
  tiers; a pending answer promises delivery only when the backend makes
  one (outcome_delivery), and the delegation contract defers to the
  message (#3233).
- Matcher: -/x, _/x_ and 1/x match; every Default_Ignorable character
  is matched removed (a run before a slash reads as a space) and spaced.
- Notices: the requester never sees the approver's email; a platform
  request reads the display name, everyone else "the agent's approver".
  A requester nobody notifies is told so and where the outcome shows.
- UI: the Chat tab, /m, the public link, the Tasks tab, Playbooks and
  Dashboard show the server's message instead of an error and poll
  nothing; held chat lines stay out of the next turn's history.

Mutation: each call site reverted from a copy turns its test red — 21
backend sites (test_3274_*, test_ent751_skill_invocation), 15 UI
sender branches (skillGateSenders / mobileAdminSkillGate), 2 MCP lines
(chat-gate.test.ts).

Fixes #3274
Fixes #3233

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stores, per agent, which skills need approval before they run and which kind
of person approves. The dispatch check (ent#751) and the in-container hook
(ent#752) read it through skill_gate_service.list_skill_gates, which stops
returning {} and reads the table — the gate is live from here.

- Storage: agent_skill_gates (agent x skill -> approver primary|approver,
  deadline 1-168h, origin set|library_default|cleared), both tracks (SQLite
  entry + Alembic 0093 on 0092), CASCADE AgentRef. A failed read raises, so
  the check refuses (gate_unavailable) instead of reading "nothing is gated".
- REST GET/PUT/DELETE /api/agents/{agent_name}/skill-gates[/{skill_name}] and
  MCP list_skill_gates / set_skill_gate / clear_skill_gate. Writes: a person
  who owns the agent or is an admin, or a skills.manage holder on an agent its
  owner owns, never on itself (require_person_or_capability, copied verbatim
  from #3236). An agent key reads its own gates; set_by is withheld from
  machine keys. Named 422/409 refusals; OSS offers only `primary`.
- Library `approval: recommended` defaults follow assignment state: applied
  before a package is delivered (existing holders backfilled at start/sync),
  removed once the skill is unassigned and its package has gone; a cleared
  default leaves a tombstone. The library can tighten, never loosen.
- Removal: an explicit gate goes only when a person unassigns the skill and
  its package removal completed. An agent's unassign, the system key, a
  deferred removal, Sync, start and the sweep keep it (gates_kept), so
  unassign-then-reassign cannot launder a gate away. Own-skill gates are
  sticky until cleared. This departs from the issue's "removing a skill
  removes its gate" for own skills and non-person removals, by the recorded
  scope decisions.
- Marker ordering (#752): the fail-closed marker is written before an
  agent's first gate (decided under the per-agent lock) and removed after
  its last; stopped agents are never exec'd.
- Audit on every change (who; via ui/api/orchestrator/system; trigger).

Tests: tests/unit/test_ent753_*.py, src/mcp-server/src/tools/skill-gates.test.ts;
every write path, auth bound and ordering line mutation-checked (each red).
Security: docs/security-reports/cso-diff-2026-10-07-ent753-skill-gate-map.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… (trinity-enterprise#753)

verify-local's unit stage (random order) found two things the per-file runs
could not:

- `write_skill_gate` marked an unsent field with a module-level `KEEP =
  object()` compared by identity. After another test re-imported `db.*`, the
  caller's KEEP and the DB module's were different objects, so the sentinel
  itself reached SQL ("type 'object' is not supported"). The write now takes
  `changes` — only the fields to set — so there is nothing to compare by
  identity and a reload cannot turn "keep" into a value.
- The service and route fixtures patched `import services.x as X`, which reads
  the package attribute; after another test's re-import that can be a stale
  copy while the code under test imports from sys.modules (learning
  2026-10-05). The fake Docker exec then missed, the real exec ran, and the
  marker tests saw no marker events. The fixtures now patch
  `importlib.import_module(...)`; reproduced with a stale package binding
  (red), then green.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#3344 item 2)

`skill_gate_requests` in database.md, `skill_gate_service.py` and
`skill_gate_map_service.py` in backend.md's service catalog, the MCP gate
result handling in mcp-server.md, and the dispatch gate's person-only endings
in security.md — each two lines or fewer, beside the gate map rows this
branch already added. Item 2 of #3344; items 1 and 3 stay open there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…able, as GET does (trinity-enterprise#753)

Found smoke-testing the routes on the verify-local sibling stack: GET reported
approver_reachable=false for an owner with no email while the PUT response's
gate carried null (only its warnings said approver_unassigned).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eQL py/overly-large-range)

CodeQL reads a character range beyond the Basic Multilingual Plane as
`�-�`, so the three such ranges sharing one class in
`_INVISIBLE` (U+1BCA0, U+1D173, U+E0000 blocks) were flagged as
overlapping (alerts 385, 386). Each now sits in a class of its own,
joined by alternation. Same characters either way: 0 of 1,111,998
non-surrogate code points match differently (4,174 invisible before and
after), and the before-slash rule's output is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…inear matcher, honest refusals (#3274)

From the full /review of PR #3345 (two independent reviewers):

- Card vs run (critical): the card sanitised the JOIN of the message and
  the caller's system prompt, while the approved run sent each sanitised
  alone. A credential pattern spanning the line between them redacted
  the system prompt from the card only, so Approve ran an instruction
  the approver never saw. The card is now built from the exact replay
  parts, with the system prompt labelled ("With these instructions as
  its system prompt:").
- Matcher: the before-slash rule was quadratic on a long run of
  invisible characters (50k took ~10 s on the event loop, reachable
  from a public-link message); a run is now matched only from its
  start. `_AFTER` regressed `/x...y` and `/x._y` against #751; the dot
  rule is #751's again, and a property test holds that every input the
  #751 matcher caught is still caught.
- /m: a refused gated request is kept out of the next message's history
  too; the notice gets its own gray-400 colour (theme token) and
  role="status".
- Playbooks: a gate refusal goes to the agent page's error toast, which
  stays until dismissed. The agent page toast carries role status/alert.
- Public link: the status route returns `gate` (held | refused); a held
  turn shows the grey notice, a refusal the red error box.
- Tests: the Workspace decider-label test now reaches its own arm;
  refusal assertions reject the JSON-rendered detail; the public-link
  spec fails on an assertion, not a timeout.

Mutation: each fix reverted from a copy turns its test red (7/7).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#3236 landed the shared `require_person_or_capability` / `_path_agent` this
branch carried a verbatim copy of. Resolved `src/backend/dependencies.py` by
keeping dev's version (with `_refuse_unless_owners_agent` and
`capability_fence`) and dropping the duplicate; `get_skill_gate_readable_agent_by_name`
stays. The gate routes' census entries still classify (test_2996 green); the
flow doc no longer calls the helper a copy.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts:
#	tests/registry.json
# Conflicts:
#	tests/registry.json
# Conflicts:
#	docs/memory/feature-flows/skill-gate.md
#	docs/memory/requirements/security.md
#	src/backend/db/migrations.py
#	tests/registry.json
…l_gates ← 0093_platform_alert_responded_heal)
@vybe vybe added the ui PR touches the frontend UI — triggers Playwright e2e tests label Oct 7, 2026
This reverts commit 1ce5642, reversing
changes made to cc26d29.
@vybe
vybe deleted the train/20261007-1609 branch October 7, 2026 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ui PR touches the frontend UI — triggers Playwright e2e tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants