Skip to content

feat(workspace): agent-initiated asks — addressed column, validation, and three renderings (ent#364) - #2300

Merged
vybe merged 4 commits into
devfrom
feat/2364-workspace-asks
Aug 19, 2026
Merged

vybe merged 4 commits into
devfrom
feat/2364-workspace-asks

Conversation

@dolho

@dolho dolho commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

The OSS half of ent#364: an agent raises a question, an approval request or an update addressed to a specific workspace user; it renders in three places and is cleared everywhere when answered.

Gated surface half: Abilityai/trinity-enterprise#427 (which also carries the design of record). Merge order: this PR → submodule pin → enterprise PR, since the enterprise module reads this column.

One row, not three

An ask is an operator_queue row with an addressee — addressed_to_email, nullable, NULL meaning "the operator answers" so every existing row keeps its meaning. Both migration tracks (invariant #3). No new table: ent#164 builds a hard human-approval gate on these same rows, and two approval systems is the failure mode.

Why the addressee is a column, not a key in context

It decides who may answer an ask and whose sidebar it appears in — an authorization decision. context is free-form agent-authored JSON whose hygiene clamp bounds size and type only, so an addressee living there is an agent picking its own audience.

So it is validated at that same #1632 ingestion boundary against agent_on_roster(agent, email, include_owned=False): an agent may address only someone already on its roster. include_owned=False is the rule, not a default — flipping it on for a portal principal would hand a client agents nobody shared with them (see that function's own docstring).

Fails closed throughout — non-string, address-shaped-nothing, over-length, off-roster, or a roster read that raises all resolve to NULL, i.e. an ordinary operator ask. Malformed values are refused before the lookup so garbage can't amplify into a DB read per item. A context.addressed_to_email is never honoured, pinned by a test, because that is exactly the hole the column exists to close.

Three renderings, one list

PortalAsks.vue renders on the agent page and inline above the chat composer; the sidebar shows store.askCount. All three come from one fetchAsks() — which is what makes "answering anywhere clears it everywhere" true with no sync step. Answering removes the row from local state rather than patching it, so a stale pending copy can't offer to answer the same ask twice.

An expired ask renders as expired and is excluded from the count: #1142 deletes terminal rows, and an ask that simply vanishes reads as "answered" to the person who didn't answer it — but it isn't something to nag about either.

Poll, not a socket. The Workspace has no WebSocket (operator_queue_new goes out on the platform /ws, which a portal client isn't on) and already polls elsewhere. 20s, visibility-aware, torn down with the view.

Honest limits

  • AC Setup improvements #5 is only half-deliverable today. Answering writes back to the agent's queue file through the OSS respond path, which the agent processes at its next turn — so scheduled agents act on answers now, and an agent with no schedule does not until ent#329 lands (tracked as ent#430, status-blocked). That's the prerequisite ent#364 names; nothing here pretends otherwise.
  • Raise-time chat attachment (AC fix: add missing logging_config.py to backend Dockerfile #4) reads context.workspace_session_id when the platform has written one; the writer is ent#429's slice. An ask without it still renders in the sidebar and on the agent page, so it is never homeless — just not yet threaded.
  • These two new calls use the bare axios export like the other 29 in that store. fix(workspace): a client session that EXPIRES on a browser holding a platform login falls back to the platform identity #2261 is moving the whole file onto a dedicated instance; half a file on each transport would be worse than either, and that PR owns the migration.

Testing

$ pytest tests/unit/test_ent364_addressed_asks.py -q                       # 16 passed
$ pytest <operator-queue suites: 1632, 1631, 1525, 1426, 1142> -q          # 62 passed
$ npx vitest run tests/unit/workspaceAsks.spec.js                          # 9 passed
$ npx vitest run                                                           # 937 passed
$ python3 scripts/ci/check_alembic_heads.py src/backend/migrations/versions # 1 head
$ npx vite build                                                           # ✓

Not covered by the unit suite (node-only, no DOM): the three template branches. Their conditions are computeds over tested store state and each carries a data-testid.

Related to ent#364

dolho and others added 4 commits August 19, 2026 12:28
… three renderings (ent#364)

The OSS half of ent#364. An agent raises a question, an approval request or an
update addressed to a specific workspace user; it renders in three places and is
cleared everywhere when answered.

**One row, not three.** An ask IS an `operator_queue` row with an addressee —
`operator_queue.addressed_to_email`, nullable, NULL meaning "the operator answers"
so every existing row keeps its meaning. Both migration tracks (invariant #3):
`migrations.py::operator_queue_addressed_to` and Alembic
`0039_operator_queue_addressed_to`. No new table, because ent#164 builds a hard
human-approval gate on these same rows and two approval systems is the failure
mode.

**Why the addressee is a column and not a key in `context`.** It decides who may
answer an ask and whose sidebar it appears in — an authorization decision — while
`context` is free-form agent-authored JSON whose hygiene clamp bounds size and type
only. Put it there and an agent picks its own audience. So it is validated at that
same #1632 ingestion boundary (`_validated_addressee`) against
`agent_on_roster(agent, email, include_owned=False)`: an agent may address only
someone already on its roster. `include_owned=False` is the rule, not a default —
flipping it on would hand a client agents nobody shared with them.

Fails closed throughout: a non-string, an address-shaped-nothing, an over-length
value, an off-roster address, or a roster read that raises all resolve to NULL — an
operator ask. Malformed values are refused BEFORE the lookup, so garbage cannot
amplify into a DB read per item. A `context.addressed_to_email` is never honoured;
pinned by a test, since that is precisely the hole the column exists to close.

**Three renderings, one list.** `PortalAsks.vue` is used by the agent page and
inline above the chat composer, and the sidebar shows `store.askCount` — all from a
single `fetchAsks()`. That is what makes "answering anywhere clears it everywhere"
true with no sync step: three bespoke queries is how it stops being true. Answering
removes the row from local state rather than patching it, so a stale "pending" copy
can never offer to answer the same ask twice.

An expired ask RENDERS as expired and is excluded from the count: #1142 deletes
terminal rows, and an ask that simply vanishes reads as "answered" to the person who
did not answer it — but it is not something to nag about either.

**Poll, not a socket.** The Workspace has no WebSocket (`operator_queue_new` goes
out on the platform `/ws`, which a portal client is not on) and already polls
elsewhere; 20s, visibility-aware, torn down with the view. An ask is a
human-latency decision, and new transport for one badge is not worth it.

The gated read/answer surface is the enterprise `workspace_asks` module; this half
is the edition-agnostic primitive plus the UI, and the UI self-hides when the
endpoint 404/403s so an OSS build renders nothing and logs nothing.

16 OSS tests (validation, fail-closed paths, no-context-honouring, column
round-trip) + 9 frontend (one-list contract, answering clears every surface,
expired-renders-uncounted, silent-on-unentitled, refusal surfaced). Full frontend
suite 937 passed; `check_alembic_heads` reports 1 head.

Related to ent#364
…s (ent#364)

0001_baseline builds operator_queue from the live db/tables.py metadata, which
now declares addressed_to_email — so on a FRESH PostgreSQL database the column
already exists by the time 0039 runs, and the unguarded add_column aborted the
Alembic upgrade with DuplicateColumn. That kills boot, not just the revision.

The SQLite track hid this: _safe_add_column already guards there, so the whole
unit suite stayed green. Found by booting the branch against a real Postgres;
CI's pg-migrations job failed on it independently.

Guarded with the same _has_column idiom 0031 uses.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…+ .env.example (ent#364)

The new addressee-email clamp read in operator_queue_service.py was env-tunable
in code but unreachable from .env — its siblings (OPERATOR_QUEUE_TITLE_MAX,
OPERATOR_QUEUE_EXECUTION_ID_MAX) are wired per the #1632 convention, so wire it
the same way. Default unchanged (320, RFC 5321).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
….js against #2298

Both PRs added state fields at the same insertion point (kept both), and #2298
(#2261) moved the store onto the dedicated portalHttp instance — this branch's
two new asks calls move with it, exactly as the fetchAsks comment promised
('when that lands, these two calls move with the other 29'), and
workspaceAsks.spec.js stubs portalHttp instead of the global axios (the
workspaceSession.spec.js pattern). All 48 tests across the three affected
specs pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated via /validate-pr: dual-track migration present (migrations.py + Alembic 0039, pg-migrations + schema-parity green), all pytest seeds + e2e + build green after the dev merge-resolution, security scans clean, architecture.md updated, tests on both layers (test_ent364_addressed_asks.py + workspaceAsks.spec.js — 48 specs pass locally post-portalHttp migration). Conflict with #2298 resolved on-branch per the fetchAsks comment's own plan; OPERATOR_QUEUE_EMAIL_MAX wired into compose + .env.example per the #1632 sibling convention. Approving.

@vybe
vybe merged commit 16caacb into dev Aug 19, 2026
26 checks passed
trinity-ability pushed a commit that referenced this pull request Aug 19, 2026
…039→0040

dev merged 0039_operator_queue_addressed_to (ent#364, #2300) after this branch
was cut, forking the Alembic version-line (two heads off 0038_portal_chat_state
— upgrade head would apply zero revisions on PostgreSQL). rl_events_failure_kind
is renamed to 0040 with down_revision = 0039_operator_queue_addressed_to; the
SQLite MIGRATIONS registry keeps both entries in the same order. Doc references
updated (subscription-auto-switch.md, subscription-usage-tracking.md).

Verified: check_alembic_heads PASS (41 revisions, 1 head); schema-parity +
alembic guard suites 71 passed; test_471 + pingpong 55 passed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dolho added a commit that referenced this pull request Aug 21, 2026
…e surface that never rendered (ent#429)

Slices 3+4 of ent#364. Most of the rendering shipped in #2300; auditing the
acceptance criteria against the code left three things genuinely undone, one of
them a promise the feature has been making falsely since it landed.

**AC #2 — nothing homeless.** `workspace_session_id` was READ by the ask
projection and written by nothing, so `chat_id` was always null and an ask
raised by a scheduled run belonged nowhere. It is now resolved at RAISE time
(`client_portal.service.ensure_thread_for_ask`), reusing the client's existing
thread with that agent and opening one only if they have never chatted — the
same `_resolve_session_id(..., None)` a first client turn takes, so an ask lands
IN the conversation rather than beside it. Render-time resolution would not be
an attachment: it is a guess repeated per view, with nothing durable to audit.

**And it is platform-written, which nothing enforced.** The projection's
docstring already promised "platform-written context only" while the clamp
happily passed an agent-authored `workspace_session_id` straight through —
`chat_id` is where the Workspace SENDS a reader, so an agent could choose that
destination. Stripped unconditionally now, before the real one is written, and
stripped whether or not an addressee resolves.

Three ordering details, each load-bearing:

  * the addressee is resolved BEFORE the context block, because the thread id is
    written INTO context and doing it after would add bytes the size cap had
    already signed off on;
  * the context dict is REBUILT, not popped — `out = dict(req)` is a shallow
    copy, so popping would reach into the caller's own dict and the clamp's
    contract is that it never mutates the request;
  * the thread id survives the oversize-context marker, or an agent-controlled
    oversize context would be the one way to produce a homeless ask.

Fail-SOFT, deliberately the opposite direction to `_validated_addressee` beside
it: that fails closed because it is an authorization decision, this one only
decides where a link points, and losing the whole question over a missing link
is the worse trade.

**AC #1 — the inline-in-chat surface has never rendered.** `PortalConversation`
passes `props.agent`, the agent OBJECT, to `asksForAgent`, which compares it
against the string `agent_name`. It matched nothing, and the wrapper is
`v-if="agentAsks.length"`, so the third of ent#364's three surfaces silently did
not exist. Verified with a throwaway probe before believing it:

    asksForAgent('scout')          -> 1 ask
    asksForAgent({name: 'scout'})  -> 0 asks

Pinned against the SOURCE, because this repo's vitest runs in `node` with no
component-mount harness — crude, but it is the difference between catching this
class again and not.

**AC #3 — expiry now says WHEN.** The row said "This expired before it was
answered" with no time, and the #1142 sweep DELETES terminal rows, so between
lapsing and being swept this is the only evidence the question was ever asked.
An hour ago and last March are different situations. `expiredLabel` lives in
`portalUtils` with `now` injected, per this file's rule that a sentence composed
inside a component is a sentence no test can reach; it degrades to the bare
wording on a missing, garbled or future timestamp, because a wrong time is worse
than no time — the reader would act on it.

Plus `askThreadLink`: "Open the conversation", shown only when the ask names a
thread the reader is not already in. Additive — it never hides an ask from the
thread being read. Narrowing the inline surface to only its own thread is the
open question, not this.

All Workspace work in the OSS repo, per the standing rule.

Backend slice: 687 passed, 1 skipped. Frontend: 52 files / 1097 tests, tokens
OK, build OK. 6 of the 9 new backend cases fail without this change; the other 3
are regression guards for properties it could have broken.

Related to ent#429.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
trinity-ability pushed a commit that referenced this pull request Aug 21, 2026
…nt#428)

`operator_queue.addressed_to_email` shipped in #2300, but nothing could ask the
DB for "the asks addressed to this person" — the only consumer read a fleet-wide
page of pending items and filtered the addressee out of the result.

That is wrong at the layer, not just inefficient. `list_items` orders by status,
then priority, then age, and applies `limit` before the caller ever sees a row,
so a post-hoc filter really means "the newest N pending items in the FLEET, of
which some happen to be yours". One person's low-priority ask falls out of the
window as soon as the fleet is busy — it disappears from the surface that
addressed it while still sitting pending in the queue, and by construction
nobody else is permitted to answer it.

So `list_items` takes an optional `addressed_to_email`, compared
case-insensitively: the ingestion boundary lowercases before it stores
(`_validated_addressee`), but `create_item` is a public writer and the read must
not silently depend on every future caller having remembered that.

Also annotates `respond_to_item(responded_by_id=...)` as Optional. It is a
`users` id, and an ask answered by someone with no platform account has no row
there — NULL id plus the answering email is what distinguishes that from an
answer by an operator whose account was since deleted, which keeps its id. The
annotation records the intent so it is not later "tidied" back to `str`.

Additive: no migration, no route change, no default behaviour moved — omitting
the argument returns exactly what it returned before, pinned by a test.

Related to ent#428.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
trinity-ability pushed a commit that referenced this pull request Aug 21, 2026
…e surface that never rendered (ent#429)

Slices 3+4 of ent#364. Most of the rendering shipped in #2300; auditing the
acceptance criteria against the code left three things genuinely undone, one of
them a promise the feature has been making falsely since it landed.

**AC #2 — nothing homeless.** `workspace_session_id` was READ by the ask
projection and written by nothing, so `chat_id` was always null and an ask
raised by a scheduled run belonged nowhere. It is now resolved at RAISE time
(`client_portal.service.ensure_thread_for_ask`), reusing the client's existing
thread with that agent and opening one only if they have never chatted — the
same `_resolve_session_id(..., None)` a first client turn takes, so an ask lands
IN the conversation rather than beside it. Render-time resolution would not be
an attachment: it is a guess repeated per view, with nothing durable to audit.

**And it is platform-written, which nothing enforced.** The projection's
docstring already promised "platform-written context only" while the clamp
happily passed an agent-authored `workspace_session_id` straight through —
`chat_id` is where the Workspace SENDS a reader, so an agent could choose that
destination. Stripped unconditionally now, before the real one is written, and
stripped whether or not an addressee resolves.

Three ordering details, each load-bearing:

  * the addressee is resolved BEFORE the context block, because the thread id is
    written INTO context and doing it after would add bytes the size cap had
    already signed off on;
  * the context dict is REBUILT, not popped — `out = dict(req)` is a shallow
    copy, so popping would reach into the caller's own dict and the clamp's
    contract is that it never mutates the request;
  * the thread id survives the oversize-context marker, or an agent-controlled
    oversize context would be the one way to produce a homeless ask.

Fail-SOFT, deliberately the opposite direction to `_validated_addressee` beside
it: that fails closed because it is an authorization decision, this one only
decides where a link points, and losing the whole question over a missing link
is the worse trade.

**AC #1 — the inline-in-chat surface has never rendered.** `PortalConversation`
passes `props.agent`, the agent OBJECT, to `asksForAgent`, which compares it
against the string `agent_name`. It matched nothing, and the wrapper is
`v-if="agentAsks.length"`, so the third of ent#364's three surfaces silently did
not exist. Verified with a throwaway probe before believing it:

    asksForAgent('scout')          -> 1 ask
    asksForAgent({name: 'scout'})  -> 0 asks

Pinned against the SOURCE, because this repo's vitest runs in `node` with no
component-mount harness — crude, but it is the difference between catching this
class again and not.

**AC #3 — expiry now says WHEN.** The row said "This expired before it was
answered" with no time, and the #1142 sweep DELETES terminal rows, so between
lapsing and being swept this is the only evidence the question was ever asked.
An hour ago and last March are different situations. `expiredLabel` lives in
`portalUtils` with `now` injected, per this file's rule that a sentence composed
inside a component is a sentence no test can reach; it degrades to the bare
wording on a missing, garbled or future timestamp, because a wrong time is worse
than no time — the reader would act on it.

Plus `askThreadLink`: "Open the conversation", shown only when the ask names a
thread the reader is not already in. Additive — it never hides an ask from the
thread being read. Narrowing the inline surface to only its own thread is the
open question, not this.

All Workspace work in the OSS repo, per the standing rule.

Backend slice: 687 passed, 1 skipped. Frontend: 52 files / 1097 tests, tokens
OK, build OK. 6 of the 9 new backend cases fail without this change; the other 3
are regression guards for properties it could have broken.

Related to ent#429.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
trinity-ability pushed a commit that referenced this pull request Aug 21, 2026
…nt#428)

`operator_queue.addressed_to_email` shipped in #2300, but nothing could ask the
DB for "the asks addressed to this person" — the only consumer read a fleet-wide
page of pending items and filtered the addressee out of the result.

That is wrong at the layer, not just inefficient. `list_items` orders by status,
then priority, then age, and applies `limit` before the caller ever sees a row,
so a post-hoc filter really means "the newest N pending items in the FLEET, of
which some happen to be yours". One person's low-priority ask falls out of the
window as soon as the fleet is busy — it disappears from the surface that
addressed it while still sitting pending in the queue, and by construction
nobody else is permitted to answer it.

So `list_items` takes an optional `addressed_to_email`, compared
case-insensitively: the ingestion boundary lowercases before it stores
(`_validated_addressee`), but `create_item` is a public writer and the read must
not silently depend on every future caller having remembered that.

Also annotates `respond_to_item(responded_by_id=...)` as Optional. It is a
`users` id, and an ask answered by someone with no platform account has no row
there — NULL id plus the answering email is what distinguishes that from an
answer by an operator whose account was since deleted, which keeps its id. The
annotation records the intent so it is not later "tidied" back to `str`.

Additive: no migration, no route change, no default behaviour moved — omitting
the argument returns exactly what it returned before, pinned by a test.

Related to ent#428.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
trinity-ability pushed a commit that referenced this pull request Aug 21, 2026
…e surface that never rendered (ent#429)

Slices 3+4 of ent#364. Most of the rendering shipped in #2300; auditing the
acceptance criteria against the code left three things genuinely undone, one of
them a promise the feature has been making falsely since it landed.

**AC #2 — nothing homeless.** `workspace_session_id` was READ by the ask
projection and written by nothing, so `chat_id` was always null and an ask
raised by a scheduled run belonged nowhere. It is now resolved at RAISE time
(`client_portal.service.ensure_thread_for_ask`), reusing the client's existing
thread with that agent and opening one only if they have never chatted — the
same `_resolve_session_id(..., None)` a first client turn takes, so an ask lands
IN the conversation rather than beside it. Render-time resolution would not be
an attachment: it is a guess repeated per view, with nothing durable to audit.

**And it is platform-written, which nothing enforced.** The projection's
docstring already promised "platform-written context only" while the clamp
happily passed an agent-authored `workspace_session_id` straight through —
`chat_id` is where the Workspace SENDS a reader, so an agent could choose that
destination. Stripped unconditionally now, before the real one is written, and
stripped whether or not an addressee resolves.

Three ordering details, each load-bearing:

  * the addressee is resolved BEFORE the context block, because the thread id is
    written INTO context and doing it after would add bytes the size cap had
    already signed off on;
  * the context dict is REBUILT, not popped — `out = dict(req)` is a shallow
    copy, so popping would reach into the caller's own dict and the clamp's
    contract is that it never mutates the request;
  * the thread id survives the oversize-context marker, or an agent-controlled
    oversize context would be the one way to produce a homeless ask.

Fail-SOFT, deliberately the opposite direction to `_validated_addressee` beside
it: that fails closed because it is an authorization decision, this one only
decides where a link points, and losing the whole question over a missing link
is the worse trade.

**AC #1 — the inline-in-chat surface has never rendered.** `PortalConversation`
passes `props.agent`, the agent OBJECT, to `asksForAgent`, which compares it
against the string `agent_name`. It matched nothing, and the wrapper is
`v-if="agentAsks.length"`, so the third of ent#364's three surfaces silently did
not exist. Verified with a throwaway probe before believing it:

    asksForAgent('scout')          -> 1 ask
    asksForAgent({name: 'scout'})  -> 0 asks

Pinned against the SOURCE, because this repo's vitest runs in `node` with no
component-mount harness — crude, but it is the difference between catching this
class again and not.

**AC #3 — expiry now says WHEN.** The row said "This expired before it was
answered" with no time, and the #1142 sweep DELETES terminal rows, so between
lapsing and being swept this is the only evidence the question was ever asked.
An hour ago and last March are different situations. `expiredLabel` lives in
`portalUtils` with `now` injected, per this file's rule that a sentence composed
inside a component is a sentence no test can reach; it degrades to the bare
wording on a missing, garbled or future timestamp, because a wrong time is worse
than no time — the reader would act on it.

Plus `askThreadLink`: "Open the conversation", shown only when the ask names a
thread the reader is not already in. Additive — it never hides an ask from the
thread being read. Narrowing the inline surface to only its own thread is the
open question, not this.

All Workspace work in the OSS repo, per the standing rule.

Backend slice: 687 passed, 1 skipped. Frontend: 52 files / 1097 tests, tokens
OK, build OK. 6 of the 9 new backend cases fail without this change; the other 3
are regression guards for properties it could have broken.

Related to ent#429.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vybe pushed a commit that referenced this pull request Aug 21, 2026
…e agent-page leak, and the module itself (ent#428) (#2356)

* feat(operator-queue): filter the queue listing by addressee in SQL (ent#428)

`operator_queue.addressed_to_email` shipped in #2300, but nothing could ask the
DB for "the asks addressed to this person" — the only consumer read a fleet-wide
page of pending items and filtered the addressee out of the result.

That is wrong at the layer, not just inefficient. `list_items` orders by status,
then priority, then age, and applies `limit` before the caller ever sees a row,
so a post-hoc filter really means "the newest N pending items in the FLEET, of
which some happen to be yours". One person's low-priority ask falls out of the
window as soon as the fleet is busy — it disappears from the surface that
addressed it while still sitting pending in the queue, and by construction
nobody else is permitted to answer it.

So `list_items` takes an optional `addressed_to_email`, compared
case-insensitively: the ingestion boundary lowercases before it stores
(`_validated_addressee`), but `create_item` is a public writer and the read must
not silently depend on every future caller having remembered that.

Also annotates `respond_to_item(responded_by_id=...)` as Optional. It is a
`users` id, and an ask answered by someone with no platform account has no row
there — NULL id plus the answering email is what distinguishes that from an
answer by an operator whose account was since deleted, which keeps its id. The
annotation records the intent so it is not later "tidied" back to `str`.

Additive: no migration, no route change, no default behaviour moved — omitting
the argument returns exactly what it returned before, pinned by a test.

Related to ent#428.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(workspace): the agent page listed every client's asks, not the viewer's (ent#428)

Found by /review on the primitive commit, and reproduced before it was believed:
two clients co-shared on one agent, an ask addressed to the first, and the
second reads its title and question verbatim.

    BOB SEES: [('Invoice Acme at the agreed discount?',
                'Alice, confirm the Q3 discount before I send it.')]

`client_portal/agent_page.py::_asks` is the client-facing "Waiting on you"
block, and it was scoped by `agent_name` alone. One agent is routinely shared
with several people, so that scope was never the audience — it was the only
scope available before ent#364 added the addressee. The file already withheld
`context` as a known leak surface, which is what makes it deceptive: it looks
like a reader that has thought about disclosure. `title`, `question` and
`options` are agent-authored free text and are exactly where an ask meant for
someone else says something not meant for this reader. The section heading was
"Waiting on you" the whole time.

Pre-existing on `dev` since ent#364: that change added the column, its
ingestion-time validation and three new renderings that honour it, and did not
teach this older reader. Fixed here rather than filed, because this is the PR
that adds the primitive it needs.

`viewer_email` is REQUIRED, not defaulted — a default is how the next caller
silently gets the unfiltered list back.

Unaddressed operator asks stop rendering here, and that narrowing is intended:
this is a client-facing page, an operator ask is agent-authored text written for
the operator, and a client cannot act on one from here anyway ("reply in chat"
is the only affordance). Operators keep the full queue in Operations, and the
page now agrees with what the Workspace's other ask surfaces show that person.

Also makes the filter fail CLOSED. `list_items` uses truthiness for every other
filter, so a falsy value means "don't filter"; for this argument that would turn
a caller which lost its email into a caller that sees everyone's asks. `None`
still means no filter (the operator listing); `""` now matches nothing.

The two `test_ent360` failures were signature-only — both stub the query out, so
their subjects (alert-type exclusion, `context` never forwarded) are untouched;
they just pass the viewer now.

Three new tests, all three failing without this change. Slice re-run:
655 passed, 1 skipped.

Related to ent#428.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(workspace): move agent-initiated asks into OSS core (ent#428)

Everything about the Workspace lives in OSS. This surface was built as the
entitled `workspace_asks` module in the private repo, and that was the wrong
edition for exactly the reason ent#356 (the Workspace itself) and ent#443
(multi-agent rooms) were: the frontend that drives it — `PortalAsks.vue`,
`store.fetchAsks()`, `store.asksAvailable` — ships in EVERY build and
self-disables on a 404. So a community install rendered the ask affordance and
then refused it: an advert for a missing feature rather than a clean absence.

Under `client_portal/`, not a new top-level package. That is what it is — a
client-portal surface, on the client-portal prefix, owning no table (an ask IS
an `operator_queue` row with an addressee, which is what makes "answering
anywhere clears it everywhere" true by construction). It also means the module
inherits `client_portal`'s Dockerfile COPY line instead of needing its own: a
new top-level package that `main.py` imports at module scope builds a clean
image that dies at import, and that has now been #1033, ent#356 and ent#443.

Verbatim port. The service and models are unchanged; the router loses its
`requires_entitlement` dependency and the service its `FEATURE_ID`. Nothing else
moved, so the diff reads as a move.

RETAINED, deliberately: the `/api/enterprise/client-portal/asks` prefix. ent#83
published it as the headless integration surface and the shipped Vue bundle
already calls it — renaming breaks a client for nothing. Provenance, not a
licensing claim, exactly like the `enterprise_`-prefixed portal tables it reads
beside.

Transition: the OSS router is included BEFORE `register_enterprise(app)`, so on
an install whose submodule still registers the gated module both mount and the
ungated OSS one wins the match order. Verified live on a pure-OSS build with an
un-bumped submodule — OSS route first, `entitled features: []`.

The suite moves with it (`tests/unit/test_ent428_workspace_asks.py`), unchanged
beyond import paths and one path calculation, plus three cases the ported ones
structurally cannot see: the routes answer with no entitlement, nothing calls
`requires_entitlement`/`register_module` (parsed, not grepped — a docstring may
still SAY it used to be gated), and the mount precedes the seam.

That last assertion is anchored on the CALL, not on `str.index` of the name:
`main.py` mentions `register_enterprise(app)` in prose above it, so a substring
match finds the comment. It false-failed here on the first run. #2355 carries
the same weaker form — it can only false-fail there, never false-pass, and is
worth tightening separately.

Local: 22 in the ported suite; `-k "portal or agent_page or ent364 or ent428 or
operator or queue or asks or entitle or version"` → 808 passed, 1 skipped.

Related to ent#428.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(workspace): review follow-ups on the ask move — stale edition claims, and an N+1 (ent#428)

Four findings from /review on the move commit. None in the ported logic; all
four are things the move itself created or exposed.

**Three now-false edition claims.** Moving the module made public prose lie:

* `docs/memory/architecture.md:1241` said "the read/answer surface is the
  ENTITLED `workspace_asks` module". A public doc calling a public module paid
  is the wrong direction of wrong — it is the sentence a reader would trust.
* `stores/clientPortal.js` explained its own 404/403 guard as "the module is
  enterprise-gated, so an OSS or unentitled build answers 404/403". The guard
  still belongs — an OLDER backend either predates the surface or still gates
  it — but the stated reason would tell a reader asks are a paid feature.
  Comments only; the logic is untouched and still correct.
* `asks/models.py` still called itself `Private.`

This does mean the PR now touches one frontend file, so the "no frontend" claim
in the body is corrected rather than quietly left standing.

**An N+1 the move promoted to an OSS polled endpoint.** `list_asks` called
`_on_roster` per ITEM. That resolves to `agent_name in roster_agent_names(...)`,
one-to-two DB reads, for an answer that cannot change inside a single request —
on an endpoint the Workspace polls every 20s, per signed-in client, per open
tab. A client's asks cluster on one or two agents, so this is now 1-2 reads
instead of N.

The FIRST attempt at that fix was wrong and the tests said so: hoisting
`roster_agent_names` out of the loop turned six of them red, because the suite's
seam is `agent_on_roster`. That was correct feedback rather than an awkward
fixture — `agent_on_roster` IS the access predicate ("the scope of what a caller
can DO must equal the scope of what they can SEE"), and re-implementing
membership beside it is exactly how the two drift apart. Redone as a per-REQUEST
memo over that same predicate: same win, seam intact, fail-closed preserved per
agent.

Pinned on the CALL COUNT, not on timing — a duration assertion would be flaky
and would not say what it means. Proven non-vacuous:

    AssertionError: roster consulted 8 times for 8 asks across 2 agents

Slice: 678 passed, 1 skipped. Guard sweep (`centralized|parity|invariant|
architecture|wiring|1310|186|293|926|enumeration`): 816 passed, 3 skipped.

Related to ent#428.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(workspace): attach an ask to a chat at raise time, and revive the surface that never rendered (ent#429)

Slices 3+4 of ent#364. Most of the rendering shipped in #2300; auditing the
acceptance criteria against the code left three things genuinely undone, one of
them a promise the feature has been making falsely since it landed.

**AC #2 — nothing homeless.** `workspace_session_id` was READ by the ask
projection and written by nothing, so `chat_id` was always null and an ask
raised by a scheduled run belonged nowhere. It is now resolved at RAISE time
(`client_portal.service.ensure_thread_for_ask`), reusing the client's existing
thread with that agent and opening one only if they have never chatted — the
same `_resolve_session_id(..., None)` a first client turn takes, so an ask lands
IN the conversation rather than beside it. Render-time resolution would not be
an attachment: it is a guess repeated per view, with nothing durable to audit.

**And it is platform-written, which nothing enforced.** The projection's
docstring already promised "platform-written context only" while the clamp
happily passed an agent-authored `workspace_session_id` straight through —
`chat_id` is where the Workspace SENDS a reader, so an agent could choose that
destination. Stripped unconditionally now, before the real one is written, and
stripped whether or not an addressee resolves.

Three ordering details, each load-bearing:

  * the addressee is resolved BEFORE the context block, because the thread id is
    written INTO context and doing it after would add bytes the size cap had
    already signed off on;
  * the context dict is REBUILT, not popped — `out = dict(req)` is a shallow
    copy, so popping would reach into the caller's own dict and the clamp's
    contract is that it never mutates the request;
  * the thread id survives the oversize-context marker, or an agent-controlled
    oversize context would be the one way to produce a homeless ask.

Fail-SOFT, deliberately the opposite direction to `_validated_addressee` beside
it: that fails closed because it is an authorization decision, this one only
decides where a link points, and losing the whole question over a missing link
is the worse trade.

**AC #1 — the inline-in-chat surface has never rendered.** `PortalConversation`
passes `props.agent`, the agent OBJECT, to `asksForAgent`, which compares it
against the string `agent_name`. It matched nothing, and the wrapper is
`v-if="agentAsks.length"`, so the third of ent#364's three surfaces silently did
not exist. Verified with a throwaway probe before believing it:

    asksForAgent('scout')          -> 1 ask
    asksForAgent({name: 'scout'})  -> 0 asks

Pinned against the SOURCE, because this repo's vitest runs in `node` with no
component-mount harness — crude, but it is the difference between catching this
class again and not.

**AC #3 — expiry now says WHEN.** The row said "This expired before it was
answered" with no time, and the #1142 sweep DELETES terminal rows, so between
lapsing and being swept this is the only evidence the question was ever asked.
An hour ago and last March are different situations. `expiredLabel` lives in
`portalUtils` with `now` injected, per this file's rule that a sentence composed
inside a component is a sentence no test can reach; it degrades to the bare
wording on a missing, garbled or future timestamp, because a wrong time is worse
than no time — the reader would act on it.

Plus `askThreadLink`: "Open the conversation", shown only when the ask names a
thread the reader is not already in. Additive — it never hides an ask from the
thread being read. Narrowing the inline surface to only its own thread is the
open question, not this.

All Workspace work in the OSS repo, per the standing rule.

Backend slice: 687 passed, 1 skipped. Frontend: 52 files / 1097 tests, tokens
OK, build OK. 6 of the 9 new backend cases fail without this change; the other 3
are regression guards for properties it could have broken.

Related to ent#429.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(operator-queue): the ingestion clamp is not pure, and its docstring said it was (ent#429)

Review follow-up. `_clamp_ingested_item` is named and documented as a
field-hygiene clamp — "Returns a NEW dict; never mutates the caller's request" —
and a reader reasonably concludes it is safe to call speculatively, just to see
what a clamped item would look like.

It is not, and has not been since ent#364: it resolves `addressed_to_email`
against the agent's roster, which is a DB READ and an authorization decision.
ent#429 went further and gave it a DB WRITE — `_workspace_thread_for` may create
a portal session.

That is fine where it is. The write has to precede the context size check,
because the thread id is written INTO context and adding it afterwards would
append bytes the cap had already signed off on. What is not fine is a docstring
that describes the opposite, so it now names both effects before the field list,
and says what the second caller would cost: an empty client thread left behind
by a clamp whose result was never created.

No behaviour change. Slice: 687 passed, 1 skipped.

Related to ent#429.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants