Skip to content

pg: add --from-now to start a new push target at the join point - #1267

Closed
Technophobe01 wants to merge 4 commits into
kenn-io:mainfrom
Technophobe01:sw0k.push-from-now
Closed

pg: add --from-now to start a new push target at the join point#1267
Technophobe01 wants to merge 4 commits into
kenn-io:mainfrom
Technophobe01:sw0k.push-from-now

Conversation

@Technophobe01

@Technophobe01 Technophobe01 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

*** Alternate Solution Found ***

Closing this: it is the wrong approach.

A target's first push has no watermark, so it backfills the entire local
archive. Pushing into a database shared with other people therefore discloses
every session on the machine, including unrelated local work, and there was no
way to ask for "only what happens from here on".

Add SyncOptions.PushFromNow, exposed as `pg push --from-now`, which seeds the
watermark with the push cutoff when a target has no history yet. It is applied
after the existing reset checks rather than before: those treat a watermark
with no matching target fingerprint or PG-side push marker as corrupt local
state and clear it, so a watermark seeded earlier (or from outside the process)
is wiped and the push falls back to a full backfill. It is also ignored for an
explicitly requested full push and for a target that already has history, so it
can only bound a first push, never open a gap in an established one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (1278767)

Verdict: Three High-severity issues undermine --from-now, potentially exposing historical sessions and sensitive metadata.

High

  • Daemon-delegated pushes ignore --from-now
    Location: cmd/agentsview/archive_write_backend.go:328
    When a running daemon handles pg push, cfg.FromNow is omitted from the request schema and handler, so postgres.SyncOptions.PushFromNow remains false. Delegated one-shot and watch pushes therefore perform a complete historical backfill.
    Fix: Add FromNow to the daemon request types, populate it from cfg.FromNow, propagate it to postgres.SyncOptions, and test both delegated paths.

  • Automatic resyncs are mistaken for explicit --full requests
    Location: internal/postgres/push.go:140
    Callers pass cfg.Full || didResync, causing an automatically required local resync to disable the from-now boundary. Historical sessions can then be uploaded even though the user did not request --full.
    Fix: Carry explicit --full intent separately from internally forced resync/full-processing state. Add coverage for FromNow=true, didResync=true, and cfg.Full=false.

  • The cutoff does not protect historical usage and identity metadata
    Locations: internal/postgres/push.go:303, internal/postgres/push.go:316, internal/postgres/push.go:493
    The boundary limits session selection only. Initial synchronization still uploads historical Cursor usage events—including user IDs and email addresses—and project identity data such as paths, remotes, branches, snapshots, and session IDs.
    Fix: Apply the boundary to every history-bearing push phase, or initialize auxiliary publication cursors/revisions without backfilling existing rows. Add integration coverage confirming pre-boundary auxiliary records remain absent.

Medium

  • Reset recovery incorrectly reapplies the from-now boundary
    Location: internal/postgres/push.go:316
    If an established target loses its PG push marker, recovery clears lastPush; the helper then treats it as a new target and reapplies --from-now. In long-running pg push --watch --from-now, a recreated schema consequently fails to restore data uploaded since the original join point.
    Fix: Preserve whether valid prior target state existed and suppress PushFromNow during same-target reset recovery, distinguishing recovery from selection of a genuinely new target.

Reviewers: 2 done | Synthesis: codex, 13s | Total: 8m41s

Fixes from the review of the previous commit.

The flag was silently dropped on daemon-delegated pushes: when the local daemon
owns the archive, `pg push` posts to /api/v1/push/pg, and neither the request
nor the handler carried it, so the whole archive was backfilled anyway. It now
travels in the request and maps into SyncOptions.

Whether --full was requested cannot be decided inside Push: callers pass
`cfg.Full || didResync`, so any automatic resync looked like explicit intent and
disabled the flag. The precedence is resolved at the CLI instead, where the
user's own flag is known.

The boundary scoped session selection only, while other phases stayed
archive-wide, so pre-boundary content was uploaded regardless. A bounded push
now refuses when a vector source is attached, because embeddings carry raw text
and cannot be bounded, and skips cursor usage events for the same reason a
filtered push already skips them: the rows are global and unattributed.

Freshness is now proven rather than inferred. The reset paths clear the
watermark, so an empty watermark alone could not distinguish a genuinely new
target from an established one whose marker was lost or whose first push failed
part-way, and re-seeding there would permanently skip the history those resets
exist to restore. Boundary state, marker presence, and whether a reset ran on
this pass are all required.

Also documents the flag and its interactions in the command reference.
@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (4723f89)

Verdict: Changes requested — three privacy-boundary issues can expose historical data despite --from-now.

High

--from-now is not enforced as a durable, target-scoped boundary

Locations: internal/postgres/push.go:310, internal/postgres/push.go:335

The boundary applies only to the initial push. Subsequent watch iterations can upload the complete historical Cursor usage table, including identities, timestamps, token usage, and charges. Additionally, if marker writing fails after local finalization—or PostgreSQL is later reset—a retry can fall back to an unbounded full push and disclose pre-join session history.

Persist an immutable, target-scoped from-now floor and enforce it during watch pushes, retries, resets, and automatic recovery. Only an explicitly requested --full operation should bypass it. Add integration coverage for each recovery path.

Initial bounded pushes still publish historical project metadata

Locations: internal/postgres/push.go:510, internal/postgres/push.go:678

syncProjectIdentityObservations runs even when unbounded phases should be skipped, publishing historical session snapshots and project observations. These can include session IDs, filesystem paths, repositories, remotes, branches, and machine information.

Skip this phase for bounded targets or restrict it to post-boundary sessions. Do not advance publication state past intentionally excluded records. Add an integration test proving pre-boundary identities remain absent.

Medium

Daemon delegation does not honor --full precedence over --from-now

Location: internal/server/huma_routes_push.go:372

Daemon-delegated pushes pass body.FromNow unchanged when body.Full is true, so a fresh --full --from-now push remains bounded. Local-mode pushes correctly disable from-now in this combination.

Set PushFromNow to body.FromNow && !body.Full and add coverage for delegated --full --from-now pushes.


Reviewers: 2 done | Synthesis: codex, 21s | Total: 8m54s

Daemon delegation sent --full and --from-now unchanged, so the precedence
resolved on the local path did not apply when the daemon owns the archive:
`pg push --full --from-now` could backfill locally but bound remotely. The
delegated request now carries the resolved value.

Freshness is judged on the state as first read, before the recovery paths can
clear the watermark and boundary state, so a target whose PG marker was lost no
longer looks brand new and cannot be re-bounded.

The incompatible vector configuration is refused before connecting rather than
after, so watch mode does not retry a combination that can never succeed having
already synced locally and opened a stream. The flag help now states the
requirement.

Known limitation, left for maintainer judgement: the boundary is per-push, so
phases that are archive-wide by nature (cursor usage events on later pushes,
project-identity publication) are only skipped on the bounded push itself.
Making that permanent needs the boundary persisted per target, and the right
treatment of identity publication is a product call: skipping it entirely
leaves a bounded target without project identities, while scoping observations
and snapshots to admitted sessions preserves them but is a larger change.
The CLI refusal added last round was itself a regression. Being flag-based it
rejected targets with vector pushing disabled and established targets where
--from-now is a documented no-op, and with --all one vector-enabled target
aborted every other one. It also sat only in the one-shot entry point, so watch
mode never reached it. Removed: the authoritative condition (the boundary
actually applying with a vector source attached) is known inside Push, which
already refuses there.

--full precedence over --from-now is now settled once, at both command entry
points, rather than at each backend. The watch backend clears Full after its
startup cycle, so a retry of `--full --from-now` could previously resurrect the
boundary and permanently skip history. The push handler no longer trusts the
caller to have resolved it either.

Known limitations, unchanged and left for maintainer judgement: the boundary is
per-push, so project-identity publication and cursor usage events on later
pushes are still archive-wide. Both need the boundary persisted per target, and
whether identity publication should be skipped (leaving a bounded target with no
project identities) or scoped to admitted sessions is a product call.
@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (8031c00)

Verdict: High-risk privacy boundary gaps can expose pre-join historical data.

High

  • Locations: internal/postgres/push.go:276, internal/postgres/push.go:307, internal/postgres/push.go:325-346
    The --from-now join boundary remains local until normal finalization. Failures can cause retries to select a later cutoff and permanently miss intervening sessions. Deleting or failing to write the remote push_marker:* row can instead trigger an unrestricted full backfill, exposing previously withheld transcripts. Persist an immutable, target-scoped local join boundary immediately after freshness verification, retain it across retries and remote resets, and remove or override it only through an explicit full-backfill action.

  • Locations: internal/postgres/push.go:315, internal/postgres/push.go:515, internal/postgres/push.go:632, internal/postgres/push.go:715-728
    Bounded pushes still run syncProjectIdentityObservations. Initial publication enumerates historical observations and session snapshots, exposing pre-boundary session IDs, project names, local paths, Git remotes, branches, and worktree metadata even when no historical session is admitted. Scope publication to admitted sessions, or suppress it for bounded targets while recording a baseline revision that prevents later replay.

  • Locations: internal/postgres/push.go:315, internal/postgres/push.go:3471
    skipUnboundedPhases protects only the first bounded invocation. A later push calls GetCursorUsageEvents(ctx, 0) and uploads the full historical Cursor usage archive, including user IDs, email addresses, timestamps, token counts, and costs. Persist bounded-target state and filter every future sync by the join boundary or a corresponding high-water ID, or permanently omit this global phase for bounded targets.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 8m40s

@Technophobe01
Technophobe01 deleted the sw0k.push-from-now branch July 26, 2026 01:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant