diff --git a/apps/cli/src/commands/results/remote.ts b/apps/cli/src/commands/results/remote.ts index 6dda5382c..1581dab1b 100644 --- a/apps/cli/src/commands/results/remote.ts +++ b/apps/cli/src/commands/results/remote.ts @@ -8,7 +8,7 @@ import { type NormalizedResultsConfig, type ResultsConfig, type ResultsRepoStatus, - directPushResults, + directPushResultsWithDetails, directorySizeBytes, getProject, getProjectForPath, @@ -155,6 +155,7 @@ export interface ResultsPublishOverrides { readonly remote?: string; readonly auto_push?: boolean; readonly require_push?: boolean; + readonly push_conflict_policy?: 'block' | 'backup_and_force_push'; } const REMOTE_RUN_PREFIX = 'remote::'; @@ -224,7 +225,8 @@ export async function loadNormalizedResultsConfig( ...(project.results.remote !== undefined && { remote: project.results.remote }), ...(project.results.path !== undefined && { path: project.results.path }), ...((project.results.sync?.autoPush !== undefined || - project.results.sync?.requirePush !== undefined) && { + project.results.sync?.requirePush !== undefined || + project.results.sync?.pushConflictPolicy !== undefined) && { sync: { ...(project.results.sync?.autoPush !== undefined && { auto_push: project.results.sync.autoPush, @@ -232,6 +234,9 @@ export async function loadNormalizedResultsConfig( ...(project.results.sync?.requirePush !== undefined && { require_push: project.results.sync.requirePush, }), + ...(project.results.sync?.pushConflictPolicy !== undefined && { + push_conflict_policy: project.results.sync.pushConflictPolicy, + }), }, }), ...(project.results.branchPrefix !== undefined && { @@ -284,8 +289,10 @@ export async function loadNormalizedResultsConfig( : {}), ...((overrides.auto_push !== undefined || overrides.require_push !== undefined || + overrides.push_conflict_policy !== undefined || baseConfig?.auto_push !== undefined || - baseConfig?.require_push !== undefined) && { + baseConfig?.require_push !== undefined || + baseConfig?.push_conflict_policy !== undefined) && { sync: { ...((overrides.auto_push ?? baseConfig?.auto_push) !== undefined && { auto_push: overrides.auto_push ?? baseConfig?.auto_push, @@ -293,6 +300,9 @@ export async function loadNormalizedResultsConfig( ...((overrides.require_push ?? baseConfig?.require_push) !== undefined && { require_push: overrides.require_push ?? baseConfig?.require_push, }), + ...((overrides.push_conflict_policy ?? baseConfig?.push_conflict_policy) !== undefined && { + push_conflict_policy: overrides.push_conflict_policy ?? baseConfig?.push_conflict_policy, + }), }, }), ...(baseConfig?.branch_prefix ? { branch_prefix: baseConfig.branch_prefix } : {}), @@ -605,14 +615,22 @@ export async function maybeAutoExportRunArtifacts( const relativeRunPath = getRelativeRunPath(payload.cwd, payload.run_dir); const commitTitle = buildCommitTitle(payload); - const pushed = await directPushResults({ + const pushResult = await directPushResultsWithDetails({ config, sourceDir: payload.run_dir, destinationPath: relativeRunPath, commitMessage: commitTitle, }); - if (!pushed) { + if (pushResult.blocked) { + if (config.require_push) { + throw new Error(pushResult.block_reason ?? 'Results branch push conflict'); + } + console.warn(`Warning: skipping results export: ${pushResult.block_reason}`); + return 'failed'; + } + + if (!pushResult.changed) { console.warn('Warning: results export produced no git changes.'); return 'already_published'; } @@ -621,6 +639,11 @@ export async function maybeAutoExportRunArtifacts( console.log( `Results ${pushLabel} to ${config.repo} (${config.branch ?? 'default branch'}:${relativeRunPath})`, ); + if (pushResult.backup_ref) { + console.log( + `Backed up previous remote ${pushResult.target_branch ?? config.branch ?? 'results branch'} at ${pushResult.backup_ref}`, + ); + } return 'published'; } catch (error) { if (config.require_push) { diff --git a/apps/dashboard/src/lib/project-sync-status.test.ts b/apps/dashboard/src/lib/project-sync-status.test.ts index 8702ca023..36b601758 100644 --- a/apps/dashboard/src/lib/project-sync-status.test.ts +++ b/apps/dashboard/src/lib/project-sync-status.test.ts @@ -85,6 +85,23 @@ describe('getProjectSyncView', () => { canSync: false, }); }); + + it('surfaces result branch push conflicts without resolution controls', () => { + expect( + getProjectSyncView({ + configured: true, + available: true, + sync_status: 'push_conflict', + push_conflict_policy: 'block', + block_reason: 'Results branch push conflict on agentv/results/v1', + }), + ).toMatchObject({ + state: 'push_conflict', + label: 'Push conflict', + tone: 'danger', + canSync: false, + }); + }); }); describe('buildProjectSyncFeedback', () => { @@ -140,6 +157,18 @@ describe('buildProjectSyncFeedback', () => { }); }); + it('keeps push conflict feedback explicit', () => { + expect( + buildProjectSyncFeedback({ + configured: true, + available: true, + sync_status: 'push_conflict', + blocked: true, + block_reason: 'Results branch push conflict on agentv/results/v1', + }).message, + ).toContain('Sync stopped: Results branch push conflict on agentv/results/v1.'); + }); + it('builds actionable sync failure feedback without hiding cached remote runs', () => { expect( buildProjectSyncErrorFeedback(new Error('GitHub authentication failed'), { diff --git a/apps/dashboard/src/lib/project-sync-status.ts b/apps/dashboard/src/lib/project-sync-status.ts index 0854f3f7f..6920ae6a0 100644 --- a/apps/dashboard/src/lib/project-sync-status.ts +++ b/apps/dashboard/src/lib/project-sync-status.ts @@ -8,6 +8,7 @@ export type ProjectSyncState = | 'ahead' | 'dirty' | 'conflicted' + | 'push_conflict' | 'syncing'; export type ProjectSyncTone = 'neutral' | 'good' | 'info' | 'warn' | 'danger'; @@ -121,6 +122,23 @@ export function getProjectSyncView( } const state = status.sync_status ?? 'clean'; + if (state === 'push_conflict') { + return { + state: 'push_conflict', + label: 'Push conflict', + actionLabel: 'Sync Project', + tone: 'danger', + summary: + status.block_reason ?? + 'The remote results branch changed before local results could be pushed.', + nextAction: + status.push_conflict_policy === 'backup_and_force_push' + ? 'Sync stopped before changing the results branch. Refresh status, then retry if this server should replace the remote branch.' + : 'Sync stopped before changing the results branch. Opt in to backup_and_force_push only if this server should replace the remote branch.', + canSync: false, + }; + } + if (state === 'conflicted' || state === 'diverged') { return { state: 'conflicted', @@ -240,7 +258,12 @@ export function buildProjectSyncFeedback(status: RemoteStatusResponse): { kind: 'success' | 'warning'; message: string; } { - if (status.blocked || status.sync_status === 'conflicted' || status.sync_status === 'diverged') { + if ( + status.blocked || + status.sync_status === 'conflicted' || + status.sync_status === 'diverged' || + status.sync_status === 'push_conflict' + ) { const repo = status.repo ? ` for ${status.repo}` : ''; const reason = status.block_reason ?? 'Sync stopped before changing the results repo.'; return { diff --git a/apps/dashboard/src/lib/types.ts b/apps/dashboard/src/lib/types.ts index 23ad8ebef..3c03e1ca0 100644 --- a/apps/dashboard/src/lib/types.ts +++ b/apps/dashboard/src/lib/types.ts @@ -456,6 +456,7 @@ export interface RemoteStatusResponse { local_dir?: string; path?: string; auto_push?: boolean; + push_conflict_policy?: 'block' | 'backup_and_force_push'; branch_prefix?: string; run_count?: number; last_synced_at?: string; @@ -468,6 +469,7 @@ export interface RemoteStatusResponse { | 'diverged' | 'dirty' | 'conflicted' + | 'push_conflict' | 'syncing'; branch?: string; upstream?: string; @@ -482,6 +484,14 @@ export interface RemoteStatusResponse { pull_performed?: boolean; push_performed?: boolean; commit_created?: boolean; + target_branch?: string; + remote_commit?: string; + local_commit?: string; + backup_ref?: string; + backup_commit?: string; + previous_remote_commit?: string; + force_pushed_commit?: string; + lease_commit?: string; } // ── Project types ────────────────────────────────────────────────────── diff --git a/apps/web/src/content/docs/docs/tools/dashboard.mdx b/apps/web/src/content/docs/docs/tools/dashboard.mdx index 6a9118cfb..903ef4436 100644 --- a/apps/web/src/content/docs/docs/tools/dashboard.mdx +++ b/apps/web/src/content/docs/docs/tools/dashboard.mdx @@ -292,9 +292,10 @@ projects: sync: auto_push: false require_push: false + push_conflict_policy: block ``` -`results.repo.remote` is the Git remote URL AgentV fetches and pushes. `results.repo.path: .` stores completed run artifacts on a dedicated branch of the source repository without checking out that branch in the source worktree. AgentV manages the local Git remote alias for that URL, so the normal config stays portable across machines. When `results.repo.remote` is omitted, `results.repo.path` means an existing local Git checkout whose object database and refs AgentV should write to, and the branch defaults to `agentv/results/v1`. AgentV creates the branch automatically on first publish and commits only AgentV result paths into it. `sync.auto_push: false` keeps the result commit local; set it to `true` to push the branch best-effort after each completed run. `sync.require_push: true` is for CI workflows where a push failure should fail the command after local artifacts are written. +`results.repo.remote` is the Git remote URL AgentV fetches and pushes. `results.repo.path: .` stores completed run artifacts on a dedicated branch of the source repository without checking out that branch in the source worktree. AgentV manages the local Git remote alias for that URL, so the normal config stays portable across machines. When `results.repo.remote` is omitted, `results.repo.path` means an existing local Git checkout whose object database and refs AgentV should write to, and the branch defaults to `agentv/results/v1`. AgentV creates the branch automatically on first publish and commits only AgentV result paths into it. `sync.auto_push: false` keeps the result commit local; set it to `true` to push the branch best-effort after each completed run. `sync.require_push: true` is for CI workflows where a push failure should fail the command after local artifacts are written. `sync.push_conflict_policy` defaults to `block`; set it to `backup_and_force_push` only for a single-writer Dashboard/server that should replace the remote results branch after creating a remote backup ref. For a separate results repository, use `results.repo.remote` and an optional managed clone `results.repo.path`: @@ -311,6 +312,7 @@ projects: path: /home/entity/projects/EntityProcess/agentv-examples-eval-results sync: auto_push: true + push_conflict_policy: block ``` `results.repo.remote` is the Git remote URL used for clone and push operations, so use HTTPS when credentials are HTTP-token based and SSH when the runtime has SSH keys configured. When `results.repo.remote` is set, `results.repo.path` is the filesystem location of the local clone AgentV manages for that remote results repo. Omit `results.repo.remote` only when `results.repo.path` points at an already-existing local checkout such as `.`. @@ -326,6 +328,7 @@ results: sync: auto_push: false require_push: false + push_conflict_policy: block ``` Project-local `.agentv/config.yaml` is for portable eval defaults such as `execution`, `eval_patterns`, and `dashboard`. Do not put `projects` in project-local config; AgentV warns and ignores it there. `results_by_project` is deprecated; use `projects[].results` in `$AGENTV_HOME/config.yaml`. @@ -421,6 +424,7 @@ After sync, newly fetched remote runs appear in the list with a **remote** sourc - A clean clone that is behind the remote is fast-forwarded. - Safe uncommitted changes under the configured results repo's owned result and metadata paths, such as remote tag overlays under `metadata/runs/**`, are committed and pushed when `sync.auto_push: true`. - A local results repo that is ahead is pushed when `sync.auto_push: true` and the committed paths are all under `.agentv/results/**`. -- Dirty non-results files, dirty metadata plus remote changes, diverged history, unresolved conflicts, missing upstream branches, non-results commits ahead, and rejected pushes are blocked instead of reset. +- Dirty non-results files, dirty metadata plus remote changes, unresolved conflicts, missing upstream branches, non-results commits ahead, and rejected pushes are blocked instead of reset. +- Non-fast-forward result branch pushes use `sync.push_conflict_policy`. The default `block` policy reports a `push_conflict` with the target branch and local/remote commits. The explicit `backup_and_force_push` policy fetches the target branch, creates `agentv/backups/--` from the current remote commit, then force-pushes with a lease tied to that same commit. Backup refs are siblings under `agentv/backups/`, not nested below the target branch. When sync is blocked, Dashboard keeps the local clone intact and shows the `block_reason`, `dirty_paths` or `conflicted_paths`, `git_status`, and a compact `git_diff_summary` so you can resolve the results repo manually before syncing again. diff --git a/apps/web/src/content/docs/docs/tools/results.mdx b/apps/web/src/content/docs/docs/tools/results.mdx index 139468319..e5f653ed8 100644 --- a/apps/web/src/content/docs/docs/tools/results.mdx +++ b/apps/web/src/content/docs/docs/tools/results.mdx @@ -231,7 +231,7 @@ The CLI contract is deliberately narrow: `agentv results` manages local result a Use these supported remote workflows instead: -- **Automatic publishing:** configure `projects[].results` or top-level `results`; new `agentv eval` and `agentv pipeline bench` runs publish completed artifacts after the run completes. Use `repo.remote` with `repo.path: .` and `repo.branch: agentv/results/v1` to store primary result records on a dedicated branch of the source repo without requiring a machine-local Git remote name. AgentV reserves `agentv/results/v1` for primary results and `agentv/artifacts/v1` for heavy artifact payloads. When `index.jsonl` rows point trace or transcript payloads at `agentv/artifacts/v1`, automatic publishing stores those bytes on that artifact branch in the same remote and publishes pointer keys such as `runs//`. The configured results branch remains the metadata/control plane (`index.jsonl`, `benchmark.json`, tags, and pointers) instead of duplicating canonical trace/transcript payload bodies. Local pre-publish run workspaces can still contain those files beside the manifest so local tools keep working. Mutable run tags are stored as `tags.json` with a `tag_revision`; there is no tag event log in the normal results layout. `results.repo.path` without `results.repo.remote` means an existing local Git checkout, distinct from `workspace.repos[].repo`, which is a portable repository identity. AgentV manages any local Git remote alias internally. Set `sync.auto_push: true` to push after publish, or `sync.require_push: true` in CI to fail when that push fails. While an eval is still running, [WIP checkpoints](/docs/tools/wip-checkpoints/) can keep partial run output durable on `agentv/wip/...` branches when auto-push is enabled. +- **Automatic publishing:** configure `projects[].results` or top-level `results`; new `agentv eval` and `agentv pipeline bench` runs publish completed artifacts after the run completes. Use `repo.remote` with `repo.path: .` and `repo.branch: agentv/results/v1` to store primary result records on a dedicated branch of the source repo without requiring a machine-local Git remote name. AgentV reserves `agentv/results/v1` for primary results and `agentv/artifacts/v1` for heavy artifact payloads. When `index.jsonl` rows point trace or transcript payloads at `agentv/artifacts/v1`, automatic publishing stores those bytes on that artifact branch in the same remote and publishes pointer keys such as `runs//`. The configured results branch remains the metadata/control plane (`index.jsonl`, `benchmark.json`, tags, and pointers) instead of duplicating canonical trace/transcript payload bodies. Local pre-publish run workspaces can still contain those files beside the manifest so local tools keep working. Mutable run tags are stored as `tags.json` with a `tag_revision`; there is no tag event log in the normal results layout. `results.repo.path` without `results.repo.remote` means an existing local Git checkout, distinct from `workspace.repos[].repo`, which is a portable repository identity. AgentV manages any local Git remote alias internally. Set `sync.auto_push: true` to push after publish, or `sync.require_push: true` in CI to fail when that push fails. Non-fast-forward result branch pushes block by default with `sync.push_conflict_policy: block`; `backup_and_force_push` is an explicit single-writer opt-in that first creates an `agentv/backups/--` remote backup branch and then force-pushes with a lease. While an eval is still running, [WIP checkpoints](/docs/tools/wip-checkpoints/) can keep partial run output durable on `agentv/wip/...` branches when auto-push is enabled. - **Manual Dashboard sync:** run `agentv dashboard`, open the project, and use **Sync Project**. - **Manual API sync:** while Dashboard is running, call `GET /api/projects/:projectId/remote/status` or `POST /api/projects/:projectId/remote/sync` for project-scoped automation. Single-project sessions also expose `GET /api/remote/status` and `POST /api/remote/sync`. - **Git escape hatch:** for advanced recovery, inspect or repair the configured `projects[].results.repo.path` clone with `git` directly, then sync again. diff --git a/docs/plans/2026-06-23-002-feat-remote-result-metadata-conflicts-plan.md b/docs/plans/2026-06-23-002-feat-remote-result-metadata-conflicts-plan.md deleted file mode 100644 index f8d5f7291..000000000 --- a/docs/plans/2026-06-23-002-feat-remote-result-metadata-conflicts-plan.md +++ /dev/null @@ -1,506 +0,0 @@ ---- -title: "feat: Remote result metadata conflict resolution UX" -type: feat -date: 2026-06-23 -bead: av-xwm ---- - -# feat: Remote result metadata conflict resolution UX - -## Summary - -Design a conflict-resolution workflow for Git-backed result metadata writes from Dashboard, hosted Dashboard, and API clients. The v1 scope keeps mutable metadata simple: colocated `tags.json` in the timestamped result folder, no local `.agentv/results/metadata` folder, and no `tag-events.jsonl`. - -This is a design artifact only. It does not migrate artifact layout or implement broad UI. - ---- - -## Problem Frame - -AgentV result artifacts are Git-backed and often edited by a browser session that has stale state. Tags, feedback, and other result metadata can be written by a hosted Dashboard/server directly to the configured results branch while another local clone, server, or browser tab still believes an older revision is current. - -The current sync surface can report dirty, conflicted, ahead, behind, and diverged states, and it blocks on unresolved Git conflicts. It does not yet give users a guided way to choose between pulling remote state, backing up and force-pushing local state, or accepting incoming/outgoing changes per file. It also does not give tag and feedback writes a durable stale-page contract. - -The product boundary stays repo-native: the Git-backed result branch remains the source of truth, the Dashboard is the zero-infra inspection and resolution surface, and Phoenix or other external systems are not involved. - ---- - -## Source Inputs - -- User request: remote conflict-resolution UX for result metadata/tag/feedback sync. -- Current remote sync decision: `docs/plans/2026-06-10-remote-results-cli-contract.md`. -- Git-backed results contract: `docs/plans/git-native-results.md`. -- Current sync/status implementation: `packages/core/src/evaluation/results-repo.ts`. -- Current Dashboard API routes: `apps/cli/src/commands/results/serve.ts`. -- Current tag sidecar helpers: `apps/cli/src/commands/results/run-tags.ts`. -- Existing remote metadata overlay helper, used as compatibility context only: `apps/cli/src/commands/results/remote-metadata.ts`. - ---- - -## Requirements - -### Conflict Choices - -- R1. When sync detects conflicts or diverged remote metadata, users can choose pull remote and overwrite local metadata, force push local metadata to remote, or resolve selected files individually. -- R2. Force-push local metadata must first create a timestamped backup branch/ref of the remote branch and surface that backup ref after creation. -- R3. Pull-remote-overwrite must show which local metadata files or commits will be discarded and must not touch non-result paths. -- R4. Per-file resolution must support accepting incoming or outgoing content for each selected conflicted file. - -### Conflict Status Contract - -- R5. The backend status contract exposes dirty paths, conflicted paths, incoming/outgoing refs, incoming/outgoing commits, and a base ref/commit when Git can compute one. -- R6. Conflict file detail includes enough blob/revision identity for the UI to render diffs and for later resolution requests to verify that the file did not change under the user's cursor. -- R7. API fields use snake_case at the process boundary and camelCase only inside TypeScript. - -### Dashboard UX - -- R8. The Dashboard conflict view shows conflicted files in a tree, expands file nodes to inline diffs, and uses accessible background color coding for incoming versus outgoing changes. -- R9. Each file row has accept incoming and accept outgoing actions, with selected and unresolved states visible before the user applies resolution. -- R10. Global pull and force-push actions require confirmation copy that names data loss, backup behavior, and the target branch/ref. -- R11. Disabled, loading, success, warning, and error states make the sync operation's current state explicit without telling users to manually edit the results checkout first. - -### Stale Browser Writes - -- R12. Tag, feedback, and metadata writes use optimistic concurrency through a commit/revision/etag check. -- R13. Stale replacement writes are rejected with a refresh-required response unless the operation is an explicit add/remove that can be safely merged. -- R14. Safe add/remove tag operations may be merged server-side when the current file revision changed but the requested operation remains deterministic. - -### Metadata Shape - -- R15. V1 stores local run tags as `.agentv/results///tags.json` beside `index.jsonl`; the published results branch stores immutable run bundles under `runs///`. -- R16. V1 must not add a local metadata folder or `tag-events.jsonl`. -- R17. Existing `metadata/runs/**/tags.json` overlays, if present, are compatibility input only; the conflict UX should not depend on that layout as the target model. -- R18. Feedback writes use the same conflict and optimistic concurrency contract as tags, even if their physical file remains `feedback.json` until a later per-run feedback storage cleanup. - ---- - -## Key Technical Decisions - -- KTD1. Treat conflicts as a first-class Dashboard/API workflow, not a CLI command family. This preserves the existing decision that manual remote sync is Dashboard/API-owned while advanced users can still use Git directly. -- KTD2. Use a narrow safe-path allowlist for mutable result metadata. V1 local paths are `.agentv/results///tags.json`; published result-branch paths are `runs/**/tags.json`, `runs/**/feedback.json` when introduced, and the current compatibility `feedback.json`. Broad run-bundle rewrites remain out of scope for conflict resolution. -- KTD3. Make force push a backup-and-lease operation. The backend creates a remote backup ref from the current remote commit, then force-pushes with a lease against that same commit so a second writer cannot be overwritten silently. -- KTD4. Use optimistic concurrency for browser writes. `expected_revision` in JSON and `If-Match` headers both map to the same file or metadata etag check. -- KTD5. Prefer operation endpoints for mergeable edits. A `run.tags.add` or `run.tags.remove` request can merge safely; a full `tags` replacement must reject stale state unless the etag still matches. -- KTD6. Keep per-file resolution file-based. Do not introduce a raw operation log, event stream, CRDT, or merge service for v1. -- KTD7. Keep branch-protection failures visible. If the remote rejects backup branch creation, force push, or direct branch updates, the API returns a blocked status with the remote error and no fallback that bypasses protection. - ---- - -## High-Level Technical Design - -```mermaid -flowchart TB - Browser[Dashboard tab with revision] --> Write[Tag feedback metadata write] - Write --> Check{expected_revision matches?} - Check -->|yes| Commit[Write metadata and commit] - Check -->|no replace| Stale[409 stale_write refresh_required] - Check -->|no add/remove| Mergeable{operation still deterministic?} - Mergeable -->|yes| Commit - Mergeable -->|no| Stale - - Sync[Sync Project] --> Fetch[Fetch remote] - Fetch --> Inspect[Inspect local remote base] - Inspect -->|clean behind| FastForward[Fast-forward] - Inspect -->|dirty/diverged/conflicted| ConflictStatus[Conflict status with refs and paths] - ConflictStatus --> PullOverwrite[Pull remote overwrite] - ConflictStatus --> ForceLocal[Backup remote then force push local] - ConflictStatus --> FileResolve[Per-file accept incoming/outgoing] - FileResolve --> CommitResolution[Commit resolved metadata] - CommitResolution --> Push[Push with lease] -``` - -The same conflict engine should serve unscoped and project-scoped Dashboard routes. Storage-branch worktree mode and checked-out results repo mode can differ internally, but they must return the same wire contract. - ---- - -## Backend Contract - -### Status Response - -Extend `GET /api/remote/status` and `GET /api/projects/:projectId/remote/status` without removing existing fields. - -```json -{ - "configured": true, - "available": true, - "sync_status": "conflicted", - "branch": "agentv/results/v1", - "upstream": "origin/agentv/results/v1", - "incoming_ref": "refs/remotes/origin/agentv/results/v1", - "outgoing_ref": "refs/heads/agentv/results/v1", - "base_ref": "merge-base:refs/heads/agentv/results/v1...refs/remotes/origin/agentv/results/v1", - "incoming_commit": "remote_sha", - "outgoing_commit": "local_sha", - "base_commit": "base_sha", - "ahead": 1, - "behind": 1, - "dirty_paths": ["runs/demo/2026-06-23T10-00-00-000Z/tags.json"], - "conflicted_paths": ["runs/demo/2026-06-23T10-00-00-000Z/tags.json"], - "conflicts": [ - { - "path": "runs/demo/2026-06-23T10-00-00-000Z/tags.json", - "status": "both_modified", - "incoming_revision": "remote_blob_or_worktree_etag", - "outgoing_revision": "local_blob_or_worktree_etag", - "base_revision": "base_blob_or_null", - "safe_path": true - } - ], - "blocked": true, - "block_reason": "Result metadata conflicts require resolution" -} -``` - -`conflicts` can be omitted from normal clean status responses. When status is `dirty`, `diverged`, or `conflicted`, the backend should include `conflicts` for safe mutable metadata paths and should separately expose any unsafe paths that block automated resolution. - -### Diff Response - -Add a lazy diff route so status polling stays cheap: - -- `GET /api/remote/conflicts` -- `GET /api/projects/:projectId/remote/conflicts` - -Response fields: - -- `schema_version: "agentv.remote_conflicts.v1"`. -- `incoming_ref`, `outgoing_ref`, `base_ref`, `incoming_commit`, `outgoing_commit`, `base_commit`. -- `files[]` with `path`, `status`, `safe_path`, `incoming_revision`, `outgoing_revision`, `base_revision`. -- `files[].diff` as hunks with `kind: "incoming" | "outgoing" | "context" | "conflict_marker"` and line numbers where available. -- `generated_at` and `status_revision` so the UI can detect when a later status refresh invalidates an open diff. - -### Resolution Endpoint - -Add a single action endpoint: - -- `POST /api/remote/resolve` -- `POST /api/projects/:projectId/remote/resolve` - -Request shape: - -```json -{ - "action": "pull_remote_overwrite", - "expected_incoming_commit": "remote_sha", - "expected_outgoing_commit": "local_sha" -} -``` - -```json -{ - "action": "force_push_local", - "expected_incoming_commit": "remote_sha", - "expected_outgoing_commit": "local_sha", - "confirm_backup": true -} -``` - -```json -{ - "action": "resolve_files", - "expected_incoming_commit": "remote_sha", - "expected_outgoing_commit": "local_sha", - "files": [ - { - "path": "runs/demo/2026-06-23T10-00-00-000Z/tags.json", - "resolution": "incoming", - "expected_incoming_revision": "remote_blob_or_worktree_etag", - "expected_outgoing_revision": "local_blob_or_worktree_etag" - } - ] -} -``` - -Successful responses include `sync_status`, `pull_performed`, `push_performed`, `commit_created`, `resolved_paths`, `backup_ref` when created, `backup_commit` when created, and the refreshed status contract. - -Stale resolution requests return `409` with: - -- `error: "stale_conflict_state"`. -- `refresh_required: true`. -- Current `incoming_commit`, `outgoing_commit`, and `conflicted_paths`. - ---- - -## Force-Push Backup Mechanics - -The backend must fetch before force push and resolve the current remote target commit. It then creates a remote backup branch/ref before overwriting the target branch. - -Backup naming: - -```text -refs/heads/agentv/backups//- -``` - -Example: - -```text -refs/heads/agentv/backups/agentv-results-v1/20260623T104512Z-a1b2c3d -``` - -Rules: - -- `target_branch_slug` is derived from the source target branch, with `/` replaced by `-` and all other unsafe characters replaced by `-`. -- `timestamp_utc` uses UTC `YYYYMMDDTHHMMSSZ`. -- `remote_short_sha` is the remote commit being backed up. -- If the backup ref already exists, append a short random suffix and retry once. -- If backup creation fails, the force-push action aborts before changing the target branch. -- The final overwrite uses a lease tied to `expected_incoming_commit`. -- The response and Dashboard success state surface `backup_ref` and `backup_commit`. - -Branch protection handling is explicit: protected branches may reject backup refs, force pushes, or both. The API should surface the provider/Git error and leave the branch untouched after a failed backup or failed lease. - ---- - -## Pull-Remote-Overwrite Semantics - -Pull remote overwrite is a destructive local/cache action, not a merge. It discards local dirty metadata or local-only result metadata commits and makes the local results checkout/ref match the remote target. - -Safeguards: - -- Fetch remote immediately before the overwrite. -- Require `expected_incoming_commit` so the user does not pull an unseen remote commit. -- Only operate when all dirty/conflicted paths are safe mutable metadata paths. -- If local commits would be abandoned, create a local recovery ref before moving the local ref and surface it in the response. -- Never switch or reset the user's source repository branch; operate only in the configured results repo or storage-branch worktree machinery. -- Show confirmation copy that lists the number of files and commits being discarded. - -Suggested confirmation copy: - -> Pull remote and overwrite local metadata? This discards unsynced local result metadata for N files and replaces it with branch `` at ``. This does not change source code. - ---- - -## Per-File Resolution Mechanics - -Per-file resolution starts from the current conflict state and applies choices only to safe mutable metadata files. - -Accepted incoming: - -- Use the blob/content from `incoming_ref` for the file. -- If incoming deleted the file, remove it locally. -- Mark the path resolved. - -Accepted outgoing: - -- Keep the local/outgoing blob/content for the file. -- If outgoing deleted the file, remove it locally. -- Mark the path resolved. - -Commit behavior: - -- If all conflicted paths are resolved and there are staged metadata changes, create a commit such as `chore(results): resolve result metadata conflicts`. -- If only a subset is resolved, keep the remaining conflict status and do not push automatically. -- If push is needed after a resolution commit, use the normal push path when it is fast-forwardable; if the remote changed again, reject with `stale_conflict_state` rather than escalating silently to force push. - -Conflict statuses to model in the UI: - -- `both_modified`. -- `added_by_incoming`. -- `added_by_outgoing`. -- `deleted_by_incoming`. -- `deleted_by_outgoing`. -- `unsafe_path`. -- `binary_or_unreadable`. - -For JSON metadata, the diff should still be file-based in v1. A future JSON-aware editor can be added after the file workflow is stable. - ---- - -## Dashboard UX - -### Entry Points - -- The existing project/run source toolbar can keep showing `Clean`, `Dirty`, `Ahead`, `Behind`, `Conflicted`, and `Unavailable`. -- When status is `conflicted` or `diverged`, the primary action becomes `Resolve Conflicts`. -- When status is `dirty` and remote has changed in a way that touches the same safe path, the action also opens the conflict view. -- Global `Sync Project` remains for clean, behind, ahead, and simple dirty cases. - -### Conflict View - -Layout: - -- Header with branch, incoming commit, outgoing commit, and base commit when available. -- Three primary actions: `Pull Remote`, `Force Push Local`, and `Apply File Resolutions`. -- Tree view grouped by path segments under `runs/`. -- File nodes show status, safe/unsafe indicator, and selected resolution. -- Expanding a node renders an inline diff. - -Diff colors: - -- Incoming remote changes use one background color and a left label `Incoming`. -- Outgoing local changes use a different background color and a left label `Outgoing`. -- Conflict marker lines, if present, use a warning background. -- Do not rely on color alone; include text labels and icons. - -File actions: - -- `Accept incoming` selects the remote version. -- `Accept outgoing` selects the local version. -- `Reset choice` clears a selection. -- Unsafe paths disable both accept buttons and link to manual recovery guidance. - -States: - -- Loading: tree skeleton and disabled action buttons while diffs load. -- Disabled: `Apply File Resolutions` disabled until at least one selected safe file exists. -- Partial: show `N of M conflicts selected`; allow applying selected paths only if the backend supports partial resolution. -- Error: show the backend `block_reason` or stale-state message and keep the latest refresh action visible. -- Success: show resolved paths and, for force push, the backup branch/ref. - -Force-push confirmation copy: - -> Force push local metadata to ``? AgentV will first back up the current remote branch to ``, then overwrite `` with local result metadata. Use this only when local metadata is the source of truth. - -Pull confirmation copy: - -> Pull remote metadata and overwrite local changes? Unsynced local result metadata for the listed files will be discarded. Source code and immutable run artifacts are not changed. - ---- - -## Optimistic Concurrency For Tags And Feedback - -Read responses for runs and metadata write targets should include a revision token: - -- `metadata_revision` on run list/detail rows for the effective mutable metadata state. -- Per-file `revision` for `tags.json` and feedback files where available. -- An HTTP `ETag` for dedicated metadata read endpoints if added. - -Write requests accept either: - -- `If-Match: ""`. -- JSON `expected_revision`. - -Replacement writes: - -- Existing `PUT /api/runs/:filename/tags` replacement semantics should require a matching revision once the field is available. -- On mismatch, return `409` with `error: "stale_write"`, `refresh_required: true`, `current_revision`, and current tags/feedback payload. - -Mergeable operations: - -- Add a v1 operation endpoint for explicit tag add/remove, or extend the existing endpoint with `operation: "add" | "remove"` rather than overloading array replacement. -- If the page is stale, recompute the operation against current tags and commit only if validation still passes. -- Return `merged: true`, `base_revision`, and `current_revision` when a stale add/remove was safely merged. - -Feedback: - -- Current feedback replacement should reject stale writes. -- If feedback later becomes per-run/per-test operation-shaped, append/update operations can opt into the same safe merge rule. - ---- - -## Interaction With Metadata Files - -V1 target paths: - -- `.agentv/results///tags.json` is the canonical local tag sidecar. -- `runs///tags.json` is the canonical published result-branch tag sidecar when mutable metadata is colocated with published run bundles. -- `feedback.json` remains compatibility storage for current Dashboard feedback until a separate cleanup moves feedback to a run-local file. -- Future result metadata files may join the same safe-path allowlist only if they are small, JSON-readable, and scoped to result metadata. - -Compatibility: - -- Existing `metadata/runs/**/tags.json` files can be read as legacy or implementation-detail metadata during a transition, but new conflict-resolution writes should target colocated `tags.json`. -- The conflict resolver should report legacy overlay conflicts if present, but it should not introduce a new local metadata folder. -- No `tag-events.jsonl` or raw event stream is introduced in this feature. - ---- - -## Git Safety And Auditability - -Safety constraints: - -- Never push directly to `main`. -- Never reset the source repository worktree to resolve result metadata. -- Never operate on paths outside the safe mutable metadata allowlist. -- Never force push without a successful backup ref and a force-with-lease check. -- Treat branch protection failures as blocked states. - -Audit fields: - -- Resolution responses include `action`, `branch`, `incoming_commit`, `outgoing_commit`, `base_commit`, `resolved_paths`, and `created_commit`. -- Force-push responses also include `backup_ref`, `backup_commit`, and `lease_commit`. -- Commits created by conflict resolution include a concise message and can include trailers such as `AgentV-Conflict-Action`, `AgentV-Resolved-Paths`, and `AgentV-Backup-Ref` when applicable. - ---- - -## V1 Scope - -- Extend the backend status contract for conflict refs, revisions, conflict files, and stale-state responses. -- Add backend resolution actions for pull remote overwrite, force push local with backup, and per-file accept incoming/outgoing. -- Add optimistic concurrency to tag writes and the current feedback write path. -- Add Dashboard conflict view with tree, inline diff, global action buttons, per-file accept buttons, confirmation copy, and loading/error/success states. -- Keep v1 limited to small JSON/text metadata files. - -### Deferred To Follow-Up Work - -- JSON-aware field-level merge UI. -- Raw operation logs, `tag-events.jsonl`, CRDTs, or multi-actor event folding. -- Broad immutable artifact conflict resolution. -- Moving feedback to a run-local storage file, unless that is needed as a small prerequisite for the first implementation slice. -- CLI command wrappers for conflict resolution. -- Hosted multi-tenant authorization policy beyond the project-scoped API contract. -- Private evidence branch automation for UAT artifacts. - ---- - -## Implementation Units - -### U1. Conflict Status And Diff Contract - -- **Goal:** Extend sync/status and add lazy conflict diff APIs with incoming/outgoing/base refs, commits, safe paths, revisions, and file statuses. -- **Files:** `packages/core/src/evaluation/results-repo.ts`, `apps/cli/src/commands/results/serve.ts`, `apps/dashboard/src/lib/types.ts`, `apps/dashboard/src/lib/api.ts`, `packages/core/test/evaluation/results-repo.test.ts`, `apps/cli/test/commands/results/serve.test.ts`. -- **Approach:** Preserve existing status fields and add conflict fields additively. Reuse Git inspection for ahead/behind and add merge-base/blob resolution only when status requires it. -- **Test Scenarios:** Diverged metadata branch returns incoming/outgoing/base commits. Dirty safe tags path returns `dirty_paths` and conflict file metadata. Unsafe dirty path blocks automated resolution. Conflict diff route returns hunks for a `tags.json` conflict. Missing base returns `base_commit` omitted without failing the response. -- **Verification:** Existing clean, behind, ahead, dirty, and conflicted sync tests still pass, and new conflict responses are stable snake_case JSON. - -### U2. Resolution Actions And Force-Push Backup - -- **Goal:** Implement pull remote overwrite, force push local with remote backup ref, and per-file accept incoming/outgoing for safe metadata files. -- **Files:** `packages/core/src/evaluation/results-repo.ts`, `apps/cli/src/commands/results/serve.ts`, `apps/cli/test/commands/results/serve.test.ts`, `packages/core/test/evaluation/results-repo.test.ts`. -- **Approach:** Add one action endpoint over core helpers. Force push creates `refs/heads/agentv/backups//-` first, then pushes with a lease. Pull overwrite fetches and moves only the results checkout/ref after validating safe paths. -- **Test Scenarios:** Force push creates backup branch containing the old remote commit before overwriting. Force push aborts if backup creation fails. Force push aborts if the lease commit changed after preview. Pull overwrite discards local tag changes and reports overwritten paths. Per-file incoming writes remote content and commits resolution. Per-file outgoing keeps local content and rejects if remote changed again. -- **Verification:** Temp Git remote tests prove remote branch, backup branch, and local checkout end in expected commits. - -### U3. Optimistic Concurrency For Metadata Writes - -- **Goal:** Add revision/etag checks to tag and feedback writes and safe merge behavior for explicit add/remove operations. -- **Files:** `apps/cli/src/commands/results/serve.ts`, `apps/cli/src/commands/results/run-tags.ts`, `apps/dashboard/src/lib/api.ts`, `apps/dashboard/src/lib/types.ts`, `apps/cli/test/commands/results/serve.test.ts`, `apps/dashboard/src/lib/api.test.ts`. -- **Approach:** Return metadata revision tokens from read paths. Require matching revisions for replacement writes. Add or shape explicit tag add/remove operations so stale but deterministic changes can merge. -- **Test Scenarios:** Fresh tag replacement succeeds with matching revision. Stale replacement returns `409 stale_write` with current tags. Stale add tag merges when the tag is still valid and absent. Stale remove tag merges when the tag exists. Feedback replacement rejects stale revision. -- **Verification:** Stale browser writes cannot silently overwrite newer remote metadata. - -### U4. Dashboard Conflict Resolution UX - -- **Goal:** Add the Dashboard conflict surface with tree view, inline diffs, global actions, per-file choices, confirmations, and status states. -- **Files:** `apps/dashboard/src/components/RunSourceToolbar.tsx`, `apps/dashboard/src/lib/project-sync-status.ts`, `apps/dashboard/src/lib/types.ts`, `apps/dashboard/src/lib/api.ts`, new conflict components under `apps/dashboard/src/components/`, `apps/dashboard/src/lib/project-sync-status.test.ts`, component tests where existing patterns support them. -- **Approach:** Keep the toolbar compact and open a dedicated conflict panel/dialog only when needed. Use the lazy diff route to populate expanded nodes. After any action, invalidate remote status and run list queries. -- **Test Scenarios:** Conflicted status shows `Resolve Conflicts` and disables plain sync. Tree groups nested run paths. Expanding a file renders incoming/outgoing color-coded diff rows with text labels. Accept incoming/outgoing updates selected resolution state. Force push confirmation shows backup behavior. Stale resolution error refreshes status and keeps user choices from being falsely applied. -- **Verification:** Browser UAT covers conflicted, pull overwrite, force-push backup, and per-file resolution flows on desktop and mobile viewports. - ---- - -## Tests And UAT Expectations - -Automated tests: - -- Core Git integration tests use temporary local remotes for clean, dirty, behind, diverged, conflicted, backup, lease rejection, protected-ref-like rejection, and per-file resolution cases. -- API tests cover project-scoped and unscoped routes. -- Dashboard unit/component tests cover state derivation, labels, disabled states, and conflict tree interactions. -- Metadata write tests cover revision match, stale replacement rejection, and safe add/remove merge. - -Manual UAT: - -- Red: reproduce a stale browser tag overwrite on the pre-change branch and record that the newer remote tag can be lost or only manually recovered. -- Green: verify the same stale replacement is rejected with refresh-required state. -- Green: create a remote/local `tags.json` conflict and resolve it with accept incoming. -- Green: create the same conflict and resolve it with accept outgoing. -- Green: force push local metadata and verify the backup branch points to the pre-overwrite remote commit. -- Green: pull remote overwrite and verify local unsynced metadata is discarded without changing source code branches. -- Browser UAT evidence should follow `.agents/verification.md`: keep screenshots out of the public repo and publish reviewable artifacts to an `agentv-private` evidence branch. - ---- - -## Open Questions - -- Whether feedback should move from project-level `feedback.json` to run-local `runs///feedback.json` before or after the conflict UX lands. -- Whether hosted Dashboard deployments need a stricter authorization role for force push than for normal metadata edits. -- Whether the UI should allow partial per-file resolution in v1 or require all conflicted files to have a selected resolution before applying. diff --git a/packages/core/src/evaluation/loaders/config-loader.ts b/packages/core/src/evaluation/loaders/config-loader.ts index 6030be217..59f8d1b15 100644 --- a/packages/core/src/evaluation/loaders/config-loader.ts +++ b/packages/core/src/evaluation/loaders/config-loader.ts @@ -37,6 +37,8 @@ export type ExecutionDefaults = { readonly pool_slots?: number; }; +export type ResultPushConflictPolicy = 'block' | 'backup_and_force_push'; + export type ResultsConfig = { readonly mode?: 'github'; /** Legacy shorthand or Git remote URL for a managed results clone. */ @@ -54,6 +56,7 @@ export type ResultsConfig = { readonly sync?: { readonly auto_push?: boolean; readonly require_push?: boolean; + readonly push_conflict_policy?: ResultPushConflictPolicy; }; readonly branch_prefix?: string; }; @@ -791,9 +794,23 @@ export function parseResultsConfig(raw: unknown, configPath: string): ResultsCon logWarning(`Invalid results.sync.require_push in ${configPath}, expected boolean`); return undefined; } + if ( + syncObj.push_conflict_policy !== undefined && + syncObj.push_conflict_policy !== 'block' && + syncObj.push_conflict_policy !== 'backup_and_force_push' + ) { + logWarning( + `Invalid results.sync.push_conflict_policy in ${configPath}, expected 'block' or 'backup_and_force_push'`, + ); + return undefined; + } sync = { ...(typeof syncObj.auto_push === 'boolean' && { auto_push: syncObj.auto_push }), ...(typeof syncObj.require_push === 'boolean' && { require_push: syncObj.require_push }), + ...((syncObj.push_conflict_policy === 'block' || + syncObj.push_conflict_policy === 'backup_and_force_push') && { + push_conflict_policy: syncObj.push_conflict_policy, + }), }; } diff --git a/packages/core/src/evaluation/results-repo.ts b/packages/core/src/evaluation/results-repo.ts index 33eb3bfcd..53cfcb8e8 100644 --- a/packages/core/src/evaluation/results-repo.ts +++ b/packages/core/src/evaluation/results-repo.ts @@ -15,12 +15,14 @@ import path from 'node:path'; import { promisify } from 'node:util'; import { getAgentvDataDir } from '../paths.js'; -import type { ResultsConfig } from './loaders/config-loader.js'; +import type { ResultPushConflictPolicy, ResultsConfig } from './loaders/config-loader.js'; import { AGENTV_RESULTS_ARTIFACTS_REF, AGENTV_RESULTS_PRIMARY_REF, } from './result-artifact-contract.js'; +export type { ResultPushConflictPolicy } from './loaders/config-loader.js'; + const execFileAsync = promisify(execFile); // Local working-tree run workspace inside the eval repo. Local commands // (`agentv eval` default --output, inspect/trend/export/combine/serve) read and @@ -78,6 +80,7 @@ export type ResultsRepoSyncStatus = | 'diverged' | 'dirty' | 'conflicted' + | 'push_conflict' | 'syncing'; export interface ResultsRepoStatus { @@ -88,6 +91,7 @@ export interface ResultsRepoStatus { readonly path?: string; readonly auto_push?: boolean; readonly require_push?: boolean; + readonly push_conflict_policy?: ResultPushConflictPolicy; readonly branch_prefix?: string; readonly local_dir?: string; readonly last_synced_at?: string; @@ -106,6 +110,14 @@ export interface ResultsRepoStatus { readonly pull_performed?: boolean; readonly push_performed?: boolean; readonly commit_created?: boolean; + readonly target_branch?: string; + readonly remote_commit?: string; + readonly local_commit?: string; + readonly backup_ref?: string; + readonly backup_commit?: string; + readonly previous_remote_commit?: string; + readonly force_pushed_commit?: string; + readonly lease_commit?: string; } export interface NormalizedResultsConfig { @@ -118,6 +130,7 @@ export interface NormalizedResultsConfig { readonly path: string; readonly auto_push: boolean; readonly require_push: boolean; + readonly push_conflict_policy: ResultPushConflictPolicy; readonly branch_prefix: string; /** @internal Runtime mode; not part of YAML wire format. */ readonly storageBranchWorktree: boolean; @@ -125,6 +138,22 @@ export interface NormalizedResultsConfig { type StorageBranchResultsConfig = NormalizedResultsConfig & { readonly branch: string }; +export interface DirectPushResultsResult { + readonly changed: boolean; + readonly blocked?: boolean; + readonly block_reason?: string; + readonly sync_status?: ResultsRepoSyncStatus; + readonly push_conflict_policy: ResultPushConflictPolicy; + readonly target_branch?: string; + readonly remote_commit?: string; + readonly local_commit?: string; + readonly backup_ref?: string; + readonly backup_commit?: string; + readonly previous_remote_commit?: string; + readonly force_pushed_commit?: string; + readonly lease_commit?: string; +} + export interface CheckedOutResultsRepoBranch { readonly branchName: string; readonly baseBranch: string; @@ -200,6 +229,7 @@ export function normalizeResultsConfig( (repoUrl && useStorageBranchWorktree ? MANAGED_RESULTS_REMOTE : 'origin'); const autoPush = config.sync?.auto_push ?? config.auto_push === true; const requirePush = config.sync?.require_push === true; + const pushConflictPolicy = config.sync?.push_conflict_policy ?? 'block'; const resolvedRepoPath = repoPath ? resolveLocalPath(repoPath, baseDir) : undefined; const resolvedPath = explicitClonePath ? resolveLocalPath(explicitClonePath, baseDir) @@ -216,6 +246,7 @@ export function normalizeResultsConfig( path: resolvedPath, auto_push: autoPush, require_push: requirePush, + push_conflict_policy: pushConflictPolicy, branch_prefix: config.branch_prefix?.trim() || 'eval-results', storageBranchWorktree: useStorageBranchWorktree, }; @@ -668,6 +699,7 @@ export function getResultsRepoStatus(config?: ResultsConfig): ResultsRepoStatus path: normalized.path, auto_push: normalized.auto_push, require_push: normalized.require_push, + push_conflict_policy: normalized.push_conflict_policy, branch_prefix: normalized.branch_prefix, local_dir: normalized.path, last_synced_at: persisted.last_synced_at, @@ -1086,6 +1118,70 @@ function lastErrorForGitInspection( return undefined; } +type ResultsBranchPushDetails = { + readonly pushConflictPolicy: ResultPushConflictPolicy; + readonly targetBranch: string; + readonly remoteCommit?: string; + readonly localCommit?: string; + readonly backupRef?: string; + readonly backupCommit?: string; + readonly previousRemoteCommit?: string; + readonly forcePushedCommit?: string; + readonly leaseCommit?: string; +}; + +type ResultsBranchPushOutcome = + | { + readonly blocked: false; + readonly details?: ResultsBranchPushDetails; + } + | { + readonly blocked: true; + readonly blockReason: string; + readonly details: ResultsBranchPushDetails; + }; + +class ResultsBranchPushConflictError extends Error { + constructor(readonly result: DirectPushResultsResult) { + super(result.block_reason ?? 'Results branch push conflict'); + this.name = 'ResultsBranchPushConflictError'; + } +} + +function pushDetailsToWire( + details?: ResultsBranchPushDetails, +): Pick< + ResultsRepoStatus, + | 'push_conflict_policy' + | 'target_branch' + | 'remote_commit' + | 'local_commit' + | 'backup_ref' + | 'backup_commit' + | 'previous_remote_commit' + | 'force_pushed_commit' + | 'lease_commit' +> { + if (!details) { + return {}; + } + return { + push_conflict_policy: details.pushConflictPolicy, + target_branch: details.targetBranch, + ...(details.remoteCommit !== undefined && { remote_commit: details.remoteCommit }), + ...(details.localCommit !== undefined && { local_commit: details.localCommit }), + ...(details.backupRef !== undefined && { backup_ref: details.backupRef }), + ...(details.backupCommit !== undefined && { backup_commit: details.backupCommit }), + ...(details.previousRemoteCommit !== undefined && { + previous_remote_commit: details.previousRemoteCommit, + }), + ...(details.forcePushedCommit !== undefined && { + force_pushed_commit: details.forcePushedCommit, + }), + ...(details.leaseCommit !== undefined && { lease_commit: details.leaseCommit }), + }; +} + function withBlockedStatus( status: ResultsRepoStatus, blockReason: string, @@ -1093,10 +1189,12 @@ function withBlockedStatus( readonly pullPerformed?: boolean; readonly pushPerformed?: boolean; readonly commitCreated?: boolean; + readonly pushDetails?: ResultsBranchPushDetails; }, ): ResultsRepoStatus { return { ...status, + ...pushDetailsToWire(flags?.pushDetails), blocked: true, block_reason: blockReason, ...(flags?.pullPerformed !== undefined && { pull_performed: flags.pullPerformed }), @@ -1105,16 +1203,41 @@ function withBlockedStatus( }; } +function withPushConflictStatus( + status: ResultsRepoStatus, + blockReason: string, + details: ResultsBranchPushDetails, + flags: { + readonly pullPerformed: boolean; + readonly pushPerformed: boolean; + readonly commitCreated: boolean; + }, +): ResultsRepoStatus { + return withBlockedStatus( + { + ...status, + sync_status: 'push_conflict', + }, + blockReason, + { + ...flags, + pushDetails: details, + }, + ); +} + function withActionFlags( status: ResultsRepoStatus, flags: { readonly pullPerformed: boolean; readonly pushPerformed: boolean; readonly commitCreated: boolean; + readonly pushDetails?: ResultsBranchPushDetails; }, ): ResultsRepoStatus { return { ...status, + ...pushDetailsToWire(flags.pushDetails), blocked: false, pull_performed: flags.pullPerformed, push_performed: flags.pushPerformed, @@ -1176,6 +1299,151 @@ function getPushTargetBranch( return upstream?.startsWith(prefix) ? upstream.slice(prefix.length) : baseBranch; } +function timestampForBackupRef(date = new Date()): string { + const pad = (value: number) => String(value).padStart(2, '0'); + return `${date.getUTCFullYear()}${pad(date.getUTCMonth() + 1)}${pad(date.getUTCDate())}T${pad( + date.getUTCHours(), + )}${pad(date.getUTCMinutes())}${pad(date.getUTCSeconds())}Z`; +} + +function slugifyBackupTargetBranch(branch: string): string { + return ( + branch + .trim() + .replace(/[^A-Za-z0-9._-]+/g, '-') + .replace(/^-+|-+$/g, '') || 'results' + ); +} + +function buildResultsBackupRef(targetBranch: string, remoteCommit: string): string { + return `agentv/backups/${timestampForBackupRef()}-${slugifyBackupTargetBranch( + targetBranch, + )}-${remoteCommit.slice(0, 7)}`; +} + +async function getCommitSha(repoDir: string, ref: string | undefined): Promise { + if (!ref) { + return undefined; + } + const { stdout } = await runGit(['rev-parse', '--verify', `${ref}^{commit}`], { + cwd: repoDir, + check: false, + }); + const sha = stdout.trim(); + return sha.length > 0 ? sha : undefined; +} + +function isNonFastForwardPushError(error: unknown): boolean { + const text = gitErrorText(error); + return ( + text.includes('non-fast-forward') || + text.includes('fetch first') || + text.includes('tip is behind its remote') || + text.includes('note about fast-forwards') || + text.includes('stale info') + ); +} + +function formatShortSha(sha: string | undefined): string { + return sha ? sha.slice(0, 12) : 'unknown'; +} + +function buildBlockedPushConflictReason(details: ResultsBranchPushDetails): string { + return `Results branch push conflict on ${details.targetBranch}: remote ${formatShortSha( + details.remoteCommit, + )}, local ${formatShortSha(details.localCommit)}. Configure results.sync.push_conflict_policy: backup_and_force_push to back up the remote ref before replacing it.`; +} + +async function resolveResultBranchPushConflict(params: { + readonly normalized: StorageBranchResultsConfig; + readonly repoDir: string; + readonly targetBranch: string; + readonly sourceRef: string; +}): Promise { + await fetchResultsRepo(params.repoDir, params.normalized.remote, params.targetBranch); + const remoteRef = remoteBranchRef(params.targetBranch, params.normalized.remote); + const remoteCommit = await getCommitSha(params.repoDir, remoteRef); + const localCommit = await getCommitSha(params.repoDir, params.sourceRef); + const baseDetails: ResultsBranchPushDetails = { + pushConflictPolicy: params.normalized.push_conflict_policy, + targetBranch: params.targetBranch, + ...(remoteCommit !== undefined && { + remoteCommit, + previousRemoteCommit: remoteCommit, + leaseCommit: remoteCommit, + }), + ...(localCommit !== undefined && { localCommit }), + }; + + if (!remoteCommit) { + return { + blocked: true, + blockReason: `Results branch push conflict on ${params.targetBranch}: remote commit could not be resolved after fetch`, + details: baseDetails, + }; + } + + if (params.normalized.push_conflict_policy === 'block') { + return { + blocked: true, + blockReason: buildBlockedPushConflictReason(baseDetails), + details: baseDetails, + }; + } + + const backupRef = buildResultsBackupRef(params.targetBranch, remoteCommit); + const backupDetails: ResultsBranchPushDetails = { + ...baseDetails, + backupRef, + backupCommit: remoteCommit, + }; + + try { + await assertValidResultsBranchName(params.repoDir, backupRef); + await runGit( + ['push', '--porcelain', params.normalized.remote, `${remoteCommit}:refs/heads/${backupRef}`], + { cwd: params.repoDir }, + ); + } catch (error) { + return { + blocked: true, + blockReason: `Results branch backup creation failed for ${params.targetBranch} at ${formatShortSha( + remoteCommit, + )}: ${getStatusMessage(error)}`, + details: backupDetails, + }; + } + + try { + await runGit( + [ + 'push', + '--porcelain', + `--force-with-lease=refs/heads/${params.targetBranch}:${remoteCommit}`, + params.normalized.remote, + `${params.sourceRef}:refs/heads/${params.targetBranch}`, + ], + { cwd: params.repoDir }, + ); + } catch (error) { + return { + blocked: true, + blockReason: `Results branch force push lease failed for ${params.targetBranch}; remote changed after backup ${backupRef} was created with lease ${formatShortSha( + remoteCommit, + )}: ${getStatusMessage(error)}`, + details: backupDetails, + }; + } + + return { + blocked: false, + details: { + ...backupDetails, + ...(localCommit !== undefined && { forcePushedCommit: localCommit }), + }, + }; +} + async function statusFromInspection( normalized: NormalizedResultsConfig, repoDir: string, @@ -1278,6 +1546,7 @@ export async function syncResultsRepoForProject(config: ResultsConfig): Promise< let pullPerformed = false; let pushPerformed = false; let commitCreated = false; + let pushDetails: ResultsBranchPushDetails | undefined; try { const repoDir = await ensureResultsRepoClone(normalized); @@ -1347,14 +1616,53 @@ export async function syncResultsRepoForProject(config: ResultsConfig): Promise< if ((inspection.ahead ?? 0) > 0 && (inspection.behind ?? 0) > 0) { const status = withGitInspection(getResultsRepoStatus(normalized), inspection); - updateStatusFile(normalized, { - last_error: 'Results repo local and remote histories have diverged', - }); - return withBlockedStatus(status, 'Results repo local and remote histories have diverged', { - pullPerformed, - pushPerformed, - commitCreated, + if (!normalized.branch) { + updateStatusFile(normalized, { + last_error: 'Results repo local and remote histories have diverged', + }); + return withBlockedStatus( + status, + 'Results repo local and remote histories have diverged', + { + pullPerformed, + pushPerformed, + commitCreated, + }, + ); + } + const localRef = `refs/heads/${normalized.branch}`; + const aheadPaths = await getAheadPaths(repoDir, inspection.upstream, localRef); + if (!inspection.upstream || !areSafeResultsRepoPaths(aheadPaths)) { + const reason = !inspection.upstream + ? 'Results repo has no upstream branch to push to' + : 'Results repo has non-results committed changes'; + updateStatusFile(normalized, { last_error: reason }); + return withBlockedStatus(status, reason, { + pullPerformed, + pushPerformed, + commitCreated, + }); + } + const outcome = await resolveResultBranchPushConflict({ + normalized: { ...normalized, branch: normalized.branch }, + repoDir, + targetBranch: normalized.branch, + sourceRef: localRef, }); + pushDetails = outcome.details; + if (outcome.blocked) { + updateStatusFile(normalized, { last_error: outcome.blockReason }); + return withPushConflictStatus(status, outcome.blockReason, outcome.details, { + pullPerformed, + pushPerformed, + commitCreated, + }); + } + pushPerformed = true; + await fetchResultsRepo(repoDir, normalized.remote, normalized.branch).catch( + () => undefined, + ); + inspection = await inspectResultsStorageBranchGit(repoDir, normalized); } if (inspection.syncStatus === 'dirty') { @@ -1446,23 +1754,51 @@ export async function syncResultsRepoForProject(config: ResultsConfig): Promise< () => undefined, ); } catch (error) { - updateStatusFile(normalized, { last_error: getStatusMessage(error) }); - if (normalized.require_push) { - throw error; + if (isNonFastForwardPushError(error)) { + const outcome = await resolveResultBranchPushConflict({ + normalized: { ...normalized, branch: normalized.branch }, + repoDir, + targetBranch: normalized.branch, + sourceRef: localRef, + }); + pushDetails = outcome.details; + if (!outcome.blocked) { + pushPerformed = true; + await fetchResultsRepo(repoDir, normalized.remote, normalized.branch).catch( + () => undefined, + ); + inspection = await inspectResultsStorageBranchGit(repoDir, normalized); + } else { + updateStatusFile(normalized, { last_error: outcome.blockReason }); + const status = withGitInspection( + getResultsRepoStatus(normalized), + await inspectResultsStorageBranchGit(repoDir, normalized), + ); + return withPushConflictStatus(status, outcome.blockReason, outcome.details, { + pullPerformed, + pushPerformed, + commitCreated, + }); + } + } else { + updateStatusFile(normalized, { last_error: getStatusMessage(error) }); + if (normalized.require_push) { + throw error; + } + const status = withGitInspection( + getResultsRepoStatus(normalized), + await inspectResultsStorageBranchGit(repoDir, normalized), + ); + return withBlockedStatus( + status, + `Results repo push was rejected: ${getStatusMessage(error)}`, + { + pullPerformed, + pushPerformed, + commitCreated, + }, + ); } - const status = withGitInspection( - getResultsRepoStatus(normalized), - await inspectResultsStorageBranchGit(repoDir, normalized), - ); - return withBlockedStatus( - status, - `Results repo push was rejected: ${getStatusMessage(error)}`, - { - pullPerformed, - pushPerformed, - commitCreated, - }, - ); } } } @@ -1475,7 +1811,7 @@ export async function syncResultsRepoForProject(config: ResultsConfig): Promise< getResultsRepoStatus(normalized), await inspectResultsStorageBranchGit(repoDir, normalized), ); - return withActionFlags(status, { pullPerformed, pushPerformed, commitCreated }); + return withActionFlags(status, { pullPerformed, pushPerformed, commitCreated, pushDetails }); } await fetchResultsRepo(repoDir, normalized.remote, normalized.branch); await checkoutConfiguredResultsBranch(repoDir, normalized); @@ -1556,14 +1892,38 @@ export async function syncResultsRepoForProject(config: ResultsConfig): Promise< if (inspection.syncStatus === 'diverged') { const status = withGitInspection(getResultsRepoStatus(normalized), inspection); - updateStatusFile(normalized, { - last_error: 'Results repo local and remote histories have diverged', - }); - return withBlockedStatus(status, 'Results repo local and remote histories have diverged', { - pullPerformed, - pushPerformed, - commitCreated, + const aheadPaths = await getAheadPaths(repoDir, inspection.upstream); + if (!inspection.upstream || !areSafeResultsRepoPaths(aheadPaths)) { + const reason = !inspection.upstream + ? 'Results repo has no upstream branch to push to' + : 'Results repo has non-results committed changes'; + updateStatusFile(normalized, { last_error: reason }); + return withBlockedStatus(status, reason, { + pullPerformed, + pushPerformed, + commitCreated, + }); + } + const baseBranch = normalized.branch ?? (await resolveDefaultBranch(repoDir)); + const targetBranch = getPushTargetBranch(inspection.upstream, baseBranch, normalized.remote); + const outcome = await resolveResultBranchPushConflict({ + normalized: { ...normalized, branch: targetBranch }, + repoDir, + targetBranch, + sourceRef: 'HEAD', }); + pushDetails = outcome.details; + if (outcome.blocked) { + updateStatusFile(normalized, { last_error: outcome.blockReason }); + return withPushConflictStatus(status, outcome.blockReason, outcome.details, { + pullPerformed, + pushPerformed, + commitCreated, + }); + } + pushPerformed = true; + await fetchResultsRepo(repoDir, normalized.remote, normalized.branch); + inspection = await inspectResultsRepoGit(repoDir, normalized); } if ((inspection.behind ?? 0) > 0 && (inspection.ahead ?? 0) === 0) { @@ -1628,18 +1988,45 @@ export async function syncResultsRepoForProject(config: ResultsConfig): Promise< await fetchResultsRepo(repoDir, normalized.remote, normalized.branch); inspection = await inspectResultsRepoGit(repoDir, normalized); } catch (error) { - await fetchResultsRepo(repoDir, normalized.remote, normalized.branch).catch( - () => undefined, - ); - inspection = await inspectResultsRepoGit(repoDir, normalized); - const status = withGitInspection(getResultsRepoStatus(normalized), inspection); - const reason = `Results repo push was rejected: ${getStatusMessage(error)}`; - updateStatusFile(normalized, { last_error: reason }); - return withBlockedStatus(status, reason, { - pullPerformed, - pushPerformed, - commitCreated, - }); + if (isNonFastForwardPushError(error)) { + const outcome = await resolveResultBranchPushConflict({ + normalized: { ...normalized, branch: targetBranch }, + repoDir, + targetBranch, + sourceRef: 'HEAD', + }); + pushDetails = outcome.details; + if (!outcome.blocked) { + pushPerformed = true; + await fetchResultsRepo(repoDir, normalized.remote, normalized.branch); + inspection = await inspectResultsRepoGit(repoDir, normalized); + } else { + await fetchResultsRepo(repoDir, normalized.remote, normalized.branch).catch( + () => undefined, + ); + inspection = await inspectResultsRepoGit(repoDir, normalized); + const status = withGitInspection(getResultsRepoStatus(normalized), inspection); + updateStatusFile(normalized, { last_error: outcome.blockReason }); + return withPushConflictStatus(status, outcome.blockReason, outcome.details, { + pullPerformed, + pushPerformed, + commitCreated, + }); + } + } else { + await fetchResultsRepo(repoDir, normalized.remote, normalized.branch).catch( + () => undefined, + ); + inspection = await inspectResultsRepoGit(repoDir, normalized); + const status = withGitInspection(getResultsRepoStatus(normalized), inspection); + const reason = `Results repo push was rejected: ${getStatusMessage(error)}`; + updateStatusFile(normalized, { last_error: reason }); + return withBlockedStatus(status, reason, { + pullPerformed, + pushPerformed, + commitCreated, + }); + } } } @@ -1652,6 +2039,7 @@ export async function syncResultsRepoForProject(config: ResultsConfig): Promise< pullPerformed, pushPerformed, commitCreated, + pushDetails, }); } catch (error) { updateStatusFile(normalized, { @@ -1808,8 +2196,6 @@ export async function createDraftResultsPr(params: { return stdout.trim(); } -const DIRECT_PUSH_MAX_RETRIES = 3; - async function hasUnpushedCommits( repoDir: string, upstreamRef: string, @@ -1822,19 +2208,6 @@ async function hasUnpushedCommits( return Number.parseInt(stdout.trim(), 10) > 0; } -async function countUnpushedCommits( - repoDir: string, - upstreamRef: string, - branch: string, -): Promise { - const { stdout } = await runGit(['rev-list', '--count', `${upstreamRef}..refs/heads/${branch}`], { - cwd: repoDir, - check: false, - }); - const count = Number.parseInt(stdout.trim(), 10); - return Number.isFinite(count) ? count : 0; -} - async function assertValidResultsBranchName(repoDir: string, branch: string): Promise { if ( branch.length === 0 || @@ -2424,61 +2797,98 @@ async function commitResultsRunWithTemporaryIndex(params: { } } +function buildDirectPushResult( + normalized: NormalizedResultsConfig, + changed: boolean, + outcome?: ResultsBranchPushOutcome, +): DirectPushResultsResult { + const details = outcome?.details; + return { + changed, + push_conflict_policy: normalized.push_conflict_policy, + ...(outcome?.blocked === true && { + blocked: true, + block_reason: outcome.blockReason, + sync_status: 'push_conflict' as const, + }), + ...pushDetailsToWire(details), + }; +} + +function mergeDirectPushResults( + normalized: NormalizedResultsConfig, + results: readonly DirectPushResultsResult[], +): DirectPushResultsResult { + const blocked = results.find((result) => result.blocked); + if (blocked) { + return blocked; + } + const detailed = [...results].reverse().find((result) => result.backup_ref !== undefined); + return { + changed: results.some((result) => result.changed), + push_conflict_policy: normalized.push_conflict_policy, + ...(detailed?.backup_ref !== undefined && { + backup_ref: detailed.backup_ref, + }), + ...(detailed?.target_branch !== undefined && { target_branch: detailed.target_branch }), + ...(detailed?.remote_commit !== undefined && { remote_commit: detailed.remote_commit }), + ...(detailed?.local_commit !== undefined && { local_commit: detailed.local_commit }), + ...(detailed?.backup_commit !== undefined && { backup_commit: detailed.backup_commit }), + ...(detailed?.previous_remote_commit !== undefined && { + previous_remote_commit: detailed.previous_remote_commit, + }), + ...(detailed?.force_pushed_commit !== undefined && { + force_pushed_commit: detailed.force_pushed_commit, + }), + ...(detailed?.lease_commit !== undefined && { lease_commit: detailed.lease_commit }), + }; +} + async function pushDirectResultsToStorageBranch(params: { readonly normalized: StorageBranchResultsConfig; readonly repoDir: string; readonly storageBranch: string; - readonly upstreamRef?: string; - readonly sourceDir: string; - readonly destinationPath: string; - readonly commitMessage: string; - readonly targetRunId: string; -}): Promise { - for (let attempt = 1; attempt <= DIRECT_PUSH_MAX_RETRIES; attempt++) { - try { - await runGit( - [ - 'push', - '--porcelain', - params.normalized.remote, - `refs/heads/${params.storageBranch}:refs/heads/${params.storageBranch}`, - ], - { cwd: params.repoDir }, - ); - updateStatusFile(params.normalized, { - last_synced_at: new Date().toISOString(), - last_error: undefined, - }); - await fetchResultsRepo(params.repoDir, params.normalized.remote, params.storageBranch).catch( - () => undefined, - ); - return; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - if (attempt < DIRECT_PUSH_MAX_RETRIES && message.includes('non-fast-forward')) { - await fetchResultsRepo(params.repoDir, params.normalized.remote, params.storageBranch); - const remoteRef = remoteBranchRef(params.storageBranch, params.normalized.remote); - const localOnlyCount = (await gitRefExists(params.repoDir, remoteRef)) - ? await countUnpushedCommits(params.repoDir, remoteRef, params.storageBranch) - : 0; - if (localOnlyCount > 1) { - throw new Error( - `Results branch has ${localOnlyCount} local commits and remote advanced; push manually after reconciling ${params.storageBranch}`, - ); - } - await commitResultsRunWithTemporaryIndex({ - normalized: params.normalized, - repoDir: params.repoDir, - sourceDir: params.sourceDir, - destinationPath: params.destinationPath, - commitMessage: params.commitMessage, - targetRunId: params.targetRunId, - preferRemoteBase: true, - }); - } else { - throw error; - } +}): Promise { + try { + await runGit( + [ + 'push', + '--porcelain', + params.normalized.remote, + `refs/heads/${params.storageBranch}:refs/heads/${params.storageBranch}`, + ], + { cwd: params.repoDir }, + ); + updateStatusFile(params.normalized, { + last_synced_at: new Date().toISOString(), + last_error: undefined, + }); + await fetchResultsRepo(params.repoDir, params.normalized.remote, params.storageBranch).catch( + () => undefined, + ); + return undefined; + } catch (error) { + if (!isNonFastForwardPushError(error)) { + throw error; + } + const outcome = await resolveResultBranchPushConflict({ + normalized: params.normalized, + repoDir: params.repoDir, + targetBranch: params.storageBranch, + sourceRef: `refs/heads/${params.storageBranch}`, + }); + if (outcome.blocked) { + updateStatusFile(params.normalized, { last_error: outcome.blockReason }); + return outcome; } + updateStatusFile(params.normalized, { + last_synced_at: new Date().toISOString(), + last_error: undefined, + }); + await fetchResultsRepo(params.repoDir, params.normalized.remote, params.storageBranch).catch( + () => undefined, + ); + return outcome; } } @@ -2490,7 +2900,7 @@ async function commitAndMaybePushRunTree(params: { readonly commitMessage: string; readonly targetRunId: string; readonly shouldPush: boolean; -}): Promise { +}): Promise { const result = await commitResultsRunWithTemporaryIndex({ normalized: params.normalized, repoDir: params.repoDir, @@ -2502,7 +2912,7 @@ async function commitAndMaybePushRunTree(params: { if (!params.shouldPush) { updateStatusFile(params.normalized, { last_error: undefined }); - return result.commitCreated; + return buildDirectPushResult(params.normalized, result.commitCreated); } if (!result.commitCreated) { @@ -2516,7 +2926,7 @@ async function commitAndMaybePushRunTree(params: { : false : localBranchExists; if (!hasUnpushed) { - return false; + return buildDirectPushResult(params.normalized, false); } const aheadPaths = result.upstreamRef @@ -2531,35 +2941,25 @@ async function commitAndMaybePushRunTree(params: { updateStatusFile(params.normalized, { last_error: error.message }); throw error; } - await pushDirectResultsToStorageBranch({ + const outcome = await pushDirectResultsToStorageBranch({ normalized: params.normalized, repoDir: params.repoDir, storageBranch: params.normalized.branch, - upstreamRef: result.upstreamRef, - sourceDir: params.sourceDir, - destinationPath: params.destinationPath, - commitMessage: params.commitMessage, - targetRunId: params.targetRunId, }); - return true; + return buildDirectPushResult(params.normalized, !outcome?.blocked, outcome); } - await pushDirectResultsToStorageBranch({ + const outcome = await pushDirectResultsToStorageBranch({ normalized: params.normalized, repoDir: params.repoDir, storageBranch: params.normalized.branch, - upstreamRef: result.upstreamRef, - sourceDir: params.sourceDir, - destinationPath: params.destinationPath, - commitMessage: params.commitMessage, - targetRunId: params.targetRunId, }); - return true; + return buildDirectPushResult(params.normalized, !outcome?.blocked, outcome); } /** * Push results directly to the configured storage branch of the results repo. - * Handles non-fast-forward conflicts by fetching, rebasing, and retrying. + * Handles non-fast-forward conflicts with the configured push conflict policy. * Returns true if artifacts were pushed, false if no changes were detected. */ export async function directPushResults(params: { @@ -2568,6 +2968,19 @@ export async function directPushResults(params: { readonly destinationPath: string; readonly commitMessage: string; }): Promise { + const result = await directPushResultsWithDetails(params); + if (result.blocked) { + throw new ResultsBranchPushConflictError(result); + } + return result.changed; +} + +export async function directPushResultsWithDetails(params: { + readonly config: ResultsConfig; + readonly sourceDir: string; + readonly destinationPath: string; + readonly commitMessage: string; +}): Promise { const normalized = normalizeResultsConfig(params.config); const repoDir = await ensureResultsRepoClone(normalized); await fetchResultsRepo(repoDir, normalized.remote, normalized.branch).catch((error) => { @@ -2596,7 +3009,7 @@ export async function directPushResults(params: { pointers: sidecarPointers, }); - let sidecarChanged = false; + const pushResults: DirectPushResultsResult[] = []; if (sidecar) { await fetchResultsRepo(repoDir, normalized.remote, AGENTV_RESULTS_ARTIFACTS_REF).catch( (error) => { @@ -2605,7 +3018,7 @@ export async function directPushResults(params: { } }, ); - sidecarChanged = await commitAndMaybePushRunTree({ + const sidecarResult = await commitAndMaybePushRunTree({ normalized: { ...normalized, branch: AGENTV_RESULTS_ARTIFACTS_REF, @@ -2617,18 +3030,24 @@ export async function directPushResults(params: { targetRunId, shouldPush, }); + pushResults.push(sidecarResult); + if (sidecarResult.blocked) { + return sidecarResult; + } } - const primaryChanged = await commitAndMaybePushRunTree({ - normalized: storageConfig, - repoDir, - sourceDir: publishedResultsSource?.sourceDir ?? params.sourceDir, - destinationPath: params.destinationPath, - commitMessage: params.commitMessage, - targetRunId, - shouldPush, - }); - return primaryChanged || sidecarChanged; + pushResults.push( + await commitAndMaybePushRunTree({ + normalized: storageConfig, + repoDir, + sourceDir: publishedResultsSource?.sourceDir ?? params.sourceDir, + destinationPath: params.destinationPath, + commitMessage: params.commitMessage, + targetRunId, + shouldPush, + }), + ); + return mergeDirectPushResults(normalized, pushResults); } finally { await publishedResultsSource?.cleanup().catch(() => undefined); await sidecar?.cleanup().catch(() => undefined); diff --git a/packages/core/src/evaluation/validation/config-validator.ts b/packages/core/src/evaluation/validation/config-validator.ts index 76b0f85d9..23834b24e 100644 --- a/packages/core/src/evaluation/validation/config-validator.ts +++ b/packages/core/src/evaluation/validation/config-validator.ts @@ -448,6 +448,18 @@ function validateResultsSyncAndBranchPrefix( `Field '${location}.sync.require_push' must be a boolean`, ); } + if ( + syncRecord.push_conflict_policy !== undefined && + syncRecord.push_conflict_policy !== 'block' && + syncRecord.push_conflict_policy !== 'backup_and_force_push' + ) { + addError( + errors, + filePath, + `${location}.sync.push_conflict_policy`, + `Field '${location}.sync.push_conflict_policy' must be 'block' or 'backup_and_force_push'`, + ); + } } } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index be52ff3ae..b62a061d2 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -118,6 +118,7 @@ export { pushResultsRepoBranch, createDraftResultsPr, directPushResults, + directPushResultsWithDetails, buildWipBranchName, setupWipWorktree, pushWipCheckpoint, @@ -126,10 +127,12 @@ export { materializeGitRun, readGitResultArtifact, type CheckedOutResultsRepoBranch, + type DirectPushResultsResult, type GitResultArtifactReadParams, type GitListedRun, type NormalizedResultsConfig, type PreparedResultsRepoBranch, + type ResultPushConflictPolicy, type ResultsRepoLocalPaths, type ResultsRepoSyncStatus, type ResultsRepoStatus, diff --git a/packages/core/src/projects.ts b/packages/core/src/projects.ts index 505c0fb02..71c933d0c 100644 --- a/packages/core/src/projects.ts +++ b/packages/core/src/projects.ts @@ -63,6 +63,7 @@ import { getAgentvConfigDir } from './paths.js'; export interface ProjectResultsSyncConfig { autoPush?: boolean; requirePush?: boolean; + pushConflictPolicy?: 'block' | 'backup_and_force_push'; } export interface ProjectResultsConfig { @@ -104,6 +105,7 @@ export function getProjectsRegistryPath(): string { interface ProjectResultsSyncYaml { auto_push?: boolean; require_push?: boolean; + push_conflict_policy?: 'block' | 'backup_and_force_push'; } interface ProjectResultsYaml { @@ -195,13 +197,21 @@ function fromYaml(raw: unknown): ProjectEntry | null { ...(resultsBranch ? { branch: resultsBranch } : {}), ...(resultsRemote ? { remote: resultsRemote } : {}), ...(clonePath ? { path: clonePath } : {}), - ...(sync && (typeof sync.auto_push === 'boolean' || typeof sync.require_push === 'boolean') + ...(sync && + (typeof sync.auto_push === 'boolean' || + typeof sync.require_push === 'boolean' || + sync.push_conflict_policy === 'block' || + sync.push_conflict_policy === 'backup_and_force_push') ? { sync: { ...(typeof sync.auto_push === 'boolean' ? { autoPush: sync.auto_push } : {}), ...(typeof sync.require_push === 'boolean' ? { requirePush: sync.require_push } : {}), + ...(sync.push_conflict_policy === 'block' || + sync.push_conflict_policy === 'backup_and_force_push' + ? { pushConflictPolicy: sync.push_conflict_policy } + : {}), }, } : {}), @@ -228,7 +238,9 @@ function toYaml(entry: ProjectEntry): ProjectEntryYaml { }; if (entry.results) { const resultsSync = - entry.results.sync?.autoPush !== undefined || entry.results.sync?.requirePush !== undefined + entry.results.sync?.autoPush !== undefined || + entry.results.sync?.requirePush !== undefined || + entry.results.sync?.pushConflictPolicy !== undefined ? { sync: { ...(entry.results.sync?.autoPush !== undefined && { @@ -237,6 +249,9 @@ function toYaml(entry: ProjectEntry): ProjectEntryYaml { ...(entry.results.sync?.requirePush !== undefined && { require_push: entry.results.sync.requirePush, }), + ...(entry.results.sync?.pushConflictPolicy !== undefined && { + push_conflict_policy: entry.results.sync.pushConflictPolicy, + }), }, } : {}; diff --git a/packages/core/test/evaluation/loaders/config-loader.test.ts b/packages/core/test/evaluation/loaders/config-loader.test.ts index edec63f52..dffc15652 100644 --- a/packages/core/test/evaluation/loaders/config-loader.test.ts +++ b/packages/core/test/evaluation/loaders/config-loader.test.ts @@ -310,6 +310,7 @@ describe('parseResultsConfig', () => { sync: { auto_push: false, require_push: true, + push_conflict_policy: 'backup_and_force_push', }, }, '/tmp/.agentv/config.yaml', @@ -323,6 +324,7 @@ describe('parseResultsConfig', () => { sync: { auto_push: false, require_push: true, + push_conflict_policy: 'backup_and_force_push', }, }); }); diff --git a/packages/core/test/evaluation/results-repo.test.ts b/packages/core/test/evaluation/results-repo.test.ts index 69d5329c9..285b53408 100644 --- a/packages/core/test/evaluation/results-repo.test.ts +++ b/packages/core/test/evaluation/results-repo.test.ts @@ -21,6 +21,7 @@ import { buildWipBranchName, deleteWipBranch, directPushResults, + directPushResultsWithDetails, ensureResultsRepoClone, getResultsRepoSyncStatus, listGitRuns, @@ -109,6 +110,59 @@ function initializeRemoteStorageBranch(seedDir: string, branch = 'agentv-results return branch; } +async function createStaleResultBranchPushFixture(params: { + readonly rootDir: string; + readonly remoteDir: string; + readonly seedDir: string; + readonly cloneDir: string; + readonly storageBranch: string; +}): Promise<{ + readonly localSourceDir: string; + readonly localDestinationPath: string; + readonly remoteAdvancedCommit: string; + readonly config: ResultsConfig; +}> { + const config: ResultsConfig = { + repo: `file://${params.remoteDir}`, + path: params.cloneDir, + branch: params.storageBranch, + sync: { auto_push: false }, + }; + const firstSourceDir = path.join(params.rootDir, `${path.basename(params.cloneDir)}-local-1`); + writeRunArtifacts(firstSourceDir, 'local-stale', '2026-06-23T09:00:00.000Z'); + await directPushResults({ + config, + sourceDir: firstSourceDir, + destinationPath: path.join('local-stale', '2026-06-23T09-00-00-000Z'), + commitMessage: 'feat(results): local stale base', + }); + + git(`git switch --quiet ${params.storageBranch}`, params.seedDir); + const remoteOnlyPath = path.join( + params.seedDir, + 'runs', + 'remote-only', + '2026-06-23T09-30-00-000Z', + ); + writeRunArtifacts(remoteOnlyPath, 'remote-only', '2026-06-23T09:30:00.000Z'); + git('git add runs && git commit --quiet -m "remote result wins race"', params.seedDir); + git(`git push --quiet origin HEAD:${params.storageBranch}`, params.seedDir); + git('git switch --quiet main', params.seedDir); + + const localSourceDir = path.join(params.rootDir, `${path.basename(params.cloneDir)}-local-2`); + writeRunArtifacts(localSourceDir, 'local-conflict', '2026-06-23T10:00:00.000Z'); + + return { + localSourceDir, + localDestinationPath: path.join('local-conflict', '2026-06-23T10-00-00-000Z'), + remoteAdvancedCommit: git( + `git --git-dir "${params.remoteDir}" rev-parse ${params.storageBranch}`, + params.rootDir, + ), + config, + }; +} + function writeRunArtifacts(runDir: string, experiment: string, timestamp: string): void { mkdirSync(runDir, { recursive: true }); writeFileSync(path.join(runDir, 'index.jsonl'), '{"test_id":"alpha"}\n'); @@ -1121,6 +1175,202 @@ describe('results repo write path', () => { ); }, 20000); + it('blocks non-fast-forward direct result branch pushes by default', async () => { + const { remoteDir, seedDir } = initializeRemoteRepo(rootDir); + const storageBranch = initializeRemoteStorageBranch(seedDir, DEFAULT_RESULTS_BRANCH); + const cloneDir = path.join(rootDir, 'results-clone-push-conflict-block'); + const fixture = await createStaleResultBranchPushFixture({ + rootDir, + remoteDir, + seedDir, + cloneDir, + storageBranch, + }); + + const result = await directPushResultsWithDetails({ + config: { ...fixture.config, sync: { auto_push: true } }, + sourceDir: fixture.localSourceDir, + destinationPath: fixture.localDestinationPath, + commitMessage: 'feat(results): blocked push conflict', + }); + + expect(result).toMatchObject({ + changed: false, + blocked: true, + sync_status: 'push_conflict', + push_conflict_policy: 'block', + target_branch: DEFAULT_RESULTS_BRANCH, + remote_commit: fixture.remoteAdvancedCommit, + previous_remote_commit: fixture.remoteAdvancedCommit, + lease_commit: fixture.remoteAdvancedCommit, + }); + expect(result.local_commit).toMatch(/^[0-9a-f]{40}$/); + expect(result.block_reason).toContain('Results branch push conflict'); + expect(git(`git --git-dir "${remoteDir}" rev-parse ${storageBranch}`, rootDir)).toBe( + fixture.remoteAdvancedCommit, + ); + expect( + git(`git --git-dir "${remoteDir}" ls-tree -r --name-only ${storageBranch}`, rootDir), + ).not.toContain(`runs/${fixture.localDestinationPath}/benchmark.json`); + }, 30000); + + it('backs up the remote result branch before force-pushing with an explicit policy', async () => { + const { remoteDir, seedDir } = initializeRemoteRepo(rootDir); + const storageBranch = initializeRemoteStorageBranch(seedDir, DEFAULT_RESULTS_BRANCH); + const cloneDir = path.join(rootDir, 'results-clone-push-conflict-backup'); + const fixture = await createStaleResultBranchPushFixture({ + rootDir, + remoteDir, + seedDir, + cloneDir, + storageBranch, + }); + + const result = await directPushResultsWithDetails({ + config: { + ...fixture.config, + sync: { auto_push: true, push_conflict_policy: 'backup_and_force_push' }, + }, + sourceDir: fixture.localSourceDir, + destinationPath: fixture.localDestinationPath, + commitMessage: 'feat(results): backup force push conflict', + }); + + expect(result).toMatchObject({ + changed: true, + push_conflict_policy: 'backup_and_force_push', + target_branch: DEFAULT_RESULTS_BRANCH, + backup_commit: fixture.remoteAdvancedCommit, + previous_remote_commit: fixture.remoteAdvancedCommit, + lease_commit: fixture.remoteAdvancedCommit, + }); + expect(result.backup_ref).toMatch( + /^agentv\/backups\/\d{8}T\d{6}Z-agentv-results-v1-[0-9a-f]{7}$/, + ); + expect(result.force_pushed_commit).toMatch(/^[0-9a-f]{40}$/); + expect(git(`git --git-dir "${remoteDir}" rev-parse ${storageBranch}`, rootDir)).toBe( + result.force_pushed_commit, + ); + expect(git(`git --git-dir "${remoteDir}" rev-parse ${result.backup_ref}`, rootDir)).toBe( + fixture.remoteAdvancedCommit, + ); + expect( + git(`git --git-dir "${remoteDir}" ls-tree -r --name-only ${storageBranch}`, rootDir), + ).toContain(`runs/${fixture.localDestinationPath}/benchmark.json`); + expect( + git(`git --git-dir "${remoteDir}" ls-tree -r --name-only ${result.backup_ref}`, rootDir), + ).toContain('runs/remote-only/2026-06-23T09-30-00-000Z/benchmark.json'); + }, 30000); + + it('aborts backup-and-force-push when creating the backup ref fails', async () => { + const { remoteDir, seedDir } = initializeRemoteRepo(rootDir); + const storageBranch = initializeRemoteStorageBranch(seedDir, DEFAULT_RESULTS_BRANCH); + const cloneDir = path.join(rootDir, 'results-clone-push-conflict-backup-fails'); + const fixture = await createStaleResultBranchPushFixture({ + rootDir, + remoteDir, + seedDir, + cloneDir, + storageBranch, + }); + const hookPath = path.join(remoteDir, 'hooks', 'pre-receive'); + writeFileSync( + hookPath, + [ + '#!/usr/bin/env sh', + 'while read _old _new ref; do', + ' case "$ref" in', + ' refs/heads/agentv/backups/*) echo "reject backup ref" >&2; exit 1 ;;', + ' esac', + 'done', + ].join('\n'), + ); + chmodSync(hookPath, 0o755); + + const result = await directPushResultsWithDetails({ + config: { + ...fixture.config, + sync: { auto_push: true, push_conflict_policy: 'backup_and_force_push' }, + }, + sourceDir: fixture.localSourceDir, + destinationPath: fixture.localDestinationPath, + commitMessage: 'feat(results): backup creation fails', + }); + + expect(result).toMatchObject({ + changed: false, + blocked: true, + sync_status: 'push_conflict', + backup_commit: fixture.remoteAdvancedCommit, + previous_remote_commit: fixture.remoteAdvancedCommit, + }); + expect(result.block_reason).toContain('backup creation failed'); + expect(result.backup_ref).toMatch(/^agentv\/backups\//); + expect(git(`git --git-dir "${remoteDir}" rev-parse ${storageBranch}`, rootDir)).toBe( + fixture.remoteAdvancedCommit, + ); + expect(git(`git --git-dir "${remoteDir}" branch --list "${result.backup_ref}"`, rootDir)).toBe( + '', + ); + }, 30000); + + it('surfaces a stale lease when the target update is rejected after backup creation', async () => { + const { remoteDir, seedDir } = initializeRemoteRepo(rootDir); + const storageBranch = initializeRemoteStorageBranch(seedDir, DEFAULT_RESULTS_BRANCH); + const cloneDir = path.join(rootDir, 'results-clone-push-conflict-lease'); + const fixture = await createStaleResultBranchPushFixture({ + rootDir, + remoteDir, + seedDir, + cloneDir, + storageBranch, + }); + + const hookPath = path.join(remoteDir, 'hooks', 'pre-receive'); + writeFileSync( + hookPath, + [ + '#!/usr/bin/env sh', + 'while read _old _new ref; do', + ' case "$ref" in', + ` refs/heads/${storageBranch}) echo "stale info" >&2; exit 1 ;;`, + ' esac', + 'done', + ].join('\n'), + ); + chmodSync(hookPath, 0o755); + + const result = await directPushResultsWithDetails({ + config: { + ...fixture.config, + sync: { auto_push: true, push_conflict_policy: 'backup_and_force_push' }, + }, + sourceDir: fixture.localSourceDir, + destinationPath: fixture.localDestinationPath, + commitMessage: 'feat(results): stale lease fails', + }); + + expect(result).toMatchObject({ + changed: false, + blocked: true, + sync_status: 'push_conflict', + backup_commit: fixture.remoteAdvancedCommit, + previous_remote_commit: fixture.remoteAdvancedCommit, + lease_commit: fixture.remoteAdvancedCommit, + }); + expect(result.block_reason).toContain('force push lease failed'); + expect(result.backup_ref).toMatch(/^agentv\/backups\//); + expect(git(`git --git-dir "${remoteDir}" rev-parse ${result.backup_ref}`, rootDir)).toBe( + fixture.remoteAdvancedCommit, + ); + expect(git(`git --git-dir "${remoteDir}" rev-parse ${storageBranch}`, rootDir)).toBe( + fixture.remoteAdvancedCommit, + ); + expect( + git(`git --git-dir "${remoteDir}" ls-tree -r --name-only ${storageBranch}`, rootDir), + ).not.toContain(`runs/${fixture.localDestinationPath}/benchmark.json`); + }, 30000); + it('commits pushed runs into the configured clone with an AgentV-Run trailer', async () => { const { remoteDir } = initializeRemoteRepo(rootDir); const cloneDir = path.join(rootDir, 'results-clone'); @@ -1660,7 +1910,7 @@ describe('results repo write path', () => { ); }, 20000); - it('blocks diverged committed histories with diff summary', async () => { + it('blocks diverged committed histories as push conflicts with diff summary', async () => { const { remoteDir, seedDir } = initializeRemoteRepo(rootDir); const cloneDir = path.join(rootDir, 'results-clone'); const config = createResultsConfig(remoteDir, cloneDir); @@ -1673,17 +1923,20 @@ describe('results repo write path', () => { writeRunArtifacts(runDir, 'local-only', '2026-05-25T10:00:00.000Z'); git('git add runs && git commit --quiet -m "local result"', cloneDir); - writeFileSync(path.join(seedDir, 'REMOTE.md'), 'remote update\n'); - git('git add REMOTE.md && git commit --quiet -m "remote update"', seedDir); + const remoteRunDir = path.join(seedDir, 'runs', 'remote-only', '2026-05-25T11-00-00-000Z'); + writeRunArtifacts(remoteRunDir, 'remote-only', '2026-05-25T11:00:00.000Z'); + git('git add runs && git commit --quiet -m "remote result"', seedDir); git('git push --quiet origin main', seedDir); const status = await syncResultsRepoForProject(config); - expect(status.sync_status).toBe('diverged'); + expect(status.sync_status).toBe('push_conflict'); expect(status.blocked).toBe(true); - expect(status.block_reason).toContain('diverged'); + expect(status.block_reason).toContain('Results branch push conflict'); + expect(status.target_branch).toBe('main'); + expect(status.remote_commit).toBe(git(`git --git-dir "${remoteDir}" rev-parse main`, rootDir)); + expect(status.local_commit).toMatch(/^[0-9a-f]{40}$/); expect(status.git_status).toContain('[ahead 1, behind 1]'); - expect(status.git_diff_summary).toContain('local-only'); expect(status.git_diff_summary).toContain('benchmark.json'); }, 20000); diff --git a/packages/core/test/evaluation/validation/config-validator.test.ts b/packages/core/test/evaluation/validation/config-validator.test.ts index f11d1e856..db907ff50 100644 --- a/packages/core/test/evaluation/validation/config-validator.test.ts +++ b/packages/core/test/evaluation/validation/config-validator.test.ts @@ -242,6 +242,7 @@ describe('validateConfigFile', () => { path: repo/subdir sync: auto_push: yes + push_conflict_policy: overwrite branch_prefix: "" - not-an-object `, @@ -269,6 +270,10 @@ describe('validateConfigFile', () => { severity: 'error', location: 'projects[0].results.sync.auto_push', }), + expect.objectContaining({ + severity: 'error', + location: 'projects[0].results.sync.push_conflict_policy', + }), expect.objectContaining({ severity: 'error', location: 'projects[0].results.branch_prefix', diff --git a/packages/core/test/projects.test.ts b/packages/core/test/projects.test.ts index 1e3e2b335..91de59568 100644 --- a/packages/core/test/projects.test.ts +++ b/packages/core/test/projects.test.ts @@ -176,6 +176,7 @@ describe('projects registry', () => { path: /srv/agentv/results/results-project sync: auto_push: true + push_conflict_policy: backup_and_force_push branch_prefix: eval-results added_at: "2026-01-01T00:00:00Z" last_opened_at: "2026-01-01T00:00:00Z" @@ -188,7 +189,7 @@ describe('projects registry', () => { repoUrl: 'https://github.com/EntityProcess/results-project-runs.git', branch: 'agentv-results', path: '/srv/agentv/results/results-project', - sync: { autoPush: true }, + sync: { autoPush: true, pushConflictPolicy: 'backup_and_force_push' }, branchPrefix: 'eval-results', }); @@ -201,6 +202,7 @@ describe('projects registry', () => { expect(yamlOnDisk).toContain('branch: agentv-results'); expect(yamlOnDisk).toContain('path: /srv/agentv/results/results-project'); expect(yamlOnDisk).toContain('auto_push: true'); + expect(yamlOnDisk).toContain('push_conflict_policy: backup_and_force_push'); expect(yamlOnDisk).toContain('branch_prefix: eval-results'); expect(yamlOnDisk).not.toContain('repo_url:'); expect(yamlOnDisk).not.toContain('localPath:'); @@ -228,6 +230,7 @@ describe('projects registry', () => { sync: auto_push: false require_push: true + push_conflict_policy: block added_at: "2026-01-01T00:00:00Z" last_opened_at: "2026-01-01T00:00:00Z" `, @@ -239,7 +242,7 @@ describe('projects registry', () => { repoUrl: 'git@github.com:example/source.git', path: '.', branch: 'agentv/results/v1', - sync: { autoPush: false, requirePush: true }, + sync: { autoPush: false, requirePush: true, pushConflictPolicy: 'block' }, }); saveProjectRegistry(registry); @@ -250,6 +253,7 @@ describe('projects registry', () => { expect(yamlOnDisk).toContain('branch: agentv/results/v1'); expect(yamlOnDisk).toContain('auto_push: false'); expect(yamlOnDisk).toContain('require_push: true'); + expect(yamlOnDisk).toContain('push_conflict_policy: block'); expect(yamlOnDisk).not.toContain('repo_path:'); expect(yamlOnDisk).not.toContain('repoPath:'); expect(yamlOnDisk).not.toContain('requirePush:');