feat(team): surface unverifiable live seats - #1521
lilyshen0722 wants to merge 2 commits into
Conversation
697e8c1 to
b4bc53d
Compare
lilyshen0722
left a comment
There was a problem hiding this comment.
SR-GATE: APPROVED @ b4bc53d7 — with one finding that is the same defect class this PR sits inside, one level up.
deriveAgentOutputState is a clean pure seam and the distinction it draws is the right one: liveness proves the runtime is up, only a persisted message proves visible output. Baseline 17/17 (Node 22). I checked the mutation claim in your PR body rather than taking it: deleting the observed-output branch reddens exactly a recent persisted message verifies output even when another liveness source is delayed, alone.
Non-blocking: a Postgres outage renders the whole roster UNVERIFIABLE, and that is indistinguishable from every seat being genuinely silent
pod-agents.ts:229-242 wraps the batch last-message lookup in a try/catch that logs a warn and leaves lastMessageByUserId empty — deliberately, and the comment says why ("a PG hiccup must never fail the roster"). But every agent then reaches buildAgentInstallationPayload with lastMessage: null, so helpers.ts:442 calls deriveAgentOutputState(liveness, null).
Measured, not argued:
live seat, spoke 5m ago -> observed
live seat, lastMessage=null -> unverifiable
and the catch produces that null for every seat. So a store outage and a genuinely quiet fleet render identically. The human is told their agents are not producing when the fact is that we could not check.
Two reasons I think it is worth closing rather than shrugging at:
- The type already has the value.
AgentOutputStateincludes'unknown', currently reachable only when there is no liveness signal at all. A failed lookup is the other natural'unknown'. - It is the pattern @pod-architect established one PR ago.
findActivityHintreturned{ count: 0 }on failure — the same value a quiet pod returns — and #1519 fixed it by addingunavailable: trueso the caller can tell them apart. This is the identical shape: an advisory read whose failure collapses into its modal success value. Same remedy: let the caller pass a "lookup failed" signal and return'unknown'.
I am not blocking on it because TASK-113's ask — a live-but-silent seat must not read as quiet — is delivered, and this case sits one step beyond that row.
Smaller notes
helpers.ts:442passeslastMessageSnippet ? lastMessage?.createdAt || null : null, so a message whose content trims to empty does not count as output. That is defensible for "visible output" and matches the snippet contract; worth a word in the docstring since the parameter is namedlastMessageAtand a reader would not expect content to gate it.- A seat that spoke two hours ago with no liveness signal returns
'unknown'rather than'quiet'. Defensible — liveness genuinely is unknown — but the name suggests "we know nothing", when in fact we know it once spoke.
Scope limits
Service Tests (Tier 1 — real DBs) is pending; everything else passes. I did not run the frontend suites — no frontend/node_modules in this checkout — so V2YourTeamPage.tsx, the two locale strings and V2YourTeamTiers.test.tsx are read-only review, and your 20/20 and the UI-guard mutation are on report.
lilyshen0722
left a comment
There was a problem hiding this comment.
UX-GATE: CHANGES_REQUESTED @ b4bc53d — on the compact card the new state is appended inside .v2-team-card__pod, a white-space: nowrap; text-overflow: ellipsis line, so it is clipped at every width: measured live (#1521 markup injected into the served /v2/agents, smoke account) the line is 216px wide against 512px of text at 1440 → "in My Workspace · UNVERIFIABL…", and 106px at 390 → "in My Workspa…" — the state is invisible on a phone, and "2 min ago" is dropped at both widths.
The featured-card path (.v2-team-feature__doing, wraps) reads fine; only the compact path fails.
Fix (no new tokens needed):
- Give the state its own line on the compact card, above
.v2-team-card__pod: e.g.<div className="v2-team-card__state" data-testid="team-output-unverifiable">…</div>withfont-size: var(--v2-fs-label); line-height: var(--v2-lh-label); font-weight: 600; letter-spacing: 0.02em; color: var(--v2-text-muted); white-space: normal;— it is a status label, and the label step of the ladder is where an all-caps token belongs (at 13px body weight it shouts). - Leave
.v2-team-card__podas it was (in <pod> · last seen), so the time survives. - Add the pin to
v2-layout-invariants.test.ts: the state rule carrieswhite-space: normal(or is not inside a nowrap ancestor) — this is exactly the class of bug the presence test exists for.
Re-gate on the new head; I will re-measure the same two widths. Captures: .dev/t113-1521-1440-card.png, .dev/t113-1521-390-card.png (pod copy alongside).
lilyshen0722
left a comment
There was a problem hiding this comment.
SR-GATE: APPROVED @ 7f120422 — carry-forward on the backend, plus a look at the new UI delta.
The four changed files are all frontend (V2YourTeamPage.tsx, its test, v2-layout-invariants.test.ts, v2.css). agentStateService.ts and registry/helpers.ts are byte-identical to b4bc53d7 on their +/- lines, so the deriveAgentOutputState review and its mutation result carry unchanged — and so does the non-blocking finding on that gate: a failed last-message lookup still renders the whole roster unverifiable.
The delta itself is right, and the new invariant is better than a presence test: it pins source order (v2-team-card__output-state before v2-team-card__pod), which is the property that actually keeps the state off the one-line-ellipsized pod row. Verified independently — :414 precedes :418 in the component, and the rule carries all four asserted declarations (font-size: 11px, line-height: 14px, font-weight: 600, overflow-wrap: anywhere). Sheet still balances at 1330 open braces, depth 0.
Frontend suites not run here (no frontend/node_modules), so the above is re-derivation plus a brace check, not execution.
lilyshen0722
left a comment
There was a problem hiding this comment.
UX-GATE: APPROVED @ 7f12042 — the state now has its own wrapping label line above the pod line and nothing is clipped at either width.
Measured live (branch rule + markup injected into the served /v2/agents, smoke account, compact card):
- 1440:
.v2-team-card__output-state11px/14px 600rgb(95,100,112), 216px wide, wraps to 2 lines, scrollWidth == clientWidth (not clipped); pod line "in My Workspace · 1d ago" 13px/20px, 1 line, not clipped; state precedes pod in DOM; card 114px tall, no overflow. - 390: state wraps to 3 lines, not clipped; card 128px, no overflow;
scrollWidth390. - Featured path unchanged (
.v2-team-feature__doingwraps).
One pre-existing note, not a blocker and not introduced here: at 390 the pod/time line itself still ellipsizes ("in My Workspa…", 106px vs 159px of text) — that is main's behaviour with any pod name and is already on the Phase B list (.v2-team-card__pod 13px nowrap → TASK-124).
Captures: .dev/t113-1521b-1440-card.png, .dev/t113-1521b-390-card.png (pod copy alongside).
|
Closing under Sam’s v2-shell freeze; TASK-113 is cancelled before merge. |
Summary
UNVERIFIABLE, with English and Chinese copyVerification
backend: npx jest --runInBand __tests__/unit/services/agentStateService.activity.test.js __tests__/unit/routes/registry.last-message-snippet.test.js __tests__/unit/routes/registry.pod-agents-activity.test.js(17 passed)frontend: npx jest --watchAll=false --runInBand src/v2/__tests__/V2YourTeamTiers.test.tsx src/v2/__tests__/V2ConnectHonesty.test.ts(20 passed)backend: npm run tsc:checkfrontend: npm run typecheckunverifiableguard makes its paired test fail.frontend npm run lintstill reports existing errors outside this diff; the changed V2 files add no lint errors.