Files
xarmian 11f67b0a98 fix: timeline can answer has_more=true with zero entries, and the client cannot page past it (BUG-2765) (#1202)
* fix(server): timeline returns the cursor for its next page (BUG-2765)

The timeline over-fetches 3x per source and drops rows that cannot render
(read/searched actions, empty-metadata updates, activities a version or a
comment already stands for, collapsed autosave bursts), so a page can carry
fewer entries than the rows it consumed — or none, while has_more is true.

The client derived its cursor from the last RENDERED entry, which fails in two
ways. With no entries it cannot form a cursor at all, so the first page is a
dead end. With a fully-dropped window LATER in the history it re-sends the same
cursor forever: nothing is appended, the oldest entry does not move, and paging
is wedged at that position permanently. The filing named the first; the second
is the one that bites an ordinary item, since a run of read activities anywhere
in its history is enough.

Both are the same root cause — the response says WHETHER to continue and not
WHERE — so the server now returns next_before / next_before_id whenever
has_more is true:

- page truncated: the last entry KEPT, because the ones cut off must be
  re-fetched. Unchanged from what the client derived.
- window exhausted: the NEWEST tail among the sources that filled their window.
  A short source has nothing older to come back for and must not drag the
  cursor forward; resuming at the oldest tail instead would step over a newer
  source's unexamined rows, and repeats are absorbed by the client's dedup
  while gaps are not recoverable.

Progress is guaranteed because every candidate is a row this page fetched and
the store's cursor predicate is strict.

Tests: an all-dropped window returns a cursor that reaches the history behind
it; paging across a dropped MIDDLE stretch terminates and yields each
renderable entry exactly once; and a control leg pins that an untruncated page
still resumes at its last rendered entry — without it, "always resume from the
oldest row touched" passes while silently skipping what truncation cut. The
two-full-source selection rule is pinned as a unit test, because a full source
whose rows RENDER puts 3x limit entries on the page and takes the truncation
branch instead, so the handler cannot cheaply reach it.

Mutation matrix, each independently detected: cursor from rendered entries;
oldest tail instead of newest; the full-window flag ignored; no cursor at all.

Client half follows in the next commit.

Refs: BUG-2765

Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X

* fix(web): page the timeline with the server's cursor (BUG-2765)

Client half. The component derived its next-page cursor from the last RENDERED
entry, so a page the server had emptied by dropping rows either gave it nothing
to page from (the first page, where loadMore returned early on
entries.length === 0) or gave it the SAME cursor it already held (a later page,
where nothing was appended and the oldest entry did not move). The second case
is a permanent wedge with a live button and a running spinner.

It now pages with next_before / next_before_id when the server sends them, and
falls back to the last entry otherwise — which is exactly the old behaviour,
including its wedge, and is there only for a server that predates the field.

One press walks at most MAX_EMPTY_HOPS pages while every row keeps dropping.
That bound is UX, not correctness, and its comment says so: a single hop is
already correct now that the cursor advances; the loop exists so a user
crossing a long run of read activity sees entries appear rather than a spinner
and nothing, and it is small so a pathological item cannot turn one click into
an unbounded request fan.

Tests assert the cursors the component ASKS FOR, not only what it displays — a
component that shows the right thing by re-fetching page one forever is the
bug. Mutation matrix: ignoring the server cursor fails three of four legs (the
fallback leg survives, correctly, since that is the path it pins); a single hop
fails the advance leg; an unbounded hop count fails the bound leg.

vitest 109 files / 1862 tests pass; vite build clean; svelte-check 0 errors
(6 warnings, all pre-existing and in other files).

Refs: BUG-2765

Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X

* fix: cursor must clear BOTH bounds, and a later page must merge not append (codex round 1)

Two findings, both real, both consequences of the cursor itself.

P1 — truncation and an exhausted window are INDEPENDENT bounds, and the second
is not implied by the first. A source whose rows all drop contributes nothing
to the page, so the truncation cursor can sit older than that source's tail,
and every unexamined row between the two falls in a gap neither page fetches.
The cursor is now the NEWEST candidate across both reasons. Its regression puts
one renderable activity in exactly that gap: two comments forcing truncation at
limit=1, three read rows filling the activity window above them, and the row at
risk in between. Run against the previous commit it comes back 0 times — the
row is not late, it is gone.

P2 — a later page can legitimately carry entries NEWER than the oldest one
already shown, because the cursor deliberately re-covers ground when one
source's window ran out before another's. Concatenating printed those below
older entries. The client merges by (created_at desc, id desc) now, comparing
INSTANTS rather than strings: precision is not uniform — the store writes whole
seconds but a structured note can carry a sub-second timestamp — and
lexicographically "…:05.123Z" sorts before "…:05Z".

Mutation checks: truncation ignoring the exhausted candidate fails the new gap
test; concatenating instead of merging fails the new order test and nothing
else.

internal/server suite green; timeline vitest 9 files / 83 tests green.

Refs: BUG-2765

Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X

* fix(web): stop the hop loop when the cursor does not advance (codex round 2)

Against a server that predates next_before, the fallback re-derives the same
last entry on every hop, so a single Load More click fired five identical
requests where the pre-fix component fired one. The client cannot give an old
server a cursor it does not have — that wedge is the old behaviour and stays —
but amplifying it was new, and mine.

A cursor that did not move cannot make progress, so the loop stops on it. Its
test asserts the request COUNT, which is the only thing that distinguishes this
from the behaviour it replaces: the entries rendered are identical either way.

Refs: BUG-2765

Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X

* fix(web): order the SSE-refresh merge too, and share one comparator (codex round 3)

The refresh prepended its genuinely-new entries on the assumption that a fresh
first page is always newer than everything on screen. Normally it is — but a
structured note or decision carries a hand-written created_at and can arrive
backdated, and the assumption was never stated, only relied on. Both merge
points now go through one byNewestFirst, which is the server's own ordering and
compares instants rather than strings.

Two of codex's three round-3 findings are not folded in:

- The refresh not adopting the response's has_more / cursor is DECLINED, not
  missed. The refresh re-fetches the NEWEST window, which says nothing about
  where the reader's paging frontier is; the stored cursor stays valid because
  the refresh consumes no older rows, and adopting the fresh page's has_more
  after the reader has paged deeper would point the cursor back at history they
  already hold. One wasted request, absorbed by the no-advance stop, in
  exchange for a correctness claim I cannot make.
- firstPageIds treating any entry missing from a refreshed first page as
  deleted is real, pre-existing, and a semantics call about what counts as a
  deletion rather than a patch: filed as BUG-2773.

Refs: BUG-2765, BUG-2773

Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X

* test(server): derive the expected entry set instead of naming two ids (codex round 4)

The test claimed every renderable entry came back exactly once and checked the
two ids it had seeded. The item's own `created` activity could have vanished or
repeated underneath that claim — the same partial-verification shape as
asserting one direction and writing the symmetric conclusion.

The expectation now comes from the store: every activity on the item minus the
kinds buildTimeline drops unconditionally, compared in both directions, so a
fixture that grows a row cannot fall outside what the test says it covers.
Still fails with the cursor withheld.

Refs: BUG-2765

Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X

* docs: state the timeline cursor contract where consumers read it (codex round 5)

The TypeScript type documented next_before/next_before_id, but the handler's
own doc comment — what a REST consumer reads — still described before + limit
only, and the API client method said nothing. A consumer following either could
still derive a cursor from its last visible entry, which is precisely the
invalid contract this change exists to replace.

Both now state the pair, that it must be forwarded rather than re-derived, why
(dropped rows make the last entry a different position, sometimes no position),
and that the id is the tie-break among entries sharing a second — send both or
neither.

Refs: BUG-2765

Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X

* fix(web): clear paging state when the timeline reloads (codex round 6)

The previous item's entries stay on screen while the new one's page 1 is in
flight — the list renders on `!loading || entries.length > 0` — so Load More is
clickable during a switch, and its cursor was the OLD item's position aimed at
the NEW item. Pre-existing in shape (the pre-fix code derived the same stale
position from the same stale entries), but now it is one line to close, and the
dedicated cursor variable is mine.

loadTimeline replaces entries with page 1 when it resolves, so clearing the
cursor and has_more before the await throws away no state that would have
survived; it just stops offering paging for a position that is no longer known.

Test holds the switch's fetch unresolved and asserts no request goes out in
that window, with the button's absence as the observable. Fails with the two
lines removed.

Refs: BUG-2765

Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X
2026-08-25 16:11:47 -04:00

210 lines
9.9 KiB
Go

package models
import (
"encoding/json"
"time"
)
// Item-level actions (existing)
var ValidActions = []string{
"created", "updated", "archived", "restored", "moved", "read", "searched",
}
// Audit action constants for auth/admin events
const (
ActionLogin = "login"
ActionLoginFailed = "login_failed"
ActionLogout = "logout"
ActionBootstrap = "bootstrap"
ActionRegister = "register"
ActionPasswordChanged = "password_changed"
ActionPasswordReset = "password_reset"
ActionTokenCreated = "token_created"
ActionTokenRevoked = "token_revoked"
ActionTokenRotated = "token_rotated"
ActionTOTPEnabled = "totp_enabled"
ActionTOTPDisabled = "totp_disabled"
ActionMemberInvited = "member_invited"
ActionMemberRemoved = "member_removed"
ActionRoleChanged = "role_changed"
ActionSettingsChanged = "settings_changed"
ActionOAuthLogin = "oauth_login"
ActionOAuthLoginFailed = "oauth_login_failed"
ActionPlanChanged = "plan_changed"
// ActionPlanOverridesChanged is logged when an admin updates a
// user's plan_overrides JSON via the admin user-detail page.
// Surfaces per-user storage / workspace / API-token quota
// overrides in the audit feed so operators can correlate a
// mysteriously-allowed upload with the override that enabled it.
ActionPlanOverridesChanged = "plan_overrides_changed"
ActionPasswordResetByAdmin = "password_reset_by_admin"
ActionUserDisabled = "user_disabled"
ActionUserEnabled = "user_enabled"
ActionAccountDeleted = "account_deleted"
// ActionEmailVerified is logged when a user confirms their email address
// via a verification link (POST /auth/verify-email). PLAN-1933 / TASK-1936.
ActionEmailVerified = "email_verified"
// ActionEmailVerifiedByAdmin is logged when an admin force-verifies a
// user's email from the admin console (DR-7). PLAN-1933 / TASK-1936.
ActionEmailVerifiedByAdmin = "email_verified_by_admin"
// ActionSessionIPChanged is logged when a session presents a different
// client IP than the one recorded at creation. We don't strict-check IP
// by default (that breaks legitimate geo shifts — VPN toggle, mobile
// roaming) but surface the change to the audit log for detection. In
// deployments configured with PAD_IP_CHANGE_ENFORCE=strict the middleware
// additionally rejects the request.
ActionSessionIPChanged = "session_ip_changed"
// ActionSessionUAChanged is logged when a session presents a different
// User-Agent hash than the one recorded at creation. Like the IP signal
// this is surfaced to the audit log for detection; in deployments
// configured with PAD_IP_CHANGE_ENFORCE=strict the middleware additionally
// revokes the session and rejects the request. The UA hash is stable for
// the life of a real session (a browser doesn't rewrite its own UA mid-
// session), so a mismatch is a stronger theft signal than an IP change —
// which is precisely why UA enforcement carries fewer false positives than
// IP enforcement. This audit row is only emitted in strict mode; log-only
// mode keeps the historical slog-only behavior to avoid changing the audit
// feed for existing self-host users.
ActionSessionUAChanged = "session_ua_changed"
// ActionStripeEventUnmarked is logged when /admin/stripe-event-unmark
// rolls back a row from stripe_processed_events (TASK-736). The
// endpoint intentionally reopens Stripe retry windows, so a persisted
// audit trail is required — a compromised cloud_secret could otherwise
// spam unmarks invisible to the admin /audit-log UI.
ActionStripeEventUnmarked = "stripe_event_unmarked"
// ActionPaymentFailedEmailSent is logged when the sidecar triggers the
// /admin/payment-failed endpoint and pad dispatches a failed-payment
// notification to the user. Audit trail exists so operators can prove
// a customer was notified before a dunning-related plan change.
ActionPaymentFailedEmailSent = "payment_failed_email_sent"
)
type Activity struct {
ID string `json:"id"`
WorkspaceID string `json:"workspace_id,omitempty"`
DocumentID string `json:"document_id,omitempty"`
Action string `json:"action"`
Actor string `json:"actor"`
Source string `json:"source"`
Metadata string `json:"metadata,omitempty"` // JSON
UserID string `json:"user_id,omitempty"`
IPAddress string `json:"ip_address,omitempty"`
UserAgent string `json:"user_agent,omitempty"`
CreatedAt time.Time `json:"created_at"`
// Enrichment fields — populated by handlers, not stored in DB
ItemTitle string `json:"item_title,omitempty"`
ItemSlug string `json:"item_slug,omitempty"`
ItemRef string `json:"item_ref,omitempty"` // e.g. "BUG-1748" — computed from the referenced item
CollectionSlug string `json:"collection_slug,omitempty"`
ActorName string `json:"actor_name,omitempty"`
}
type ActivityListParams struct {
Action string
Actor string
Source string
// Since, when non-zero, restricts results to activity created on or
// after this instant (a.created_at >= Since). Applied in the SQL query
// so LIMIT counts post-filter rows. Used by `pad project activity
// --since` and the pad_project.activity MCP action.
Since time.Time
Limit int
Offset int
}
// AuditLogParams are query parameters for the audit log endpoint.
type AuditLogParams struct {
Action string
Actor string
WorkspaceID string
Days int
Limit int
Offset int
}
// TimelineEntry represents a single entry in the unified item timeline.
// It wraps one of: a comment, an activity, a version, an implementation
// note, or a decision-log entry.
//
// Notes and decisions differ from the other three kinds in where they live:
// they are elements of the item's own fields blob, not rows in a table, so
// they arrive already-loaded on the item rather than through a cursor query
// (BUG-2301). The handler still runs them through the same (created_at, id)
// cursor predicate the SQL uses, so paging behaves identically for all five —
// over a STABLE dataset. The five sources are read at five instants with no
// shared snapshot, so a write landing mid-request can put one page slightly
// out of step with another (an `updated` activity present while the note that
// caused it is not, or the reverse). That predates this type and is not
// specific to the structured kinds — the three SQL sources were already read
// one after another. Nothing is lost: the blob is authoritative and the next
// fetch is consistent.
type TimelineEntry struct {
ID string `json:"id"`
Kind string `json:"kind"` // "comment", "activity", "version", "note", "decision"
CreatedAt time.Time `json:"created_at"`
Actor string `json:"actor"`
ActorName string `json:"actor_name,omitempty"`
// AgentName is set on "comment" entries only, and is DERIVED: it is a
// copy of Comment.AgentName, surfaced at entry level to match the
// actor_name idiom (ActorName is likewise a copy of Comment.Author for
// comment entries), so a client reads entry-level attribution the same
// way for every kind. The nested comment's value is the authoritative
// one — it is what the store's join wrote. Activity entries deliberately
// do NOT get it: their name already lives in Activity.Metadata, and a
// second copy there would be a second source that can drift (TASK-2760,
// lead ruling on the trail).
AgentName string `json:"agent_name,omitempty"`
Source string `json:"source"`
Comment *Comment `json:"comment,omitempty"`
Activity *Activity `json:"activity,omitempty"`
Version *Version `json:"version,omitempty"`
Note *ItemImplementationNote `json:"note,omitempty"`
Decision *ItemDecisionLogEntry `json:"decision,omitempty"`
}
// TimelineResponse is the paginated response from the timeline endpoint.
//
// NextBefore / NextBeforeID are the cursor for the following page, set
// whenever HasMore is true, and a client must page with THEM rather than with
// the last entry it received (BUG-2765). The two differ exactly when the
// server discarded rows: the handler over-fetches per source and drops what
// cannot render (read/searched actions, empty-metadata updates, activities a
// version or a comment already stands for, collapsed autosave bursts), so a
// page can carry fewer entries than the rows it consumed — or none at all,
// while more history waits behind them. A cursor derived from the last
// rendered entry then either cannot be formed or does not advance, and the
// client re-requests the same window forever.
//
// The value is the position the next page must start strictly before, in the
// same (created_at, id) space the store's cursor queries use.
type TimelineResponse struct {
Entries []TimelineEntry `json:"entries"`
HasMore bool `json:"has_more"`
NextBefore string `json:"next_before,omitempty"`
NextBeforeID string `json:"next_before_id,omitempty"`
}
// AgentNameFromMetadata returns the agent display name stamped on an
// activity's metadata JSON (`handlers_documents.go::agentMeta` writes the
// `agent` key from the X-Pad-Agent header), or "" when the metadata is empty,
// unparseable, or carries no non-empty string under that key. It is the Go
// twin of the web client's agentNameOf (web/src/lib/utils/agentActor.ts) and
// applies the same contract: verbatim, no normalization, and a non-string
// value counts as absent rather than being rendered.
//
// Parsing happens here, in Go, rather than in SQL: the store targets both
// SQLite and Postgres, whose JSON accessors (json_extract vs ->>) differ, and
// the comment queries currently have no dialect fork to add one to.
func AgentNameFromMetadata(metadata string) string {
if metadata == "" {
return ""
}
var m map[string]any
if err := json.Unmarshal([]byte(metadata), &m); err != nil {
return ""
}
name, _ := m["agent"].(string)
return name
}