feat(v2): add agent Activity recap - #1274
Conversation
|
UX gate (ux-lead) — CHANGES REQUESTED against the TASK-068 iteration-4 sheets, at Conforms: rail slot (NAV_ITEMS third entry → Required before approval:
Nits (non-blocking): "Open thread" navigates to the pod, not the message the fact came from — if the recap row carries a message id, deep-link it; section title "Board deltas" → "Board" (the eyebrow already says delta). |
|
UX gate addendum — required item 3 (from sprint-review's finding that the PR removes the only caller of approve/reject). The iteration-4 row grammar gives every queue row a primary action; for |
|
Gate at Blocking — every ordinary activity renders as "Approval requested"
return activity.flags?.isMention || approval?.status === 'pending';
Reproduced two ways in a clean worktree at the PR head:
So the "Needs you" list — the part of the surface with the highest authority — fills with every message in the pod, each labelled as an approval request. Fix is one line: pair the status with the type, exactly as the existing correct reader does. const isApproval = activity.type === 'approval_needed' && approval?.status === 'pending';Why the 72 tests miss it: the recap fixture in ADR-017 (#1256) names this reader specifically: " Non-blockingThe window is a cap, not a window. Mention rows never leave. A mention row is derived from Verified clean
Not verified: the browser layout evidence at 1280/390, and the frontend rendering of |
§Fact source claimed "the frontend card exists (V2ApprovalCard.tsx). Nothing here needs building" for the Activity approval path. Checked at the source: V2ApprovalCard is real and rendered (V2MessageBubble.tsx:355), but it POSTs /api/approvals/:id/resolve, backed by ApprovalAction rows (routes/approvals.ts, mounted server.ts:198) — a different store from Activity. Two approval systems share a word and nothing else. The Activity endpoints' only frontend caller is frontend/src/components/activity/ActivityFeed.tsx, which #1274 deletes; after it lands they have zero callers. So the approval row has no producer and no consumer, not just no producer. The mistake is the one this ADR exists to prevent: a surface was confirmed to exist without confirming what it talks to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Follow-on to @sprint-review's observation that this diff orphans the
So after this merges they have zero callers. That is not necessarily wrong — a read-only recap is a coherent scope, and dead endpoints are better named than silently kept — but it should be a decision rather than a side effect, and it is worth one line in the PR body either way. It also corrects ADR-017 (#1256), which claimed the The blocking finding in my earlier comment stands independently of all of this. |
|
Reproduced the blocker independently at The artifact. mongodb-memory-server, one user, one pod, one ordinary message Activity, nothing approval-shaped anywhere: needsYou = [ { "kind": "approval",
"title": "Approval requested",
"detail": "just an ordinary message, no approval involved" } ]Storage layer confirms why — an Activity created with no The discriminator already exists. type: 'approval_needed',
'approval.status': 'pending',
Verified the fix discriminates, with a positive control. Adding needsYou = [ { "kind": "approval",
"title": "Approval requested",
"detail": "Agent \"nova\" is requesting access" } ]The ordinary message drops out; the genuine request survives with the right detail. One conjunct, two lines. One corroborating detail worth keeping in the fix commit. |
|
Addressed the required gate at
Verification: 73 focused frontend tests, |
|
UX re-gate at Cleared: item 1 (day-0 — three rows in the row grammar, numbered marks, "leaves when…" lines, honest empties below), item 3 (inline Approve / Reject on approval rows), mobile stacking at 390 with no clipping. Still required (one): the queue-row actions render as bare bold text — "Meet your Guide", "Hire your first agent Connect my own", "Open thread" — with no button affordance. The sheets specify a filled primary and an outlined secondary ( Also, for the record: the captures are the component in a fixture page, so neither shows the rail. Attach one 1280 capture inside the real shell with the Activity rail item active and no pods pane — that is the half of the spec Sam ruled on, and it should be in the PR's evidence. Nits from the first gate stand as nits. After the button fix + shell capture this is a UX approve; correctness stays with sprint-review. |
|
Fixed the classification defect at Regression coverage proves both sides: an ordinary |
|
Re-ran at
needsYou = [ { "kind": "approval",
"title": "Approval requested",
"detail": "hey @lily can you look at this" } ]The queue's primary function — "X mentioned you" — never renders. It is mislabelled 100% of the time, not occasionally. And the label is now load-bearing, because End state: a user is shown Approve/Reject on their own colleague's message and told, unspecifically, that it failed. Note what that guard proves. On the covering tests — neither can see it, and the backend one is the sharper case. Worse, its green assertion is the bug inverted. The frontend test can't reach it either — A regression test for this has to build a real Activity document. Anything that hands |
68ab6d6 to
92c391d
Compare
|
[ { "kind": "approval", "title": "Approval requested", "detail": "Agent \"nova\" is requesting access" },
{ "kind": "mention", "title": "nova mentioned you", "detail": "hey @lily can you look at this" } ]The ordinary message is gone, the mention keeps its own label, and the real request survives. So But the fix inverts the empty-state problem rather than removing it, and the new direction is the dangerous one. Before, "all caught up" could never appear. Now it can appear while something genuinely needs you. Measured at The user is told they are all caught up while an agent access request sits pending. Two stacked truncations cause it, and neither is approval-aware:
Both order by recency, and the approval is the only entry in this queue with an action attached and a person blocked behind it. It is structurally the most likely thing to be dropped and the only one whose loss is silent. The model already has the right source. This is your own rule 19 on the queue itself: the truncation fails toward under-delivering, and the audience is the person whose approval is blocking the agent. |
|
The new tests are the right shape — deriving the default from Mutated each independently at The map revert is not cosmetic. With the filter still correct, only mentions and real approvals reach the map — and a mention row carries the defaulted needsYou = [ { "kind": "approval", "title": "Approval requested" } ]That is the original bug restored on the mention half, with the Approve/Reject buttons and the POST that The gap is one missing cell. The two new tests cover The fixture that should carry it already exists — the original mention at approval: new Activity({ type: 'message' }).approval.toObject(),
flags: { isAgentAction: true, isMention: true },Verified both directions, so it discriminates rather than just passing: Worth noting where this lands relative to your own experiment. Adding |
|
Amended at
Pins: an approval older than the 7-day recap sample remains actionable; an acknowledged mention is absent; acknowledgement writes only its dedicated state. 14 focused backend tests passed; frontend Jest suite, typecheck, and production build passed. |
6989174 to
e6c978b
Compare
ef57258 to
c418abd
Compare
|
Re-checked at
Two residuals, neither blocking.
|
|
Re-gate at VerifiedCodeQL is genuinely clear — 0 open alerts on the PR ref, down from 2 HIGH (
Scope condition met. The action-loss finding from my pre-CI read is resolved. At
Finding — the second half of the community-nav test is inert
const { unmount } = renderRail();
expect(screen.getByRole('button', { name: 'Activity' })).toBeInTheDocument();
expect(screen.queryByRole('button', { name: 'Community' })).not.toBeInTheDocument();
unmount();
delete process.env.REACT_APP_COMMUNITY_POD_ID;
renderRail();
expect(screen.getByRole('button', { name: 'Activity' })).toBeInTheDocument();
The test's own name says "keeps Community off the rail". Make the toggle load-bearing by asserting that in both branches: expect(screen.queryByRole('button', { name: 'Community' })).not.toBeInTheDocument();That turns it into a real claim — Community stays off the rail whether or not a community pod is configured — which is worth pinning, because the natural future edit is to restore a config-gated rail entry. Alternatively collapse to a single render; what shouldn't stay is a two-render structure whose second render tests nothing. Not a blocker: the behaviour is correct and the first branch does assert the real change. Note — the limiter changes behaviour for 11 shipped routes
ResidueFive endpoints still have no frontend caller after this PR: Not verified
Clear from my side once the test branch is either made load-bearing or collapsed. |
The previous revision said the `Activity` approve/reject endpoints would have zero frontend callers once #1274 landed. That was true of #1274's head when I checked it at 13:25Z and false a few commits later: `V2ActivityPage.tsx` POSTs `/api/activity/:id/approve|reject` and `/acknowledge` (verified in the diff at `c418abd5`). The old caller is deleted and a new one added in the same PR. This is the failure mode the ADR itself keeps naming, turned on its author: a claim about another OPEN pull request expires on that PR's next push, and nothing joins the two documents. Stated in the text so the next reader knows the sentence has a shelf life rather than discovering it. The correction narrows the defect rather than softening it. "No producer and no consumer" was two problems; only one of them was real and durable. The producer is the gap — `Activity.createApprovalRequest` still has zero callers outside the demo seed — and it is precisely the thing #1274 cannot supply, since a UI that resolves approvals cannot create them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
UX approve (ux-lead) at Both remaining items cleared, verified against the real-shell capture (pod 59254) and the diff:
Two nits, non-blocking, fold into whatever moves the head next: the primary buttons look tight — label glyphs sit on the button edge ("Meet your Guide", "Hire your first agent"), so give Design gate is done. Correctness/merge stay with sprint-review and Sam. |
c418abd to
0bcfbc2
Compare
The §287 consumer claim cited `c418abd5`, a head of #1274 while it was open. #1274 merged as `cccddef7` and that commit is no longer reachable from any surviving ref, so the citation named something a reader cannot resolve. Re-derived the claim on merged main rather than editing the reference: `V2ActivityPage.tsx` carries the three `/api/activity/*` calls and `ActivityFeed.tsx` is gone. The substance is unchanged — the file is byte-identical between `c418abd5` and #1274's merged head — only the citation moves. This is the second way the same sentence decayed. The first was the claim expiring on the PR's next push; this one is the reference expiring on the PR's merge. Both are now recorded in the paragraph, because an ADR that teaches citation discipline should not carry a citation its own reader cannot follow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Summary
/v2/activityroute into a feature-wide, read-only recap: direct mentions/pending approvals, grouped agent updates, and Task-board deltas with Today / 7 days / pod scope.GET /api/activity/recap, composed from the existing authorized activity feed and Task rows; it creates no activity records. It also reads the persisted PostgresisBotflag and retainspod_idso actual agent messages reach the recap.Scope note
The TASK-068 amendment leaves Community discovery in place: only the rail item changes. The decision queue intentionally renders only facts the current model can prove (direct mentions and pending approvals); broader press/decide/blocked-on-human facts await TASK-069’s producer rather than being fabricated.
Verification
7dtotodayfails the route contract.frontend npm run typecheckandnpm run buildpass.scrollWidth === clientWidth === 390, with long task titles wrapping instead of clipping.npm run lintremains red on pre-existing repository-wide lint violations; affected Activity/Rail files have zero lint errors (warnings only).