Skip to content

Commit 3c951eb

Browse files
committed
pg: address second review of --from-now
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.
1 parent 4723f89 commit 3c951eb

4 files changed

Lines changed: 18 additions & 3 deletions

File tree

cmd/agentsview/archive_write_backend.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ func (b daemonArchiveWriteBackend) PGPush(
333333
SyncStateTarget: target.SyncStateTarget,
334334
MigrateLegacySyncState: target.MigrateLegacySyncState,
335335
NoVectors: cfg.NoVectors,
336-
FromNow: cfg.FromNow,
336+
FromNow: cfg.FromNow && !cfg.Full,
337337
ScopeVectorsToChangedSessions: cfg.
338338
ScopeVectorsToChangedSessions,
339339
LastReconciledVectorGeneration: cfg.

cmd/agentsview/cli.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -652,7 +652,7 @@ func newPGPushCommand() *cobra.Command {
652652
cmd.Flags().DurationVar(&cfg.Debounce, "debounce", defaultWatchDebounce, "Coalesce window after a change before pushing (--watch only)")
653653
cmd.Flags().DurationVar(&cfg.Interval, "interval", defaultWatchInterval, "Periodic floor push interval (--watch only)")
654654
cmd.Flags().BoolVar(&cfg.NoVectors, "no-vectors", false, "Skip pushing semantic-search vectors")
655-
cmd.Flags().BoolVar(&cfg.FromNow, "from-now", false, "On a target's FIRST push, start from now instead of backfilling local history (ignored with --full)")
655+
cmd.Flags().BoolVar(&cfg.FromNow, "from-now", false, "On a target's FIRST push, start from now instead of backfilling local history (ignored with --full; requires --no-vectors)")
656656
return cmd
657657
}
658658

cmd/agentsview/pg.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,16 @@ func pgVectorPushSource(
9999
func runPGPush(
100100
cfg PGPushConfig, targetName string,
101101
) error {
102+
// Refuse up front: the vector phase cannot be bounded, and discovering that
103+
// only after connecting leaves watch mode retrying a configuration that can
104+
// never succeed, having already synced locally and opened a stream.
105+
if cfg.FromNow && !cfg.Full && !cfg.NoVectors {
106+
return fmt.Errorf(
107+
"--from-now cannot bound the vector phase (it would upload " +
108+
"embeddings and raw text for sessions before the boundary); " +
109+
"re-run with --no-vectors",
110+
)
111+
}
102112
appCfg, err := config.LoadMinimal()
103113
if err != nil {
104114
return fmt.Errorf("loading config: %w", err)

internal/postgres/push.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,10 @@ func (s *Sync) PushWithOptions(
178178
lastPushBoundaryStateKey, err,
179179
)
180180
}
181+
// Freshness must be judged on what was on disk when this push started: the
182+
// recovery paths below clear lastPush and boundaryState, which would make an
183+
// established target whose marker was lost look brand new.
184+
initialLastPush, initialBoundaryState := lastPush, boundaryState
181185
pushStateCleared := false
182186
if reset, reason := pushTargetState(
183187
lastPush,
@@ -301,7 +305,8 @@ func (s *Sync) PushWithOptions(
301305
// (watermark, boundary state) and nothing remote (this marker) has ever
302306
// recorded a push, and that no reset cleared state on this run.
303307
applyFromNow := pushFromNowApplies(
304-
s.pushFromNow, lastPush, boundaryState, markerExists, pushStateCleared,
308+
s.pushFromNow, initialLastPush, initialBoundaryState,
309+
markerExists, pushStateCleared,
305310
)
306311
// The boundary scopes SESSION selection only. Phases that are archive-wide
307312
// would still upload pre-boundary content, defeating the point, so a bounded

0 commit comments

Comments
 (0)