fix(scheduler): stop bumping updated_at in run-time writes (#420) - #425
Merged
Merged
Conversation
`update_schedule_run_times` was setting `updated_at = now()` alongside `last_run_at` / `next_run_at`. The scheduler's periodic sync loop watches `updated_at` to detect config edits — so every sync tick saw its own write from the previous `_add_job`, flagged every schedule as "updated," and re-registered all N jobs once per minute. Logs grew linearly with fleet size (~7k "Added job" lines per 8h for 13 schedules). Fix: `updated_at` now tracks config changes only. User-initiated edits (`update_schedule`, `set_schedule_enabled`) still bump it and still trigger the sync branch correctly. Runtime writes (`last_run_at` / `next_run_at`) no longer do. Applied to three call sites: - src/scheduler/database.py: update_schedule_run_times, update_process_schedule_run_times - src/backend/db/schedules.py: update_schedule_run_times Added regression tests in tests/scheduler_tests/test_sync_loop.py: - update_run_times does not mutate updated_at (agent + process) - three consecutive sync ticks with no DB edits produce zero add_job calls - a legitimate cron edit still triggers the sync update branch Closes #420 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add the "run-time writes must not bump updated_at" invariant to Flow 6 in scheduler-service.md and flag it in the update_schedule_run_times / update_process_schedule_run_times entries. This is the contract the sync loop depends on — future maintainers touching these methods need to know. Also bump the stale line ranges in those two method entries and add an entry for #420 to the feature-flows index. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add test_sync_loop.py to the Scheduler Tests category, a 2026-04-20 entry to Recent Test Additions, and bump unit-test totals by the 4 new tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks done
vybe
added a commit
that referenced
this pull request
Sep 14, 2026
…#554) (#2623) * feat(canvas): delete, pin, search and a stated bound for the canvas pile An agent that uses its canvas the way ent#438 intends accumulates dozens: one per report, per topic, per run. The Workspace could only ever ADD to that pile — the client-portal surface had no delete at all, the only ordering was "newest updated", and nothing bounded the table. Two decisions, both by operator ruling 2026-09-08, recorded because each had a plausible alternative: **Deleting is owner-or-admin.** The answer ent#548 gives for files — the owner deletes the shared artifact. This NARROWS the platform DELETE route, which accepted any user with agent access; safe because no UI called it, so no workflow depended on the wider gate. A canvas is one shared surface with no per-user copy, so a non-owner has no "hide it from my list" middle ground: per AC #2 they see no control at all rather than one that 403s. Agents keep clearing their own (`clear_canvas`, the #918 self-gate). **The bound is a per-agent CAP, not a retention window.** ent#438 recorded "no retention window" because the composite key bounds rows per canvas — but `canvas_id` is agent-chosen, so the COUNT was unbounded; the axis was missed, not decided. `CANVAS_MAX_PER_AGENT` (100, env-tunable) is checked inside `upsert_canvas`'s insert branch, in the same transaction as the INSERT, so it is not a check-then-act race. Updating an existing canvas is NEVER refused — a cap that froze updates would punish exactly the agent that reuses ids — and the refusal is a named 409 telling the agent to retire one, never an eviction: deleting a person's surfaces on a timer is the #1638 failure direction. Both surfaces resolve permission through `db.can_user_share_agent`, the same predicate `assert_agent_owner` uses, so Agent Detail and the Workspace cannot disagree about who owns an agent. The Workspace learns it from `PortalAgentCard.can_manage_canvases` — the portal's only capability channel (#2128), since a portal principal cannot read `/api/settings/feature-flags` — and it fails closed. `pinned` (dual-track: `agent_canvases_pinned` + Alembic 0058, NOT NULL DEFAULT 0, no backfill) is written only by the human pin route and is deliberately absent from every agent-facing tool: `audience` is the agent's decision about who may read, `pinned` is the reader's about what they see first, and an agent that could pin itself to the top would defeat the ordering. A pin survives the agent rewriting the canvas. Living with many is `CanvasPanel.vue`, shared by both surfaces (one rendering layer, per ent#475): search once the list passes six, a height-bounded strip so a long list does not cost the rail its other tabs, and a Manage mode giving each row its age, stale mark, pin and delete. Decidable rules are pure in `canvasUtils.js` — vitest runs `environment: 'node'` with no mount harness, so a rule inside the SFC is one no test can reach. Bulk delete is a POST, not a body-carrying DELETE (bodies on DELETE are permitted-but-unreliable and this one is not optional), declared above the parameterized routes on both routers (Invariant #4), and it reports the ids that EXISTED rather than the ids requested so "3 of 5 removed" is sayable. Three pre-existing guards failed and each was right to: `empty_canvas` was missing the new field (a real bug in this change, fixed), the self-gate guard needed to learn the new gate's name, and the positional-read guard needed its synthetic row extended — that one exists precisely because `_row_to_summary` reads by index. Tests: 20 new backend cases (cap refusal + update-at-cap, pin ordering and survival, bulk scoping, the permission matrix on both surfaces, route ordering, dual-track migration parity, and that the MCP tools cannot pin) and ~24 vitest cases for the pure rules. Verified against a real database: the cap refuses the 4th of 3, updates still succeed at the cap, a pin outranks recency and survives a rewrite, and bulk delete returns only the ids that existed. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * feat(canvas): audit the single delete, cover the default canvas, document the lifecycle Closes the three acceptance criteria the first commit left open: * AC #1 asked for the deletion to be audited and only the BULK route was — the single-canvas route is the one a person actually clicks. Logged only when something was removed, since the route is idempotent and a repeat click would otherwise fill the trail with events where nothing happened. * AC #8: deleting the DEFAULT canvas is allowed, comes back empty on the next write, and frees a slot against the cap. `main` is the id both the MCP tools and the voice panel fall back to, so it is the one most likely to be deleted by accident and the one whose deletion must strand nobody. * AC #9: the user doc gains a "Removing canvases" section stating the permission rule, the cap, and that nothing is ever deleted to make room. Also records a latent pre-existing mismatch found while testing: `empty_canvas` returns None timestamps while `models.Canvas` requires strings, so `Canvas(**empty_canvas(...))` raises. Not live — its only caller declares no `response_model` — but adding one there would turn the voice teardown poll into a 500. Left as a comment where the next person to reach for that will meet it, rather than fixed out of scope. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * feat(canvas): share a canvas at a link, and download it as a PDF A canvas is where an agent's real output lives, and until now it could not leave the Workspace: no share link, no export, nothing in the tree that renders a PDF. **Sharing never widens the audience by accident.** Two scopes, and the default is the narrow one: `authorized` makes the link a DEEP link — opening it requires signing in and the server re-checks `can_user_access_agent`, so it reaches "the people who could already see it" and nobody else. `public` is an explicit, separate, audited choice. Failing narrow is enforced in five independent places (column default, Pydantic default, `normalize_scope`'s fallback, the order of `SHARE_SCOPES`, the radio the dialog preselects), because a link that reaches further than the sharer understood is the one failure this feature must not have. **`agent_canvas_shares` is deliberately its own table.** `agent_public_links` has a `type` column that looks purpose-built for this, and reusing it would have been a real vulnerability: nothing in that table's read path filters on type — `get_public_link_by_token`, `is_link_valid` and `routers/public.py::_validate_public_link` all resolve a token whatever it is — so a canvas row there would ALSO be a working public-CHAT token, and anyone sent a canvas could talk to the agent. (`type='site'` is the same trap already laid; it is unexploited only because nothing creates those rows today.) A separate table makes the isolation structural instead of dependent on every consumer remembering to check. **Live, and it says so** (AC #3, operator ruling): the link renders the canvas as it is now, carrying its `updated_at` and stale mark, and the page states it is not a copy taken at share time. That follows ent#438's model — a canvas is a surface an agent keeps current — and means a share stores nothing. The cost is drift after sharing; the mitigation is revocation, not freezing. **Revocation keeps the row.** `revoked_at` is stamped, never deleted, because a revoked link has to be able to SAY it was revoked (AC #2), which it cannot do once the row is gone. The status vocabulary splits along disclosure: `revoked` and `expired` are returned only for a token that MATCHED a row — whoever holds such a link was already told the canvas exists — while an unknown token and a canvas deleted out from under a link both collapse into one `not_found`, so a stranger guessing tokens learns nothing from the difference. An unparseable `expires_at` reads as expired: a lifetime we cannot read is one we cannot promise is live. **PDF is print-first**, the path the issue recommended: a print stylesheet plus the browser's own PDF. No headless service to run, and — the deciding reason — no second renderer to keep in step with `CanvasBlock`. A server-side renderer was the stated fallback and was not needed: pagination (`break-inside: avoid` per block) and fidelity both fall out of the same markup the screen uses. `CanvasDocument.vue` is the one printable form, rendered by both the shared page and every authenticated surface, so AC #7's "identical from every canvas surface" is true by construction rather than by three surfaces agreeing. It carries title, agent and generation date, forces the light rendering under `@media print`, and when `window.print` is unavailable the control says so and the share link still works. `ent#425` (hosted deliverable pages) is still open, so per this issue's own boundary rule this ships the narrower share link and #425 adopts it later. Verified against a real database, not stubs: the full resolution matrix (public/anonymous → ok, authorized/anonymous → sign-in, authorized/owner → ok, authorized/stranger → refused, unknown → not-found, revoked, expired, unparseable expiry → expired, canvas deleted → not-found), views counted only on a successful render, cross-agent revoke refused, and a second revoke keeping the first revocation time. 23 backend tests, 20 vitest cases for the pure rules. Related to Abilityai/trinity-enterprise#554 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * test(canvas): patch the cap where the live code reads it, not where the test imports it Two ent#553 tests passed in isolation and failed in a full-suite run: `test_the_cap_refuses_a_new_canvas_by_name` and `test_deleting_the_default_canvas_frees_a_slot_against_the_cap`. Order-dependence, not a defect in the feature. They patched `db.canvas.CANVAS_MAX_PER_AGENT` via a fresh `import db.canvas`. Some earlier test in the suite evicts that module from `sys.modules`, so the fresh import hands back a NEW module object while the live `db._canvas_ops` is still an instance of the OLD class — whose `upsert_canvas` reads the OLD module's globals. The patch lands somewhere nothing consults, the cap stays at its default of 100, and the "refuses the 4th of 3" assertions fail. `_set_cap` patches the bound method's own `__globals__`, which is whichever module dict the running code actually closes over — correct whether or not an eviction happened, so it does not depend on knowing which test pollutes. Same failure and same fix as #2589, where the identical shape bit `mark_stale_activities_failed`. Worth noting the class: a monkeypatch on a module attribute is only as good as the assumption that the live object came from that module object, and in a suite that evicts modules that assumption is not free. The feature is unchanged — this touches only the test file. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * fix(canvas): print the canvas, not the whole page Reported from hands-on testing: pressing PDF offered to export the entire page. Correct report — the print stylesheet only STYLED the document and never hid anything else, so `window.print()` printed the nav bar, the tabs, the on-screen panel AND the print copy. That is not "one clean column" by any reading (AC #4), and it is the first thing anyone pressing the button hits. Two halves, both required: * a print rule that hides every `body` child except `.canvas-print-root`; * the print copy TELEPORTED to <body>, so it is a body child and the rule can spare it. Nested inside the app the rule would hide its ancestor and print nothing at all — worse than the bug. `body > *` rather than a class on the app root: it needs no knowledge of how the app is mounted and works identically on the standalone shared page, which keeps AC #7's "identical from every surface" true rather than approximately true. The copy is rendered only while printing (`v-if="printing"` + a `nextTick` flush before `print()`, since printing a not-yet-rendered teleport yields a blank sheet), so the DOM carries no permanent hidden duplicate. `canvasPrintIsolation.spec.js` pins all three structural facts. Nothing automated can inspect a print preview, which is exactly why the bug shipped — so the guard asserts the mechanism instead: the hiding rule exists, the root is teleported to body, and the document mounts before print() is called. Mutation-tested: removing the hiding rule fails it. Also fixes a design-system violation the ratchet caught in the same file: `SharedCanvas` gated its skeleton on a bare `v-if="loading"`. Now `viewState()` — loading means "no data yet", never "a fetch is in flight" (#1927, design-system p13-p15). The page fetches once today, so this is the rule holding rather than a bug fixed; it stays correct if a refresh is added. Baselining my own new violation was the alternative and would have been the wrong one. Related to Abilityai/trinity-enterprise#554 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * fix(canvas): re-parent the pinned revision, and make the canvas cap reachable Two review items from #2619. **Alembic head fork (#2068 class).** `0058_agent_canvases_pinned` shared `down_revision = 0057` with #2608's `0058_portal_file_dismissals`, which has since landed on `dev` — two heads, and `alembic upgrade head` resolves its single target before applying anything, so EVERY revision merged since the fork stops arriving, not just one. Re-parented onto `0058_portal_file_dismissals` and renumbered to `0059` so the prefix keeps being a usable ordering cue; the id is not applied anywhere yet, so the rename costs nothing. `check_alembic_heads.py` reports 1 head. **`CANVAS_MAX_PER_AGENT` was inert (#1039 class).** The refusal message names the number, but the variable was read only from `os.getenv` in `models.py` and appeared in no compose file — so an operator following the refusal's own advice would raise a lever that never reaches the container. Wired into `docker-compose.yml`, `.prod.yml` and `.hosted.yml` (the last two launch standalone, no base merge / no `env_file`) plus `.env.example`. Related to #2619 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP * chore(design-system): re-freeze CanvasPanel's raw-gray ceiling for ent#553 The raw-colour ratchet became enforceable on dev while this branch was open (#2605/#2609), and the merge brings it here: this PR's delete/pin/ search chrome takes `components/canvas/CanvasPanel.vue` from 25 to 46 `raw_gray`, so `tests/unit/rawColorRatchet.spec.js` fails the frontend build. That growth is the honest kind. The design-system contract SPELLS the neutral ink ladder as `gray-N` — surfaces gray-50/100/800/900, borders gray-200/300/700/800, ink gray-300/400/500/600 — and there is no semantic token for a neutral, which is exactly why the spec's own comment says gray is ratcheted but never held to zero for new files. The rule it does hold new code to is `raw_nongray`, and this file stays at **0**. Re-frozen in its OWN commit with the increase named in the baseline's `refrozen` block, which is what the ratchet's error message asks for — not absorbed silently into the feature diff. The entry is hand-edited rather than regenerated so #2605's provenance block survives; no other file's ceiling moves (verified: nothing grew, nothing is stale, no un-baselined file carries `raw_nongray`). Related to #553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP * fix(canvas): state the bound, audit the Workspace writes, gate Manage on ownership (ent#553) Three review findings, all in the same direction — the backend was right and the user-facing half did not arrive — plus the two smaller ones. 1. **The stated bound now reaches the user.** `canvasLimit` was a `ref(0)` nothing ever assigned, so `canvasHeadroom(n, 0)` returned `{label: null}` and the early warning could not render at any count. The ceiling rides `GET /api/settings/feature-flags` as `canvas_max_per_agent` — the established home for a value the browser needs to render a surface, and where `platform_default_model` / `install_source` already set the precedent for a non-boolean. Not a new route (Invariant #13 would owe three surfaces for one integer) and not an envelope around the canvas list (the MCP tool and the Workspace both read it as a bare array). It is a CONSTANT, not per-agent state, and the client already holds the count. `0` still means "not told" and still renders nothing, so an older backend is unchanged. 2. **The Workspace canvas writes are audited.** The three portal routes recorded nothing while their operator twins have logged since they shipped, and `docs/user-docs/agents/agent-canvas.md` tells users deletion is audited — so the claim was false for exactly the client-facing surface. `_audit_canvas_change` is the shared helper; the actor is `actor_email` (the documented #848 inline-auth path) rather than a fabricated `User`, which is honest because `_require_canvas_manager` is platform-only and owner-or-admin, so a real Trinity user is always behind it. Ids and counts only (G-04). The three routes become `async def` to await it, matching their operator twins, which already call the same sync db functions from an async handler. Pinning is audited too, on BOTH surfaces — the operator route was the one recording nothing. A pin decides which canvas an entire roster sees first, so it is an administrative act on a shared surface, not a per-viewer preference. 3. **`canManage` comes from the parent.** It was hardcoded `true` on the argument that the server decides. It does — but a merely-shared user was then shown Manage → Delete / Pin and got a 403, which is the failing-control problem `can_manage_canvases` exists to prevent on the Workspace. Agent Detail passes `agent.can_share`, the same predicate `ReportsPanel` five lines above already reads and the same one `_gate_human_removal` enforces. The prop defaults FALSE, so a caller that forgets it hides an affordance rather than offering one that refuses. 4. **`get_agent_card` resolves `can_manage_canvases`.** It omitted it, so the same owner read `true` in the sidebar and `false` on the agent's own page — the disagreement #2160's own docstring says that function exists to prevent. 5. **An agent genuinely cannot pin its own canvas now.** The user doc said so; `_gate_human_removal` allowed it (right for delete — an agent tidying up after itself — and wrong for pin), and "no MCP tool exposes it" is a property of the client, not of the route. `_gate_pin` is humans-only, which makes the documented sentence true rather than aspirational. Tests: the audit guard now walks the portal routes as well as `routers.canvas` (it only ever inspected the latter, which is why three unaudited routes passed it), plus pin-audit parity, the humans-only pin gate beside the still-permitted agent self-delete, the feature-flags constant being the same object the refusal is raised from, the agent-card/roster agreement, and four frontend wiring cases. 1025 backend / 2538 frontend tests green. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RSjVEjay9ztC1oDXXkh9uN * fix(canvas): the Workspace audit names the operator, not the platform (ent#553) Found re-reviewing my own audit fix. Adding the rows was right; the attribution was wrong, and a row that lands under the wrong actor is worse than the missing row it replaced — nothing fails, so the wrong answer is believed. `_audit_canvas_change` passed `actor_email` only. But `platform_audit_service._resolve_actor` derives `actor_type` from `actor_user` / `actor_agent_name` / `mcp_scope` / `mcp_key_id` and never from the email, so an email-only call falls through to its last branch: _resolve_actor(None, None, None, None) -> ("system", "trinity-system", None) So every Workspace canvas delete and pin was recorded as `actor_type="system"`, `actor_id="trinity-system"` — a named operator's action attributed to the platform, invisible to any `actor_type=user` query and to the audit UI's per-actor filter. Verified against the real resolver, not by reading the call. The `actor_email`-only path I cited (#848 inline auth) is right where the caller genuinely has no `users` row. That is not this route: `_require_canvas_manager` is platform-only and resolves through `db.can_user_share_agent`, so a row exists by construction. It now resolves that row and passes `actor_user`, producing the same `("user", <id>, <email>)` shape the operator twin has always written — which is the point, since auditing the two surfaces differently buys little more than auditing one of them. Best-effort by construction: the action has already happened, so a lookup that raises or misses must not drop the row. It falls back to the email-only call with a WARNING, since a miss would mean the gate admitted someone the user table does not know. Tests: the regression is pinned against the REAL `_resolve_actor` (both the shape the fix must not return to and the shape it produces now), plus a source guard that the helper resolves a row, passes `actor_user`, keeps the email as a fallback and cannot raise. Removing `actor_user=` reds it. 31 passed on the ent#553 file; 953 across canvas / portal / audit. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RSjVEjay9ztC1oDXXkh9uN * fix(migrations): chain 0060_agent_canvas_shares off the renamed pinned revision ent#553 renamed its revision 0058_agent_canvases_pinned -> 0059 when it absorbed dev's 0058_portal_file_dismissals; this revision still pointed at the old id, so after the merge the directory resolved to two heads and `alembic upgrade head` would have applied nothing. Renumbered to 0060 as well so the numeric prefix stays a unique ordering cue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * chore(frontend): re-freeze CanvasPanel.vue raw_gray 46 -> 62 for ent#554 The share/PDF controls add gray chrome copied from the panel's existing header; the branch predates the #2605 ratchet, so the guard first bit when dev was merged in. Scoped to this one entry, in its own commit, as the guard's own message prescribes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * test(canvas): resolve CanvasLimitExceeded from the live method's globals The two cap tests imported the class from `db.canvas` while `_set_cap` already patches the cap through `upsert_canvas.__globals__` — because an earlier test can evict and re-import the module. The same eviction gives the test a different class object than the one the live code raises, and `pytest.raises` then reports the correct refusal as an unexpected exception. Seen once in a full local run after the dev merge (both tests pass in isolation and under CI's three seeds); resolve the class from the same globals the cap comes from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * fix(canvas): keep the selector visible when a search narrows to one match (ent#553 review) `CanvasPanel.vue` gated the chip strip on `visible.length > 1 || manage`, where `visible` is the FILTERED list. Searching down to exactly one canvas hid the strip while the previously selected canvas stayed on screen, and the auto-select watcher — keyed off the unfiltered `props.canvases` — never selected the match. Proven by execution: 7 canvases, query "Topic 3" → strip false, no-match message false. The one canvas the user just searched for was unreachable. Fix: - `canvasSelectorVisible({visible, manage, query})` — with a query, any hit shows the strip; without one, a single canvas is no choice (unchanged). - `canvasAutoSelect(visible, selectedId, query)` — while a query is active the selection follows the matches; no-op with no query or when the current selection already matches. - `CanvasPanel.vue` consumes both: `v-if="selectorVisible"` and a watcher on `[visible, query]`. Tests: - `canvasUtils.spec.js`: the two pure rules. - `canvasPanelSelectorGate.spec.js`: slices the `selectorVisible` computed out of the SFC and RUNS it against the ejection's numbers; pins that the template reads the computed, not a re-derived length test, and that the watcher calls `canvasAutoSelect`. - `test_ent553_canvas_lifecycle.py`: the second review ask — the per-agent cap reaches the wire as a 409 through the real router → service → db chain (only the Redis rate limiter stubbed), names the remedy, and the same PUT against an existing id stays an update. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * fix(canvas): the search box outlives a shrink below the threshold (ent#553 review) `query` has exactly one writer — the search input's `v-model` — and that input was `v-if="showSearch"` with `showSearch = ordered.length > 6`. Seven canvases, type "Topic 3", delete the one match: six canvases, the box unmounts, `visible` still filters on the stale query, the strip collapses, and the panel says *No canvas matches "Topic 3"* with no control left to clear it. Every remaining canvas is unreachable via the chips until navigation. Also reachable with no operator action: the agent's own `clear_canvas` plus a rail refresh while a query is typed. The rule is pure — `canvasSearchVisible(count, threshold, query)` — and keeps the box while a query is active regardless of the count: the typed intent survives the shrink, and the no-match line keeps the one control that clears it. Resetting `query` when the box would flip off was the other option and was rejected: it erases a search the user was mid-way through because a sibling canvas went away. The gate spec that pinned the previous ejection drove `visible`/`query` in isolation from `showSearch`, which is why it could not see this one. It now slices the real `showSearch` computed out of the SFC and RUNS it against the ejection's own numbers (7 → 6 with "Topic 3" typed → box stays; 6 with no query → box gone), and pins that the input is gated on that computed and is the sole writer of `query`. Mutation-checked: reverting the gate to the old length test reds three cases. Four mechanical items from the same review ride along: - requirements/core-agent.md: the ent#438 "deliberately no retention window: bounded by construction" line now says why that reasoning was wrong (rows are bounded per canvas, the count was not) and what bounds it instead; FR-18..FR-22 record delete / bulk / cap / pin / search, which had no requirements entries at all. - raw-color-baseline.json: the `_ent553_note` naming CanvasPanel.vue's 25 → 46 raw_gray was added in 2794388 and dropped by the dev merge aa248f7; re-added so the growth is named in the file. - routers/canvas.py `# mcp:` header now says pin and bulk-delete are unexposed on purpose, and why — Invariant #13's deliberate-vs-forgotten signal. - feature-flows/agent-canvas.md: the two search-state rules and the defect class they close. Verified: vitest 2696 passed (121 files); canvas backend suites 101 passed; raw-colour ratchet and loading-gate ratchet unchanged. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RS8FBA7oEtAAv6GamZKong * fix(canvas): a share link is a grant, so only a human may mint one (ent#554 review) `create_canvas_share`'s docstring said "Owner-or-admin and human-only via `_gate_human_removal`". That gate is not human-only — its own docstring, one screen above, says an agent-scoped key may act on its own agent, which is correct for `clear_canvas` ("an agent tidying up after itself") and wrong for every verb that decides what someone OTHER than the agent may see. So a prompt-injected agent could POST /api/agents/<self>/canvas/<id>/share {"scope": "public"} with the TRINITY_MCP_API_KEY already in its container and publish its own canvas at an unauthenticated URL. Three things make that worse than it first reads: * the share is LIVE, not a snapshot, so one link is a self-updating channel rather than a one-time disclosure; * the agent is the only writer of canvas blocks, so anything it can read it can copy into a canvas and publish; * `audience` is not consulted on the share path, so ent#438's fail-closed "a canvas reaches a client only because the agent said so" would not have applied — the agent would have been choosing for itself. `list_canvas_shares` had the same gate and returns the TOKEN, which is the capability itself; `revoke_canvas_share` too, so an agent could also turn off a person's link. The fix is the grant-vs-use line (Invariant #8): the endpoint that USES a capability may be agent-callable, the one that GRANTS one is human-only. * `_gate_human_only(current_user, name, *, agent_detail)` is factored out of `_gate_pin` — the predicate was always right, only its NAME described one caller. A gate named for a verb ("removal") is one a fourth caller reaches past by accident; a gate named for its rule is not. `_gate_pin` and the new `_gate_share` both delegate to it, with per-caller refusal text because an agent reads that message to decide what to do next. * The three share routes now call `_gate_share`. * The delete routes deliberately KEEP `_gate_human_removal`, and a test guards that boundary in the other direction — the first attempt at this fix swept `clear_canvas` into the human-only gate, because one `str.replace` matched both bodies. That would have broken a real MCP tool for every agent: a security fix breeding the next bug, the /review §4.14 class. Six regression tests; four of them fail against the previous commit (the other two are the over-correction guards, which must pass both ways by design). The 23 tests already here covered scope defaults, expiry, revocation and enumeration, but none used an agent principal on any share route — which is how this shipped. Docs: the user doc now states that sharing is the owner's alone and that the routes refuse an agent's own key, beside the same sentence for pin; the flow doc records the decision, the blast radius, and why the delete routes stay permissive. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ * docs(canvas): record the share routes in the file's own mcp: convention (ent#554 review) The header comment lists which canvas routes are deliberately NOT exposed as MCP tools and why. ent#554 added three that qualify — minting, listing and revoking a share link — and the list did not grow with them. Worth more than a comment here: the ent#553 entry states the rule the share routes then failed to follow ("no tool exposes it" is a property of the client), so a reader consulting this header to decide a fourth route's gate would have found the reasoning but not the precedent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ * docs(learnings): a gate named after a verb gets reached for by the wrong route (ent#554 review) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Eugene Vyborov <eugene@ability.ai>
vybe
added a commit
that referenced
this pull request
Sep 14, 2026
#2628) * feat(canvas): delete, pin, search and a stated bound for the canvas pile An agent that uses its canvas the way ent#438 intends accumulates dozens: one per report, per topic, per run. The Workspace could only ever ADD to that pile — the client-portal surface had no delete at all, the only ordering was "newest updated", and nothing bounded the table. Two decisions, both by operator ruling 2026-09-08, recorded because each had a plausible alternative: **Deleting is owner-or-admin.** The answer ent#548 gives for files — the owner deletes the shared artifact. This NARROWS the platform DELETE route, which accepted any user with agent access; safe because no UI called it, so no workflow depended on the wider gate. A canvas is one shared surface with no per-user copy, so a non-owner has no "hide it from my list" middle ground: per AC #2 they see no control at all rather than one that 403s. Agents keep clearing their own (`clear_canvas`, the #918 self-gate). **The bound is a per-agent CAP, not a retention window.** ent#438 recorded "no retention window" because the composite key bounds rows per canvas — but `canvas_id` is agent-chosen, so the COUNT was unbounded; the axis was missed, not decided. `CANVAS_MAX_PER_AGENT` (100, env-tunable) is checked inside `upsert_canvas`'s insert branch, in the same transaction as the INSERT, so it is not a check-then-act race. Updating an existing canvas is NEVER refused — a cap that froze updates would punish exactly the agent that reuses ids — and the refusal is a named 409 telling the agent to retire one, never an eviction: deleting a person's surfaces on a timer is the #1638 failure direction. Both surfaces resolve permission through `db.can_user_share_agent`, the same predicate `assert_agent_owner` uses, so Agent Detail and the Workspace cannot disagree about who owns an agent. The Workspace learns it from `PortalAgentCard.can_manage_canvases` — the portal's only capability channel (#2128), since a portal principal cannot read `/api/settings/feature-flags` — and it fails closed. `pinned` (dual-track: `agent_canvases_pinned` + Alembic 0058, NOT NULL DEFAULT 0, no backfill) is written only by the human pin route and is deliberately absent from every agent-facing tool: `audience` is the agent's decision about who may read, `pinned` is the reader's about what they see first, and an agent that could pin itself to the top would defeat the ordering. A pin survives the agent rewriting the canvas. Living with many is `CanvasPanel.vue`, shared by both surfaces (one rendering layer, per ent#475): search once the list passes six, a height-bounded strip so a long list does not cost the rail its other tabs, and a Manage mode giving each row its age, stale mark, pin and delete. Decidable rules are pure in `canvasUtils.js` — vitest runs `environment: 'node'` with no mount harness, so a rule inside the SFC is one no test can reach. Bulk delete is a POST, not a body-carrying DELETE (bodies on DELETE are permitted-but-unreliable and this one is not optional), declared above the parameterized routes on both routers (Invariant #4), and it reports the ids that EXISTED rather than the ids requested so "3 of 5 removed" is sayable. Three pre-existing guards failed and each was right to: `empty_canvas` was missing the new field (a real bug in this change, fixed), the self-gate guard needed to learn the new gate's name, and the positional-read guard needed its synthetic row extended — that one exists precisely because `_row_to_summary` reads by index. Tests: 20 new backend cases (cap refusal + update-at-cap, pin ordering and survival, bulk scoping, the permission matrix on both surfaces, route ordering, dual-track migration parity, and that the MCP tools cannot pin) and ~24 vitest cases for the pure rules. Verified against a real database: the cap refuses the 4th of 3, updates still succeed at the cap, a pin outranks recency and survives a rewrite, and bulk delete returns only the ids that existed. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * feat(canvas): audit the single delete, cover the default canvas, document the lifecycle Closes the three acceptance criteria the first commit left open: * AC #1 asked for the deletion to be audited and only the BULK route was — the single-canvas route is the one a person actually clicks. Logged only when something was removed, since the route is idempotent and a repeat click would otherwise fill the trail with events where nothing happened. * AC #8: deleting the DEFAULT canvas is allowed, comes back empty on the next write, and frees a slot against the cap. `main` is the id both the MCP tools and the voice panel fall back to, so it is the one most likely to be deleted by accident and the one whose deletion must strand nobody. * AC #9: the user doc gains a "Removing canvases" section stating the permission rule, the cap, and that nothing is ever deleted to make room. Also records a latent pre-existing mismatch found while testing: `empty_canvas` returns None timestamps while `models.Canvas` requires strings, so `Canvas(**empty_canvas(...))` raises. Not live — its only caller declares no `response_model` — but adding one there would turn the voice teardown poll into a 500. Left as a comment where the next person to reach for that will meet it, rather than fixed out of scope. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * feat(canvas): share a canvas at a link, and download it as a PDF A canvas is where an agent's real output lives, and until now it could not leave the Workspace: no share link, no export, nothing in the tree that renders a PDF. **Sharing never widens the audience by accident.** Two scopes, and the default is the narrow one: `authorized` makes the link a DEEP link — opening it requires signing in and the server re-checks `can_user_access_agent`, so it reaches "the people who could already see it" and nobody else. `public` is an explicit, separate, audited choice. Failing narrow is enforced in five independent places (column default, Pydantic default, `normalize_scope`'s fallback, the order of `SHARE_SCOPES`, the radio the dialog preselects), because a link that reaches further than the sharer understood is the one failure this feature must not have. **`agent_canvas_shares` is deliberately its own table.** `agent_public_links` has a `type` column that looks purpose-built for this, and reusing it would have been a real vulnerability: nothing in that table's read path filters on type — `get_public_link_by_token`, `is_link_valid` and `routers/public.py::_validate_public_link` all resolve a token whatever it is — so a canvas row there would ALSO be a working public-CHAT token, and anyone sent a canvas could talk to the agent. (`type='site'` is the same trap already laid; it is unexploited only because nothing creates those rows today.) A separate table makes the isolation structural instead of dependent on every consumer remembering to check. **Live, and it says so** (AC #3, operator ruling): the link renders the canvas as it is now, carrying its `updated_at` and stale mark, and the page states it is not a copy taken at share time. That follows ent#438's model — a canvas is a surface an agent keeps current — and means a share stores nothing. The cost is drift after sharing; the mitigation is revocation, not freezing. **Revocation keeps the row.** `revoked_at` is stamped, never deleted, because a revoked link has to be able to SAY it was revoked (AC #2), which it cannot do once the row is gone. The status vocabulary splits along disclosure: `revoked` and `expired` are returned only for a token that MATCHED a row — whoever holds such a link was already told the canvas exists — while an unknown token and a canvas deleted out from under a link both collapse into one `not_found`, so a stranger guessing tokens learns nothing from the difference. An unparseable `expires_at` reads as expired: a lifetime we cannot read is one we cannot promise is live. **PDF is print-first**, the path the issue recommended: a print stylesheet plus the browser's own PDF. No headless service to run, and — the deciding reason — no second renderer to keep in step with `CanvasBlock`. A server-side renderer was the stated fallback and was not needed: pagination (`break-inside: avoid` per block) and fidelity both fall out of the same markup the screen uses. `CanvasDocument.vue` is the one printable form, rendered by both the shared page and every authenticated surface, so AC #7's "identical from every canvas surface" is true by construction rather than by three surfaces agreeing. It carries title, agent and generation date, forces the light rendering under `@media print`, and when `window.print` is unavailable the control says so and the share link still works. `ent#425` (hosted deliverable pages) is still open, so per this issue's own boundary rule this ships the narrower share link and #425 adopts it later. Verified against a real database, not stubs: the full resolution matrix (public/anonymous → ok, authorized/anonymous → sign-in, authorized/owner → ok, authorized/stranger → refused, unknown → not-found, revoked, expired, unparseable expiry → expired, canvas deleted → not-found), views counted only on a successful render, cross-agent revoke refused, and a second revoke keeping the first revocation time. 23 backend tests, 20 vitest cases for the pure rules. Related to Abilityai/trinity-enterprise#554 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * test(canvas): patch the cap where the live code reads it, not where the test imports it Two ent#553 tests passed in isolation and failed in a full-suite run: `test_the_cap_refuses_a_new_canvas_by_name` and `test_deleting_the_default_canvas_frees_a_slot_against_the_cap`. Order-dependence, not a defect in the feature. They patched `db.canvas.CANVAS_MAX_PER_AGENT` via a fresh `import db.canvas`. Some earlier test in the suite evicts that module from `sys.modules`, so the fresh import hands back a NEW module object while the live `db._canvas_ops` is still an instance of the OLD class — whose `upsert_canvas` reads the OLD module's globals. The patch lands somewhere nothing consults, the cap stays at its default of 100, and the "refuses the 4th of 3" assertions fail. `_set_cap` patches the bound method's own `__globals__`, which is whichever module dict the running code actually closes over — correct whether or not an eviction happened, so it does not depend on knowing which test pollutes. Same failure and same fix as #2589, where the identical shape bit `mark_stale_activities_failed`. Worth noting the class: a monkeypatch on a module attribute is only as good as the assumption that the live object came from that module object, and in a suite that evicts modules that assumption is not free. The feature is unchanged — this touches only the test file. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * fix(canvas): print the canvas, not the whole page Reported from hands-on testing: pressing PDF offered to export the entire page. Correct report — the print stylesheet only STYLED the document and never hid anything else, so `window.print()` printed the nav bar, the tabs, the on-screen panel AND the print copy. That is not "one clean column" by any reading (AC #4), and it is the first thing anyone pressing the button hits. Two halves, both required: * a print rule that hides every `body` child except `.canvas-print-root`; * the print copy TELEPORTED to <body>, so it is a body child and the rule can spare it. Nested inside the app the rule would hide its ancestor and print nothing at all — worse than the bug. `body > *` rather than a class on the app root: it needs no knowledge of how the app is mounted and works identically on the standalone shared page, which keeps AC #7's "identical from every surface" true rather than approximately true. The copy is rendered only while printing (`v-if="printing"` + a `nextTick` flush before `print()`, since printing a not-yet-rendered teleport yields a blank sheet), so the DOM carries no permanent hidden duplicate. `canvasPrintIsolation.spec.js` pins all three structural facts. Nothing automated can inspect a print preview, which is exactly why the bug shipped — so the guard asserts the mechanism instead: the hiding rule exists, the root is teleported to body, and the document mounts before print() is called. Mutation-tested: removing the hiding rule fails it. Also fixes a design-system violation the ratchet caught in the same file: `SharedCanvas` gated its skeleton on a bare `v-if="loading"`. Now `viewState()` — loading means "no data yet", never "a fetch is in flight" (#1927, design-system p13-p15). The page fetches once today, so this is the rule holding rather than a bug fixed; it stays correct if a refresh is added. Baselining my own new violation was the alternative and would have been the wrong one. Related to Abilityai/trinity-enterprise#554 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * feat(canvas): the open canvas is shared context for the turn When a user with a canvas on screen says "add a column to this", the agent now knows which canvas they mean. Before this the turn carried the message and nothing about the surface around it, so the agent asked, guessed, or minted a new canvas beside the one being looked at. The mechanism is a per-turn context field — `schedule_executions.open_canvas_id`, the same shape as the `source_channel*` columns beside it — stamped at dispatch and read back by the tools and the prompt. It is CONTEXT, never AUTHORITY, and two independent halves keep it there: - `validated_open_canvas` decides what may be STAMPED. The id is client-supplied, so it is checked against the agent's own canvases and against what that caller can see: an operator-only canvas is invisible to an external client (otherwise the field is an existence oracle for canvases the agent keeps privately), and another agent's canvas is refused outright. Every failure degrades to "nothing open" — never an error, never a wider reach. - `effective_canvas_id` decides what a tool ACTS on, and cannot widen anything: every read and write still passes the existing ownership and audience gates. Precedence is stated once so all three tools agree: `explicit canvas_id > the canvas the user has open > the default canvas`. It returns WHY as well as WHICH, because with nothing named the agent has to be able to say which canvas it wrote to — "I updated the canvas" is not good enough when there are eight and the user is looking at one. Both delivery paths are needed, not one. The MCP tools resolve a missing `canvas_id` through `GET /api/agents/{name}/canvas/context` (declared above `/{canvas_id}` — Invariant #4, since "context" is a valid id shape), AND the turn prompt names the open canvas: a tool default handles a call that omits an id, but an agent must READ a canvas before editing it and cannot read what it cannot name. The prompt line rides the same prefix as the file manifest, so it is present on a resumed turn too — the open canvas changes between turns while the session's memory of it does not. A canvas deleted mid-conversation resolves to nothing, re-checked at read time rather than trusted from the stamp: ent#553 made deleting one click, and a surviving id would have the agent's next write CREATE a canvas under it, silently resurrecting something a person deleted. Voice inherits this by construction — ent#440 submits a spoken utterance through the same `deliver()` a typed one takes. The `canvas` tools in `gemini_voice.py` are deliberately untouched: that is VOICE-001's ephemeral display panel, a different surface with no persisted id. Dual-track migration (`execution_open_canvas` + Alembic `0060`); the column is nullable, so every existing row and every un-updated caller reads as "nothing open". Related to Abilityai/trinity-enterprise#555 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * fix(canvas): re-parent the pinned revision, and make the canvas cap reachable Two review items from #2619. **Alembic head fork (#2068 class).** `0058_agent_canvases_pinned` shared `down_revision = 0057` with #2608's `0058_portal_file_dismissals`, which has since landed on `dev` — two heads, and `alembic upgrade head` resolves its single target before applying anything, so EVERY revision merged since the fork stops arriving, not just one. Re-parented onto `0058_portal_file_dismissals` and renumbered to `0059` so the prefix keeps being a usable ordering cue; the id is not applied anywhere yet, so the rename costs nothing. `check_alembic_heads.py` reports 1 head. **`CANVAS_MAX_PER_AGENT` was inert (#1039 class).** The refusal message names the number, but the variable was read only from `os.getenv` in `models.py` and appeared in no compose file — so an operator following the refusal's own advice would raise a lever that never reaches the container. Wired into `docker-compose.yml`, `.prod.yml` and `.hosted.yml` (the last two launch standalone, no base merge / no `env_file`) plus `.env.example`. Related to #2619 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP * chore(design-system): re-freeze CanvasPanel's raw-gray ceiling for ent#553 The raw-colour ratchet became enforceable on dev while this branch was open (#2605/#2609), and the merge brings it here: this PR's delete/pin/ search chrome takes `components/canvas/CanvasPanel.vue` from 25 to 46 `raw_gray`, so `tests/unit/rawColorRatchet.spec.js` fails the frontend build. That growth is the honest kind. The design-system contract SPELLS the neutral ink ladder as `gray-N` — surfaces gray-50/100/800/900, borders gray-200/300/700/800, ink gray-300/400/500/600 — and there is no semantic token for a neutral, which is exactly why the spec's own comment says gray is ratcheted but never held to zero for new files. The rule it does hold new code to is `raw_nongray`, and this file stays at **0**. Re-frozen in its OWN commit with the increase named in the baseline's `refrozen` block, which is what the ratchet's error message asks for — not absorbed silently into the feature diff. The entry is hand-edited rather than regenerated so #2605's provenance block survives; no other file's ceiling moves (verified: nothing grew, nothing is stale, no un-baselined file carries `raw_nongray`). Related to #553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP * fix(canvas): state the bound, audit the Workspace writes, gate Manage on ownership (ent#553) Three review findings, all in the same direction — the backend was right and the user-facing half did not arrive — plus the two smaller ones. 1. **The stated bound now reaches the user.** `canvasLimit` was a `ref(0)` nothing ever assigned, so `canvasHeadroom(n, 0)` returned `{label: null}` and the early warning could not render at any count. The ceiling rides `GET /api/settings/feature-flags` as `canvas_max_per_agent` — the established home for a value the browser needs to render a surface, and where `platform_default_model` / `install_source` already set the precedent for a non-boolean. Not a new route (Invariant #13 would owe three surfaces for one integer) and not an envelope around the canvas list (the MCP tool and the Workspace both read it as a bare array). It is a CONSTANT, not per-agent state, and the client already holds the count. `0` still means "not told" and still renders nothing, so an older backend is unchanged. 2. **The Workspace canvas writes are audited.** The three portal routes recorded nothing while their operator twins have logged since they shipped, and `docs/user-docs/agents/agent-canvas.md` tells users deletion is audited — so the claim was false for exactly the client-facing surface. `_audit_canvas_change` is the shared helper; the actor is `actor_email` (the documented #848 inline-auth path) rather than a fabricated `User`, which is honest because `_require_canvas_manager` is platform-only and owner-or-admin, so a real Trinity user is always behind it. Ids and counts only (G-04). The three routes become `async def` to await it, matching their operator twins, which already call the same sync db functions from an async handler. Pinning is audited too, on BOTH surfaces — the operator route was the one recording nothing. A pin decides which canvas an entire roster sees first, so it is an administrative act on a shared surface, not a per-viewer preference. 3. **`canManage` comes from the parent.** It was hardcoded `true` on the argument that the server decides. It does — but a merely-shared user was then shown Manage → Delete / Pin and got a 403, which is the failing-control problem `can_manage_canvases` exists to prevent on the Workspace. Agent Detail passes `agent.can_share`, the same predicate `ReportsPanel` five lines above already reads and the same one `_gate_human_removal` enforces. The prop defaults FALSE, so a caller that forgets it hides an affordance rather than offering one that refuses. 4. **`get_agent_card` resolves `can_manage_canvases`.** It omitted it, so the same owner read `true` in the sidebar and `false` on the agent's own page — the disagreement #2160's own docstring says that function exists to prevent. 5. **An agent genuinely cannot pin its own canvas now.** The user doc said so; `_gate_human_removal` allowed it (right for delete — an agent tidying up after itself — and wrong for pin), and "no MCP tool exposes it" is a property of the client, not of the route. `_gate_pin` is humans-only, which makes the documented sentence true rather than aspirational. Tests: the audit guard now walks the portal routes as well as `routers.canvas` (it only ever inspected the latter, which is why three unaudited routes passed it), plus pin-audit parity, the humans-only pin gate beside the still-permitted agent self-delete, the feature-flags constant being the same object the refusal is raised from, the agent-card/roster agreement, and four frontend wiring cases. 1025 backend / 2538 frontend tests green. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RSjVEjay9ztC1oDXXkh9uN * fix(canvas): the Workspace audit names the operator, not the platform (ent#553) Found re-reviewing my own audit fix. Adding the rows was right; the attribution was wrong, and a row that lands under the wrong actor is worse than the missing row it replaced — nothing fails, so the wrong answer is believed. `_audit_canvas_change` passed `actor_email` only. But `platform_audit_service._resolve_actor` derives `actor_type` from `actor_user` / `actor_agent_name` / `mcp_scope` / `mcp_key_id` and never from the email, so an email-only call falls through to its last branch: _resolve_actor(None, None, None, None) -> ("system", "trinity-system", None) So every Workspace canvas delete and pin was recorded as `actor_type="system"`, `actor_id="trinity-system"` — a named operator's action attributed to the platform, invisible to any `actor_type=user` query and to the audit UI's per-actor filter. Verified against the real resolver, not by reading the call. The `actor_email`-only path I cited (#848 inline auth) is right where the caller genuinely has no `users` row. That is not this route: `_require_canvas_manager` is platform-only and resolves through `db.can_user_share_agent`, so a row exists by construction. It now resolves that row and passes `actor_user`, producing the same `("user", <id>, <email>)` shape the operator twin has always written — which is the point, since auditing the two surfaces differently buys little more than auditing one of them. Best-effort by construction: the action has already happened, so a lookup that raises or misses must not drop the row. It falls back to the email-only call with a WARNING, since a miss would mean the gate admitted someone the user table does not know. Tests: the regression is pinned against the REAL `_resolve_actor` (both the shape the fix must not return to and the shape it produces now), plus a source guard that the helper resolves a row, passes `actor_user`, keeps the email as a fallback and cannot raise. Removing `actor_user=` reds it. 31 passed on the ent#553 file; 953 across canvas / portal / audit. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RSjVEjay9ztC1oDXXkh9uN * fix(migrations): chain 0060_agent_canvas_shares off the renamed pinned revision ent#553 renamed its revision 0058_agent_canvases_pinned -> 0059 when it absorbed dev's 0058_portal_file_dismissals; this revision still pointed at the old id, so after the merge the directory resolved to two heads and `alembic upgrade head` would have applied nothing. Renumbered to 0060 as well so the numeric prefix stays a unique ordering cue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * chore(frontend): re-freeze CanvasPanel.vue raw_gray 46 -> 62 for ent#554 The share/PDF controls add gray chrome copied from the panel's existing header; the branch predates the #2605 ratchet, so the guard first bit when dev was merged in. Scoped to this one entry, in its own commit, as the guard's own message prescribes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * fix(migrations): renumber execution_open_canvas to 0061 behind 0060_agent_canvas_shares Follows the ent#554 renumber so the chain reads 0059 pinned -> 0060 shares -> 0061 open-canvas with unique prefixes and a single head. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * test(canvas): resolve CanvasLimitExceeded from the live method's globals The two cap tests imported the class from `db.canvas` while `_set_cap` already patches the cap through `upsert_canvas.__globals__` — because an earlier test can evict and re-import the module. The same eviction gives the test a different class object than the one the live code raises, and `pytest.raises` then reports the correct refusal as an unexpected exception. Seen once in a full local run after the dev merge (both tests pass in isolation and under CI's three seeds); resolve the class from the same globals the cap comes from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * fix(canvas): keep the selector visible when a search narrows to one match (ent#553 review) `CanvasPanel.vue` gated the chip strip on `visible.length > 1 || manage`, where `visible` is the FILTERED list. Searching down to exactly one canvas hid the strip while the previously selected canvas stayed on screen, and the auto-select watcher — keyed off the unfiltered `props.canvases` — never selected the match. Proven by execution: 7 canvases, query "Topic 3" → strip false, no-match message false. The one canvas the user just searched for was unreachable. Fix: - `canvasSelectorVisible({visible, manage, query})` — with a query, any hit shows the strip; without one, a single canvas is no choice (unchanged). - `canvasAutoSelect(visible, selectedId, query)` — while a query is active the selection follows the matches; no-op with no query or when the current selection already matches. - `CanvasPanel.vue` consumes both: `v-if="selectorVisible"` and a watcher on `[visible, query]`. Tests: - `canvasUtils.spec.js`: the two pure rules. - `canvasPanelSelectorGate.spec.js`: slices the `selectorVisible` computed out of the SFC and RUNS it against the ejection's numbers; pins that the template reads the computed, not a re-derived length test, and that the watcher calls `canvasAutoSelect`. - `test_ent553_canvas_lifecycle.py`: the second review ask — the per-agent cap reaches the wire as a 409 through the real router → service → db chain (only the Redis rate limiter stubbed), names the remedy, and the same PUT against an existing id stays an update. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * fix(canvas): the search box outlives a shrink below the threshold (ent#553 review) `query` has exactly one writer — the search input's `v-model` — and that input was `v-if="showSearch"` with `showSearch = ordered.length > 6`. Seven canvases, type "Topic 3", delete the one match: six canvases, the box unmounts, `visible` still filters on the stale query, the strip collapses, and the panel says *No canvas matches "Topic 3"* with no control left to clear it. Every remaining canvas is unreachable via the chips until navigation. Also reachable with no operator action: the agent's own `clear_canvas` plus a rail refresh while a query is typed. The rule is pure — `canvasSearchVisible(count, threshold, query)` — and keeps the box while a query is active regardless of the count: the typed intent survives the shrink, and the no-match line keeps the one control that clears it. Resetting `query` when the box would flip off was the other option and was rejected: it erases a search the user was mid-way through because a sibling canvas went away. The gate spec that pinned the previous ejection drove `visible`/`query` in isolation from `showSearch`, which is why it could not see this one. It now slices the real `showSearch` computed out of the SFC and RUNS it against the ejection's own numbers (7 → 6 with "Topic 3" typed → box stays; 6 with no query → box gone), and pins that the input is gated on that computed and is the sole writer of `query`. Mutation-checked: reverting the gate to the old length test reds three cases. Four mechanical items from the same review ride along: - requirements/core-agent.md: the ent#438 "deliberately no retention window: bounded by construction" line now says why that reasoning was wrong (rows are bounded per canvas, the count was not) and what bounds it instead; FR-18..FR-22 record delete / bulk / cap / pin / search, which had no requirements entries at all. - raw-color-baseline.json: the `_ent553_note` naming CanvasPanel.vue's 25 → 46 raw_gray was added in 2794388 and dropped by the dev merge aa248f7; re-added so the growth is named in the file. - routers/canvas.py `# mcp:` header now says pin and bulk-delete are unexposed on purpose, and why — Invariant #13's deliberate-vs-forgotten signal. - feature-flows/agent-canvas.md: the two search-state rules and the defect class they close. Verified: vitest 2696 passed (121 files); canvas backend suites 101 passed; raw-colour ratchet and loading-gate ratchet unchanged. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RS8FBA7oEtAAv6GamZKong * fix(canvas): a share link is a grant, so only a human may mint one (ent#554 review) `create_canvas_share`'s docstring said "Owner-or-admin and human-only via `_gate_human_removal`". That gate is not human-only — its own docstring, one screen above, says an agent-scoped key may act on its own agent, which is correct for `clear_canvas` ("an agent tidying up after itself") and wrong for every verb that decides what someone OTHER than the agent may see. So a prompt-injected agent could POST /api/agents/<self>/canvas/<id>/share {"scope": "public"} with the TRINITY_MCP_API_KEY already in its container and publish its own canvas at an unauthenticated URL. Three things make that worse than it first reads: * the share is LIVE, not a snapshot, so one link is a self-updating channel rather than a one-time disclosure; * the agent is the only writer of canvas blocks, so anything it can read it can copy into a canvas and publish; * `audience` is not consulted on the share path, so ent#438's fail-closed "a canvas reaches a client only because the agent said so" would not have applied — the agent would have been choosing for itself. `list_canvas_shares` had the same gate and returns the TOKEN, which is the capability itself; `revoke_canvas_share` too, so an agent could also turn off a person's link. The fix is the grant-vs-use line (Invariant #8): the endpoint that USES a capability may be agent-callable, the one that GRANTS one is human-only. * `_gate_human_only(current_user, name, *, agent_detail)` is factored out of `_gate_pin` — the predicate was always right, only its NAME described one caller. A gate named for a verb ("removal") is one a fourth caller reaches past by accident; a gate named for its rule is not. `_gate_pin` and the new `_gate_share` both delegate to it, with per-caller refusal text because an agent reads that message to decide what to do next. * The three share routes now call `_gate_share`. * The delete routes deliberately KEEP `_gate_human_removal`, and a test guards that boundary in the other direction — the first attempt at this fix swept `clear_canvas` into the human-only gate, because one `str.replace` matched both bodies. That would have broken a real MCP tool for every agent: a security fix breeding the next bug, the /review §4.14 class. Six regression tests; four of them fail against the previous commit (the other two are the over-correction guards, which must pass both ways by design). The 23 tests already here covered scope defaults, expiry, revocation and enumeration, but none used an agent principal on any share route — which is how this shipped. Docs: the user doc now states that sharing is the owner's alone and that the routes refuse an agent's own key, beside the same sentence for pin; the flow doc records the decision, the blast radius, and why the delete routes stay permissive. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ * docs(canvas): record the share routes in the file's own mcp: convention (ent#554 review) The header comment lists which canvas routes are deliberately NOT exposed as MCP tools and why. ent#554 added three that qualify — minting, listing and revoking a share link — and the list did not grow with them. Worth more than a comment here: the ent#553 entry states the rule the share routes then failed to follow ("no tool exposes it" is a property of the client), so a reader consulting this header to decide a fourth route's gate would have found the reasoning but not the precedent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ * docs(learnings): a gate named after a verb gets reached for by the wrong route (ent#554 review) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Eugene Vyborov <eugene@ability.ai>
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.
Summary
update_schedule_run_timeswas bumpingupdated_atalongside the run-time columns. The scheduler's sync loop watchesupdated_atto detect config edits, so every tick saw its own write from the previous_add_joband re-registered all N schedules once per minute — ~7k "Added job" log lines per 8h for 13 schedules, growing linearly with fleet size.updated_atis now only bumped on legitimate config changes (update_schedule,set_schedule_enabled). Runtime writes leave it alone. APScheduler idempotency (replace_existing=True) was already masking the scheduling impact, so no execution behavior changes — just DB/log churn.Changes
src/scheduler/database.py—update_schedule_run_times,update_process_schedule_run_timesno longer touchupdated_at.src/backend/db/schedules.py— same for the backend's copy ofupdate_schedule_run_times.tests/scheduler_tests/test_sync_loop.py— new regression test file (4 tests).Test Plan
pytest scheduler_tests/test_sync_loop.py -v— 4/4 passpytest scheduler_tests/— 149/149 passpytest test_schedules.py— 23/23 passdocker logs trinity-scheduler --since 5m | grep -c \"Added schedule job\"should report 0 (was ~5×N).Notes
updated_atfrom snapshot hash) to avoid a maintenance liability — Fix B would require every new schedule field (RETRY-001, VALIDATE-001, etc.) to remember to extend the hash.next_run_atis still refreshed on every_add_job/ execution completion. It wasn't gaining new information from the 60s re-writes anyway (cron is deterministic).next_run_at; unaffected — only theupdated_atbump was removed.Closes #420
Generated with Claude Code