feat(frontend): playground save asks to reload when the config changed underneath

[mmabrouk]

Context

A browser tab holding an old draft could silently overwrite a newer config on save: the frontend sent no concurrency guard anywhere. Mahmoud hit it live during QA (a 2-hour-old tab wiped a skill an agent had just committed).

Changes

The playground's commit flow now sends the revision the page loaded as base_revision_id (the backend check already existed, flag-independent). When the head moved, the save answers with the message "This agent changed since you opened it. Reload before saving." and a Reload button, rendered inside the same error alert every other commit error uses. The 409 is parsed defensively across both the current and the in-flight flattened error body shapes. Slug-conflict handling is untouched (the new path keys on an error code, never on the 409 status).

This is Option A of the agreed two-step: minimal guard now. Option B (the same guard on every entity plus a reconcile screen) is a recorded follow-up.

Tests

• 8 tests pin that the commit payload carries the loaded revision id.
• 4 tests drive the real commit atom against a seeded store with only the network mocked: a moved head produces the message and the reload affordance and does not commit; a slug conflict still takes its old path.
• Suites: agenta-entities 951 tests, agenta-entity-ui 312 tests, typecheck clean in all touched packages plus web/oss.

What to QA

• Open an agent in the playground, keep the tab open; commit a change to the same agent from a second tab; save in the first tab. Expect the reload message and button, not a silent save.
• Regression: rename flows and slug conflicts behave as before; a normal save with no interleaving change commits as before.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
agenta-documentation	Ready	Preview	Aug 6, 2026 7:35pm

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 366577bc-c9de-4054-a9ad-937925db0e2b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[ardaerzin]

Requesting changes. The problem is real and worth fixing, and the test discipline here is good — the atom test mocks only the network boundary and drives the real store. But as it stands this cannot ship: on this PR's base branch the guard does nothing, and the two behaviours it does add (a false conflict on a legitimate flow, and a full page reload as the recovery) are worse than the status quo for the users who hit them.

Please do not merge until we have re-reviewed. Three blocking items, then some non-blocking notes and a concrete alternative to the reload.

---

B1 — On this base branch the guard is inert, and the PR's own QA script cannot pass

The description says "the backend check already existed, flag-independent". It does not exist on this PR's base.

• base_revision_id, RevisionConflictError and the 409 are not on release/v0.109.0, and not on main. They live in the unmerged draft stack agent-config-editing-s1a…s7e (the lock lane is draft #5750, itself based on agent-config-editing-s6).
• WorkflowRevisionCommit inherits RevisionCommit(Slug, Header, Metadata) (api/oss/src/core/git/dtos.py:96). Nothing in that chain sets extra= — the only model_config anywhere in the ancestry is AliasConfig's populate_by_name/from_attributes in sdks/python/agenta/sdk/models/shared.py:186. So pydantic's default extra="ignore" applies and the server silently drops the field.

Net effect if this merges as-is: nothing breaks, and nothing is guarded. The overwrite Mahmoud hit in QA still happens, silently, exactly as before. The "What to QA" steps in the description would fail on this base — a second tab's save still wins with no message — which is a confusing signal to hand to whoever runs them.

What we want instead: stack this on the API lane that introduces base_revision_id and set the PR base to it, so the diff and the QA script describe the same system. If there is a reason to land the FE plumbing ahead of the API, say so explicitly in the description and put it behind something that makes the inertness visible, rather than describing a guard that isn't wired.

B2 — A legitimate flow now fails with a message that is wrong and a button that doesn't help

baseRevisionId: revisionId is sent unconditionally (commit.ts:329), where revisionId is whatever revision is currently displayed — not necessarily the head.

Selecting an older revision in SelectVariant, editing it, and saving is a supported flow today: it's how you restore a previous configuration. Nothing gates it. The Save button's disabled is !variantId || (!isEphemeral && !hasChanges) (CommitVariantChangesButton/index.tsx:27) — purely a dirty check. isLatestRevision in PlaygroundVariantConfigHeader.tsx:136 only drives a display tag.

Once the API lane lands, every save from a non-head revision returns 409 and the user is told "This agent changed since you opened it. Reload before saving." Nobody changed anything. Reloading returns them to the same non-head revision, so the button cannot fix it either. The guard should apply to "I loaded the head and the head moved", not to "I deliberately opened an old version" — those need to be distinguished before the base id is attached, or the copy and the affordance need to cover both cases.

B3 — window.location.reload() is the wrong recovery, and it probably loops

Two problems, one of them functional.

It likely doesn't resolve the conflict. The playground encodes drafts into the URL hash: buildEncodedSnapshot reports hasDrafts from snapshot.drafts (urlSnapshotController.ts:195) and the OSS URL adapter writes it as #snapshot=… (web/oss/src/state/url/playground.ts:308). window.location.reload() re-navigates to the same URL — same stale revision id in the query param, same draft patch in the hash. The draft rehydrates onto the same stale base and the next save hits the same 409. Worth confirming with a live repro, but the mechanism is all in the code.

Even if it worked, it is the most destructive option available. A full Next boot, and it discards unsaved run/chat state alongside the config draft — the user's work is the thing we were trying to protect.

The pattern we'd rather see: rebase the draft, don't reload the document

The repo already has the primitive. snapshotAdapter.ts:122 gives you both halves:

• buildDraftPatch(revId) → a shallow diff of the user's edits against that revision's server baseline (all data keys, with parameters diffed at its own level).
• applyDraftPatch(otherRevId, patch) → mergeDataPatch re-applies that patch onto another revision's server data.

That is a rebase, and it is already load-bearing for URL snapshots and cross-navigation hydration. The other half — moving the playground onto a different revision id without a reload — is what the successful commit path already does via switchEntity (workflowEntityBridge.ts:251), which handles selection, chat history, URL sync and drawer state.

So the recovery is roughly:

// on 409, with conflict.currentRevisionId in hand
const adapter = snapshotAdapterRegistry.get("workflow")
const {hasDraft, patch} = adapter.buildDraftPatch(staleRevId)     // my edits vs my old base
const head = await fetchWorkflowRevisionById(projectId, headRevId)
primeWorkflowRevisionDetailCacheImperative(head)                   // same prime the success path does
if (hasDraft && patch) adapter.applyDraftPatch(headRevId, patch)   // same edits, new base
set(playgroundController.actions.switchEntity, {currentEntityId: staleRevId, newEntityId: headRevId})
set(discardWorkflowDraftAtom, staleRevId)

The user stays on the page, keeps every edit, and the retry carries base_revision_id = head, so it commits. This is also what the backend contract asks for: to_detail() sets retryable: true and a next_step of "read the new revision, re-anchor your edits, and send the commit again with the new base_revision_id". The PR reads neither.

But don't auto-merge blindly

mergeDataPatch is shallow at data.* and data.parameters.*. If both writers touched parameters.agent, the rebase replaces theirs wholesale — the same silent overwrite this PR exists to prevent, one level down. Gate it with the diff helper that's already there, both sides computed client-side once the head is fetched:

• computeShallowDiff(myDraft, oldBase) vs computeShallowDiff(newHead, oldBase) (runnable/snapshotDiff.ts:53)
• disjoint key sets → rebase and retry silently; a toast naming the revision that landed is enough
• overlapping keys → keep the modal open, name the colliding fields, offer mine/theirs. The commit modal already renders change summaries (AgentChangesSummary), so there's a surface to reuse rather than invent

That keeps this Option-A-sized: two calls to an existing diff helper, one existing adapter, one existing switchEntity. No reload, no lost work, and the sub-key overwrite is surfaced instead of buried.

---

Non-blocking notes

1. The envelope the tests pin is not the one the server sends. RevisionConflictError.to_detail() returns the ids flat on detail: {code, message, next_step, base_revision_id, current_revision_id, retryable} — there is no nested details object. The parser still returns the right answer (we ran it against the real body; the ?? detailRecord fallback at revisionConflict.ts:48 catches it), so this is not a bug — but the shape documented as "canonical" doesn't exist, and the only shape that will ever arrive in production is the one no test covers. Please add that case and correct the module doc.

2. retryable is contradicted. The server says retryable: true and tells the caller how; the code comments say "must not retry the same commit". Both can't be the guidance. If we adopt the rebase above, the server's reading is the right one.

3. The parsed conflict is dead data. conflict.currentRevisionId is parsed, attached, and asserted in tests, but nothing consumes it. It's exactly the id the rebase needs — or, minimally, a "see what changed" deep link.

4. The error side-channel is untyped across three packages. error.code / error.action are cast at each call site (playgroundController.ts:1804, EntityCommitContent.tsx:105, and the OSS modal). WorkflowRevisionCommitError is exported but WorkflowCommitOutcome.error is still Error, so no consumer benefits from it. Widening the outcome type removes all three casts.

5. Minor: the early return in CommitVariantChangesModal duplicates the generic branch except for errorAction — one object with a conditional field reads better. And the description says "8 tests pin that the commit payload carries the loaded revision id"; it's 2 (the other 6 are parser tests).

---

What we'd approve

• The guard sits on a base where the server honours it, with a QA script that can actually pass.
• The base id is attached only when the loaded revision is the head, or a non-head save gets copy that matches what happened.
• Recovery rebases the draft onto the new head in place, with the overlap check deciding between a silent retry and an explicit choice. No window.location.reload().
• A test on the real 409 body.