Skip to content

feat(workspace): the rail's Canvas/Files dot lights on the write, not the next refetch (ent#532) - #2709

Draft
trinity-ability wants to merge 10 commits into
devfrom
vybe/issue-ent532
Draft

trinity-ability wants to merge 10 commits into
devfrom
vybe/issue-ent532

Conversation

@trinity-ability

Copy link
Copy Markdown
Contributor

Summary

The Workspace rail's Canvas and Files dots (ent#475) already know how to light — what they lacked
was anything telling them a write had happened. The signal was derived from refetches the client
could observe: a turn ending, a room going working→idle, a loop event, a terminal
agent_activity, a tab open, an upload. A canvas rewritten by a scheduled run has none of those
behind it, so the dot waited for the next chat turn or tab open.

Two backend events close that gap, both #918-shaped thin triggers:

Event Emitted from Payload
canvas_updated canvas_service.write_canvas — the only path that writes agent_canvases {type, agent_name, canvas_id}
file_shared agent_shared_files_service._persist_and_register — the shared tail of create_share and create_share_from_bytes {type, agent_name, file_id}

Three properties are the whole design:

  • Ids only, never content. Nothing is rendered from the event; the store re-reads through the
    access-controlled portal routes it already used. The Files payload carries the file_id and
    deliberately not the url — that url embeds a ?sig= download token, a bearer credential, and
    /ws is SCOPE_ALL. The tests assert this over the serialized bytes, not the dict: a
    secret seeded into a block, a title, or a filename must not appear on the wire.
  • Scope is payload-derived (ent#467). A top-level agent_name is the scoping mechanism — a
    client receives the event only if it may access that agent, decided by
    agent_names_in_payload(). No event_bus.py change, no scope= kwarg, no
    FLEET_LEVEL_ALLOWLIST entry. The call sites are dict literals on purpose: ent#467's
    discovery guard resolves the payload by AST, and a helper that splices {"type": event, **payload} would resolve to <dynamic> and fail it. Live discovery goes 36 → 38 sites; the
    guard's own comment and integrations.md are updated in the same commit so the counts do not
    rot (the floor assertion is >= 30, so neither would have failed).
  • A trigger never fails a write. Both writers are synchronous and every caller reaches them on
    the event loop, so the emit is a fire-and-forget task with a strong reference. All three failure
    directions end in no trigger, never in a failed write: no manager wired, no running loop (a
    future caller from an executor thread), or a manager that raises — caught inside the task, which
    is also what keeps it from resurfacing as "Task exception was never retrieved". A lost trigger
    costs latency only; the rail still re-reads on every pre-ent#532 trigger. The manager arrives by
    setter from main.py (Invariant Fix: Add missing Docker labels to system agent container #1 — a service does not import the app).

One emit per write. patch_canvas funnels through write_canvas; the create_share idempotent
replay returns its snapshot without reaching the tail, so a replay correctly emits nothing; a write
that raised emits nothing.

One internal constant, open to veto

portalRailFeeds.js's push debounce is shared with the ent#475 loop/activity triggers, and it
is trailing-edge: it re-arms on every call, so a stream of events closer together than the delay
fires only when the stream ends. That was fine while every caller fired once per execution. It is
not fine for canvas writes (patch_canvas during a streaming run, the voice panel writing per tool
call), and because the timer is shared, an uncapped burst would also defer the loop and activity
reads riding it — making a shipped signal slower.

So a pending read is never deferred past PUSH_MAX_WAIT_MS = 2 × PUSH_DEBOUNCE_MS from the first
event of a burst. This is an internal timing constant, not a setting and not a user-facing
default: it is derived from the existing debounce so the two cannot drift, and single-event timing
is unchanged byte for byte (a lone event still fires at exactly 2s, pinned by its own test). Happy
to drop the cap and document the starvation instead, or to pick a different multiple — say the
word and it is a one-line change.

Fixes abilityai/trinity-enterprise#532

User-visible change: none

No .vue file is touched, and no component, token, control, copy, empty state, error state or
default changes. The Canvas and Files dots, their derivation and their clear-on-open behaviour are
exactly ent#475's. The only delta is when an already-shipped dot lights: within the existing 2s
debounce of the write, instead of at the next observable refetch. The issue records its own journey
impact as none.

Verification

Full local verification ran before this branch was pushed (/verify-local --skip-agent; no
docker/base-image change):

  • unit: 15,107 passed / 31 skipped
  • production image build + import smoke: OK
  • stack boot + /health: OK
  • integration: 70 passed / 13 skipped / 2 registry-deselected

Re-run after the rebase onto dev (the neighbourhood, not the full suite):

tests/ $ python3 -m pytest unit/test_ent532_rail_thin_triggers.py unit/test_918_report_broadcast.py \
    unit/test_ent467_ws_agent_scope.py unit/test_1483_ws_setters_wired.py unit/test_ent438_agent_canvas.py \
    unit/test_ent536_canvas_vocabulary.py unit/test_2582_portal_uploads.py unit/test_117_voice_replies_v2.py \
    unit/test_whatsapp_outbound_media.py unit/test_2338_journey_catalog.py -q -p no:randomly
→ 322 passed

src/frontend $ npx vitest run tests/unit/portalRail.spec.js tests/unit/portalRailFeeds.spec.js \
    tests/unit/rawColorRatchet.spec.js tests/unit/loadingGateRatchet.spec.js
→ 103 passed (4 files)

Also: /review and /cso --diff both clean on this branch; a second-voice review ran during
planning and its objections are reconciled in the plan's audit trail (the debounce cap above is the
one that changed the design). tests/registry.json carries the new file; test_1483_ws_setters_wired.py
needed no edit — it AST-discovers any alias matching ^set_.*(?:ws|websocket)_manager$ in main.py
and asserts it is called, which both new setters satisfy by name.

New tests: tests/unit/test_ent532_rail_thin_triggers.py (ids-only by strict equality plus leak
checks over serialized bytes; one emit per write; nothing for a raised write or an idempotent
replay; channel media emits too; the three never-fail directions parametrized over both
per-service copies of the helper; an AST case pinning the dict literal; the main.py wiring) and
six cases added to portalRailFeeds.spec.js (participant gating, burst coalescing, the bound, the
unchanged single-event timing, and that a canvas burst cannot defer the loop refresh sharing the
timer).

Rebase and collisions

Rebased onto dev at 682fce300. One conflict, tests/registry.json — rebuilt from the git
stages (dev's array plus this lane's single entry appended, deduped and JSON-validated), never by
splicing markers; a repo-wide marker grep is clean.

  • feat(canvas): delete, pin, search and a stated bound for the canvas pile (ent#553) #2619 (ent#553 canvas delete/pin/search) is still open (mergeable) as of this push, so no
    resolution was needed. When it lands it wraps this exact db.upsert_agent_canvas(...) call in
    try/except CanvasLimitExceeded. The emit here is already written to survive that shape — it
    sits after the store, on the success path only, so a cap-rejected write (409) emits nothing
    either way. Whichever merges first, the other rebases with a one-hunk resolution.
  • ent#533 (the Work card's pipeline-stage broadcast, in flight in parallel) shares two files
    with this branch: src/backend/main.py (the ws-manager setter imports and calls) and
    src/frontend/src/utils/websocket.js (the default: route branch). Both edits on this side are
    pure appends that touch no existing line, so first-merged wins and the loser re-adds its
    lines — trivial as long as neither branch reorders or regroups those blocks.
  • docs/memory/requirements/core-agent.md §5.33 was still the next free section number after the
    rebase; §5.20's two cross-references point at it.

Follow-ups — listed, not filed (maintainer's call)

  1. External (portal-token) clients still get no push. They never open /ws (it needs the
    platform JWT), and their only push is the per-execution stream, which ends at stream_end —
    there is no long-lived portal channel to carry a trigger, and building one is a new transport
    with its own auth, not this issue's scope. The flow doc's caveat stays, reworded to say why.
    This is the one item worth a .claude/DEBT_INBOX.md entry; that file lives in a separate
    (private) repo, so it is left for a human to add rather than written from here.
  2. chat_response_ready could drive ent#557's refreshThreads(). It is already a /ws event
    carrying agent_name; ent#557 deliberately shipped its unread badge on the existing 20s poll
    rather than wait for this mechanism. For platform sessions it could now ride the same
    websocket.js route block this PR adds.
  3. Test-catalog row. .claude/agents/test-runner.md wants a row for the new file, in its
    - **Name** (unit/test_x.py) - description [UNIT] form. That file is in the private .claude
    submodule — a separate repo — so it is not part of this PR.
  4. scheduleRefresh(delay) truncates an explicit delay above the cap (latent, not reachable
    today: every caller uses the default). If a future caller passes a deliberately longer delay it
    would be clamped to the max wait.
  5. logger.debug on a failed broadcast — deliberate (a trigger is best-effort and a noisy
    warning on every disconnect would be worse), but arguable if these ever need to be observable.
  6. Refetch granularity — the trigger re-reads both canvases and documents. Splitting the read
    per event type would save a round trip; not worth the branch for a P3 whose reads are already
    coalesced.
  7. ent#467 fail-open on a falsy agent_name — an event naming no agent stays fleet-visible by
    design. Unreachable from these two call sites (both are keyed on a required agent name), noted
    only because it is a property of the channel rather than of this change.
  8. portalWork.js:119 has the identical un-capped debounce shape. Deliberately left alone —
    it is ent#533's file in this same wave, and a cross-lane edit to a shared-shape file is how two
    branches collide on one hunk.

Two process notes: this PR's diff touches src/frontend/ but no .vue file, so frontend-e2e
will not run unless you add the ui label when flipping it out of draft — your call whether that
is worth it for a store/util change. And the closing reference is cross-tracker, so the private
issue does not auto-promote on merge; it needs a manual status-in-dev.

🤖 Generated with Claude Code

trinity-ability and others added 10 commits September 11, 2026 15:37
…t#532)

Requirements before code (Rule #1). §5.20's Canvas and Files dots derive from
store data, so they light only when something the client can observe makes the
store re-read — a turn ending, a loop event, a terminal agent_activity, the tab
being opened, an upload. A canvas rewritten by a scheduled run had no such
moment and waited for the next turn or tab open. §5.20 deferred a backend
broadcast to the debt inbox; this is that broadcast, written up before it is
built.

- requirements/core-agent.md: §5.20 AC-4 gains the two triggers and the
  out-of-scope bullet points at the new §5.33, which states both event shapes,
  the one-chokepoint-each rule (the idempotent share replay emits nothing), the
  ids-only wire, the ent#467 payload-derived scope, fire-and-forget delivery
  that can never fail a write, the §5.20 consumer, and why the shared refresh
  debounce needs a max-wait cap.
- architecture/integrations.md: the thin-trigger-from-a-sync-service paragraph
  beside the ent#467 contract it publishes under.
- architecture/backend.md: canvas_service.py joins the services catalog (it had
  no entry); the shared-files line names the emit.
- feature-flows: workspace-rail.md's "Updated since last view" no longer says
  no backend event exists, and says why an external client still waits (a
  portal token opens no /ws at all — a new transport, deferred, not an
  oversight); agent-canvas.md's "Deferred still" becomes shipped;
  websocket-event-bus.md gains a 2b block with both shapes and the sync
  fire-and-forget mechanics.

Refs Abilityai/trinity-enterprise#532

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… (ent#532)

Failing first (5 failed, 6 errored on `no attribute 'set_websocket_manager'`).
What each case is actually for, since a leak guard that passes vacuously is
worse than none:

- the payload is asserted by EQUALITY, not by absence of a known secret, so a
  field added to the wire later fails here instead of shipping onto SCOPE_ALL;
  the seeded block text, the title, the display name and the `?sig=` url are
  then checked against the serialized bytes as a second line.
- `agent_names_in_payload(event)` pins the ent#467 contract from the consumer's
  side, and an AST case pins that the call site is a dict LITERAL — a helper
  splicing `{"type": event, **payload}` resolves to `<dynamic>` in that guard's
  discovery and is classified fleet-level, i.e. it silently fails open.
- one emit per write (patch funnels through write_canvas), none for a write
  that raised, none for `create_share`'s idempotent replay — paired with the
  non-replay call in the same test so the zero is not vacuous.
- no manager, no running loop, and a raising manager are all silent; the
  raising case gathers the pending task, which re-raises if the inner await
  wrapper is ever removed.
- the main.py wiring case also asserts our alias names match #1483's discovery
  regex — an unmatched alias is silently uncovered, not loudly broken.

Refs Abilityai/trinity-enterprise#532

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… (ent#532)

Green: 11 new + the 257 that were already passing over the eight neighbouring
files (268 total). The emit sits in the service, not the router, because the
router is not the chokepoint: `write_canvas` is the only path that writes
`agent_canvases` (REST PUT, REST PATCH via patch_canvas, MCP, and the Gemini
voice panel all funnel through it), and `_persist_and_register` is the shared
tail of both share entry points (agent extract and channel bytes).

Three properties are load-bearing.

* The dict at the call site is a LITERAL. ent#467's discovery guard resolves a
  broadcast payload by AST to decide who may receive it; a `_broadcast(event,
  payload)` helper that splices `{"type": event, **payload}` resolves to
  `<dynamic>`, carries no agent key, and is classified fleet-level — it fails
  open, silently. Verified by running that guard's own discovery: it now finds
  38 sites and reads both new payloads as agent-keyed, so neither needed a
  FLEET_LEVEL_ALLOWLIST entry and event_bus.py is untouched.
* The emit is AFTER the store and on the success path only, so a cap rejection
  or a validation refusal emits nothing — and `create_share`'s idempotent
  replay returns its snapshot without reaching the tail, so a replay emits
  nothing either. Nothing new was shared.
* A trigger can never fail or delay a write. Both writers are sync and reach
  the emit on the event loop, so it is a `create_task` held by a strong-ref set
  (a bare handle can be GC'd mid-flight) with the await wrapped inside the
  task. No manager, no running loop, and a raising manager all end in "no
  trigger" — which degrades to the pre-ent#532 refetch triggers, i.e. latency,
  never a wrong dot.

The wire carries ids only (#918): never blocks, title, audience, filename, or
the share url, which embeds a `?sig=` bearer token on a SCOPE_ALL channel.

main.py: two setter imports and two setter calls, both PURE APPENDS to the
existing regions — ent#533 appends its own pair to the same two hunks, and a
reorder there is what turns a trivial rebase into a real conflict.

Also corrects three "36 live broadcast sites" counts to the 38 the guard now
reports (the guard's own comment, integrations.md, websocket-event-bus.md).
The floor assertion is `>= 30`, which is exactly why the number rots silently.

Refs Abilityai/trinity-enterprise#532

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
5 of the 6 new cases fail on today's store (the sixth is the shared-timer
regression guard, which can only bite once the route exists — it is checked
against a route-without-cap build before the cap lands, so it is not a test
that cannot fail).

The one worth reading is "a sustained write stream refreshes on a bound, not
only when it stops". `scheduleRefresh` is a pure trailing debounce that clears
and re-arms on every call. That was correct for its ent#475 callers, which fire
once per execution; canvas writes are not that shape — `patch_canvas` during a
streaming run and the voice panel writing per tool call are a sub-2-s stream,
and an uncapped trailing timer under a sustained stream fires when the stream
ENDS, which for a long run is never. Worse, the timer is SHARED with the
loop/activity triggers, so adding a high-frequency writer to it makes a shipped
signal slower. That is a regression this lane would introduce, not a
pre-existing condition, which is why the cap is required rather than an
optimisation.

Paired with a case pinning that a SINGLE event still fires at exactly 2 s, for
both a new type and an ent#475 one — the cap must bound the pathological case
without moving any shipped timing.

portalRail.spec.js's source guard goes 2 → 3 and now also names the route
condition and the derived `PUSH_MAX_WAIT_MS`, so deleting either is a failing
test rather than a silently slower dot.

Refs Abilityai/trinity-enterprise#532

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Green: 89 in the two rail specs, 103 with the raw-colour and loading-gate
ratchets (no `.vue` touched, so neither moves).

`websocket.js` gets ONE appended block in the `default:` branch — ent#533
appends its own below it, and reordering what is there is how two lanes collide
on one hunk. The store's participant gate is unchanged: an event naming an
agent the open conversation does not include is still nobody's to act on, and
nothing is rendered from the event — the data comes back through the
access-controlled client-portal routes, which is the half of #918 that makes an
unfiltered channel safe to publish on.

`scheduleRefresh` gains a max wait. It was a pure trailing debounce that
re-arms on every call, which was correct while every caller fired once per
execution. Canvas writes are a different shape, and the timer is shared — so
without the cap this lane would have made the shipped Loops/Work refresh slower
under a canvas burst. The wait is now bounded from the FIRST event of a burst,
so a sustained stream refreshes every ~4s instead of only when it stops, while
a single event still fires at exactly 2s: `_debounceSince` is `now`, the wait
is the full delay, and no shipped ent#475 timing moves.

`PUSH_MAX_WAIT_MS` is derived from `PUSH_DEBOUNCE_MS` rather than written as
4000, so the two cannot drift. The identical un-capped shape in
`stores/portalWork.js` is deliberately left alone — it is ent#533's file in
this same wave.

Refs Abilityai/trinity-enterprise#532

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…te-tests)

15 passed (was 11). /update-tests coverage review found one real gap and one
missing path:

- the `_broadcast` helper is a per-service COPY — two modules, two guards — and
  the three "a trigger never fails a write" directions (no manager, no running
  loop, a raising manager) were asserted only against the canvas copy. A
  defence proven on one of two sibling call sites is the incomplete-fix class
  the methodology names explicitly, so all three are now parametrized over both
  services.
- `create_share_from_bytes` (WhatsApp media, voice notes) reaches the same tail,
  and `list_active_for_agent` does not filter by creator, so those rows ARE
  Files-tab rows. Now asserted directly rather than inferred from the shared
  tail.

Refs Abilityai/trinity-enterprise#532

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ync-feature-flows)

/sync-feature-flows mapped the changed code files to their flows and found one
the plan's §6 list had missed: `file-sharing-outbound.md` owns
`agent_shared_files_service.py`, so the new `file_shared` trigger belongs in its
service-behaviours table and its side-effects list, not only in the rail's flow
and the event-bus flow. Adds the revision-history row those two edits are
recorded under.

`workspace-rail.md`'s header gains ent#532 in its folded-in line, the same way
it carries #2540 — the rail flow now documents behaviour that arrives from the
backend, not only from the shell.

No new flow file, so `feature-flows.md` (the index) is unchanged.

Refs Abilityai/trinity-enterprise#532

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
PEP 8 spacing between a top-level function and the constants block that
follows it. No behaviour change.

Refs Abilityai/trinity-enterprise#532

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nt#532)

The repo's own test index, appended (purely additive — 12 lines, no existing
line touched, so the rebase against ent#533's own append is the array tail and
nothing else). Nothing enforces completeness here, which is exactly why it
drifts.

Refs Abilityai/trinity-enterprise#532

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…eview)

`test_a_raising_manager_never_fails_the_write` claimed in its own comment that
deleting `_send`'s inner try/except would turn it red. It would not. The test
drained the loop first and only then snapshotted `asyncio.all_tasks()`, which
returns only tasks that have NOT finished — so `pending` was empty and the
`gather` that was supposed to re-raise awaited nothing at all.

Measured, not reasoned: with the wrapper deleted from BOTH service copies, the
whole file stayed green at 15 passed. The three "a trigger never fails a write"
directions were parametrized over both copies precisely so a guard proven on one
of two siblings could not ship — and the one direction that needed a live task
was proving nothing on either.

Snapshot the task before letting it run, and assert there is one. The gather now
runs the emit and re-raises what it raised, so both parametrizations fail with
the wrapper gone and pass with it. No production code changes: the wrapper was
always correct, only the proof of it was hollow.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

@github-actions

Copy link
Copy Markdown

⚠️ Live-instance suite skipped — merge conflict against dev.

Resolve by merging dev locally and pushing the result; the next nightly re-tests.

@vybe

vybe commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Still wanted: nothing equivalent is on dev (no canvas_updated / file_shared emitters), ent#532 is open, and ent#557's unread indicator plans to reuse this mechanism. To un-draft: merge dev (about 277 commits behind; conflicts in canvas_service.py, websocket.js, registry.json, backend.md, core-agent.md), re-check the requirements section number, re-run the unit tests, then mark it ready.

vybe added a commit that referenced this pull request Oct 9, 2026
…orable secret value is a named 422 (#3325) (#3417)

Fixes #3325 — independent fix, one of ten small bug fixes in the trinity-pm `chain-easy-1008` run. Base is `dev`. **Draft until the operator merges.**

## What
- `src/backend/services/credential_encryption.py::decrypt`: every malformed envelope shape raises `ValueError`, as the function promises — JSON that is not an object (`[]`, `null`), and a non-string nonce or ciphertext. Importing such a `.credentials.enc` now returns **400** instead of a 500 with a stack trace in the log. The envelope format and every successful decrypt are unchanged.
- `src/backend/services/secret_settings.py::encrypt_secret_setting`: the key is checked first, in its own `try`, and only that check can raise `MissingEncryptionKeyError`. A value that cannot be encoded (for example a lone surrogate) raises the new `SecretSettingValueError(ValueError)`. Its message names the setting, never the value.
- `src/backend/error_handlers.py` + `src/backend/main.py`: one app-level handler maps `SecretSettingValueError` to **422**. `src/backend/routers/settings/credentials.py`: three pass-through lines in the routes whose catch-all would have turned it into a 500 (Anthropic, GitHub PAT, `PUT /slack`).
- Tests: the three strict-xfail markers in `test_ec_credential_crypto_edges.py` are removed; new `tests/unit/test_3325_credential_error_types.py` (registered in `tests/registry.json`) asserts no value in the message or the exception chain, a missing key reported before a bad value, `PUT /api/settings/api-keys/anthropic` → 422, and that `main.py` registers the handler.

## Rulings carried (orchestrator, on the operator's behalf — plan file)
- TD-1: one app-level handler plus the three pass-through lines — as recommended.
- TD-2: a `.credentials.enc` that decrypts correctly but whose contents are not an object is deferred — as recommended; forging one needs the platform key.
- TD-3: 422, matching the existing 422 for refused cleartext writes — as recommended.

## Review + security
`/review` reading pass (claude-fable-5-1, report-only): **MERGEABLE**, no critical findings. The secret cannot leave through the new error: it is raised after the `except` block, so `__context__` is cleared, not merely hidden; the handler returns only the message; nothing logs it; audit rows are written only after a successful write. The only residual carrier is traceback frame locals, which nothing in the backend renders. The handler is keyed on the subclass, so no other `ValueError` is swallowed or relabelled. `/cso --diff`: no findings.

Evidence sweep (claude-opus-5-5): **GREEN**. 23 unit files import the touched modules; the 16 most direct ran shuffled (seed 12345), one file per process, all passed. All 7 tests in the new file ran, 0 skipped. With both service files taken from `dev`, exactly the three formerly-xfail tests fail (8 cases). Seven less direct files are left to CI and named in the review file. `git merge-tree` against `dev` f6bedcd is clean.

## Tests
Targeted counts above. Not run here: the full unit island (CI).

## Before merge
- Startup auto-import (`lifecycle.py`) on a malformed envelope now lands in its `ValueError` retry arm: it retries without a warning per attempt and ends in the same "failed" result, with the existing error log after the last attempt.
- Enterprise callers of `decrypt` get `ValueError` where they got `AttributeError` / `TypeError` on a malformed envelope. Two catch broadly, three propagate as before; no change needed there.
- The new test file has a module-level `importorskip` for `sqlalchemy` and `cryptography`: it skips silently where those are missing. They are present on the engineer and in CI.
- Ten open PRs share `tests/registry.json` or `src/backend/main.py` with this branch (#3412, #3411, #3407, #3304, #3271, #3256, #2984, #2956, #2713, #2709). A test-merge of this head against each adds no conflict beyond what that PR already has with `dev`.

## Handoffs (not in this diff)
- TD-2 above: the decrypted-but-not-an-object case can still 500 on import.
- `PUT /api/settings/slack` saves the client ID before a later secret write fails (partial update, pre-existing).
- The GitHub PAT, Resend and Gemini routes are covered by the app-level handler but not exercised by the new route test.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
vybe added a commit that referenced this pull request Oct 9, 2026
…L is refused, not stored (#3323) (#3420)

Fixes #3323 — independent fix, one of ten small bug fixes in the trinity-pm `chain-easy-1008` run. Base is `dev`. **Draft until the operator merges.**

## What
- `src/backend/utils/url_validation.py::reject_embedded_credentials`: strips its input and treats a leading `//` as already having a host, the same rule `strip_url_credentials` uses. `//<token>@github.com/o/r` is now refused like the `https://` form, including with leading whitespace or mixed case. All three write paths that store a skills-library URL go through this one function (`routers/skills.py` create and update, legacy adoption in `services/skill_service.py`), so the token is no longer saved in plaintext to `skill_sources.url` or the audit details.
- If `urlparse` refuses an input that starts with `//`, the function falls back to the module's existing userinfo regex instead of raising, so this fix adds no new 500.
- Tests: the D3 strict-xfail marker is removed; four more refused shapes and a four-case "must not raise / must not over-reach" test are added.

## Rulings carried (orchestrator, on the operator's behalf — plan file)
- TD-1: the fallback applies only to inputs starting with `//`; the bare `ValueError` for every other malformed input is #3322's and is unchanged (D4 stays a strict xfail) — as recommended.
- TD-2: keep the username/password check; `//@host` carries no secret and is not refused — as recommended.
- TD-3, TD-4: deferred, see Handoffs — as recommended.

## Review + security
`/review` reading pass (claude-fable-5-1, report-only): **MERGEABLE**, zero critical, five informational. The pre-fix and post-fix modules were executed side by side over forty inputs: nothing that was refused before now passes, credential-free inputs are unchanged, the fallback regex is anchored and linear. `/cso --diff`: no findings.

Fixed after review:
- `546f329a` — the `tests/registry.json` descriptions no longer call #3323 a strict xfail.
- `a75a5e04` — the reject stub in `test_ent183_skill_packages.py` mirrors the fixed `//` rule. It is deliberately stricter than production on a malformed bracket (`//[oops/x` raises).

Evidence sweep after the fix (claude-opus-5-5): **GREEN**. 28 unit files import the module; the 14 most direct ran shuffled (seed 12345), one file per process. Three did not come back clean, none because of this branch:
- `test_ent236_skills_lifecycle.py::TestNonRepoDirectoryRecovers::test_real_repo_still_pulls` and `test_skill_service_user_agent.py` (collection error) fail the same way on `dev` f6bedcd in the engineer's environment.
- `test_ent237_skill_sources.py` hit the 240 s per-file limit at about 72 % with no failure printed — left to CI.

`git merge-tree` against `dev` f6bedcd is clean.

## Tests
895 passed across the 11 files that completed cleanly. Not run here: the 14 less direct importer files, `test_ent237_skill_sources.py` to completion, and the full unit island (CI).

## Before merge
- Ten open PRs share `tests/registry.json` with this branch (#3417, #3412, #3411, #3407, #3304, #3271, #3256, #2984, #2956, #2709). A test-merge of this head against each adds no conflict beyond what that PR already has with `dev`.
- One stale docstring remains at `tests/unit/test_ec_url_validation_properties.py:36` (still calls D3 a strict xfail).

## Handoffs (not in this diff)
- TD-3 (P3): shapes that still pass both reject and strip — `/\t/tok@`, `/\n/tok@`, `///tok@`, `https:/tok@` (`urlparse` removes tab and newline after the check). Fixing it needs the same change in `strip_url_credentials` and cases in both test families.
- TD-4: a property test that reject and strip always agree over the shared `test_2052` corpus; it needs exceptions for empty userinfo.
- #3322 (bare `ValueError` / `UnicodeError` → 500) is the sibling issue in the same file and is untouched.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
vybe added a commit that referenced this pull request Oct 9, 2026
…t the database (#3385) (#3421)

Fixes #3385 — one of ten small bug fixes in the trinity-pm `chain-easy-1008` run. **Stacked on #3414** (the fix for #3314): base is `feature/3314-queue-fingerprint-falsy`, because both issues edit `_clamp_ingested_item` and the fingerprint helpers. Merge #3414 first, then retarget this to `dev`. **Draft until the operator merges.**

## What
- `src/backend/services/operator_queue_service.py`: new `OPERATOR_QUEUE_TYPE_MAX = 64` and `_bounded_type`. An agent-written `type` longer than 64 characters is shortened with the existing `…[truncated]` marker at ingest (`_clamp_ingested_item`); the ask is never refused or lost. `_comparable_type` applies the same bound, so a shortened row does not read as a rewrite (#2915) and a row stored before this fix at full length still compares equal.
- `src/backend/db/operator_queue.py::_insert_values`: `_DB_BELT_TYPE_MAX_BYTES = 1024`; above it the insert raises `ValueError`, like the id / title / question / context belts. All three create paths go through it.
- Docs: five lines in `architecture/api-endpoints.md`, `feature-flows/operating-room.md` and `requirements/security.md` that list the clamp and belt fields now name `type`.
- Tests in `tests/unit/test_1632_operator_queue_caps.py`: an oversized type is shortened with the marker; every platform-emitted type and every type `ask_operator` accepts is unchanged; the belt rejects an oversized type and passes 64 four-byte characters; `test_3385_clamped_type_is_not_a_rewrite` (clamped row vs raw entry, legacy full-length row, a real rewrite inside the first 52 characters).

## Rulings carried (orchestrator, on the operator's behalf — plan file)
- T1: a fixed constant, no environment variable — as recommended.
- T2: 64 characters — as recommended. The longest type the platform emits is `workspace_problem_report`, 24.
- T3: the DB belt raises, like its siblings — as recommended.
- Orchestrator: stacked on #3314 instead of branching from `dev`, so the two fixes do not hand the operator a conflict in the same two functions.

## Review + security
`/review` reading pass (claude-fable-5-1, report-only, against the stacked base): **MERGEABLE**, zero critical. Census of every literal `type` the platform and the enterprise submodule pass to the insert paths: longest is 24 characters, none is altered. Every truncated value ends in the marker, which no set member can match, so truncation cannot move a value into or out of a set that decides control flow (`_BUDGETED_ALERT_TYPES` and neighbours). The new `ValueError` is reachable only from the file sync loop, which quarantines it under its existing handler. `/cso --diff`: no findings; the error text carries the constant, never the value.

Fixed after review: `eab91c10` — I1, the five doc lines. Noted, not changed: I2, a non-string `type` over 1 KiB now raises the named `ValueError` where the base failed at the bind — same quarantine outcome.

Evidence sweep after the fix (claude-opus-5-5): **GREEN**. 13 operator-queue files ran shuffled (seed 12345), one file per process: 836 passed, 7 xfailed. With both production files taken from the #3314 branch, the two `TestClampType` cases, the belt test and the clamped-row fingerprint case go red. `git merge-tree` against `dev` f6bedcd is clean; the base branch has not moved (`923b1edc`).

## Tests
Counts above. Not run here: `test_ent815_queue_walk`, `test_ent815_broad_list_agent_scope`, `test_ent751_gate_entries` (2–4 minutes each; they ran green on the #3314 base in that PR's sweep) and the full unit island.

## Before merge
- **This PR's checks are weaker than a dev-based PR's:** a PR whose base is a feature branch runs a reduced CI set and `backend-unit-test` does not run. The first full run happens when it retargets to `dev` after #3414 merges — read that run before merging.
- Accepted trade-off: an agent that rewrites an oversized type only after character 52 is not detected as a change. Titles and questions already behave this way.
- Thirteen open PRs share a file with this branch (mostly `tests/registry.json` and `docs/memory/architecture/api-endpoints.md`; #3256 also `db/operator_queue.py` and the three docs). A test-merge of this head against each of #3420, #3417, #3412, #3411, #3407, #3304, #3271, #3256, #2984, #2956, #2713 and #2709 adds no conflict beyond what that PR already has with `dev`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

This branch has not been deployed

No deployments
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.

2 participants