YAML review: say when the resource changed after review by nadaverell · Pull Request #1962 · skyhook-io/radar

nadaverell · 2026-10-02T14:08:01Z

When you apply a reviewed YAML edit and someone else changed the resource after your review, the apply is refused with a 409. Radar already re-ran the review against the latest version. It didn't say so: the diff quietly changed under you, and the raw "resource changed after review" error sat beside it.

Now the review says it plainly: "This resource changed after your review. The diff above now shows its latest version. Check it, then apply again." The raw error is hidden while that notice shows, so there is one message, not two.

The notice shows only when the refreshed review is the same resource, on the same cluster, at a newer version. In every other case the attempt's own error shows:

a deleted resource;
a cluster switch;
a retry that fails for a different reason and can't refresh.

A refresh that lands after you went back, cancelled, or prepared another review is dropped, so it can never restore an earlier draft for the next apply. Only the server's changed-after-review refusal gives way to the notice; a timeout or admission denial still shows beside it.

Verification

Live, on a kind cluster:
1. Open a ConfigMap, edit greeting, and review.
2. Change a different key with kubectl.
3. Apply. The PUT returns 409, the preview reruns (200), and the notice shows over the refreshed diff.
4. Apply again. The PUT returns 200, and the ConfigMap keeps both the editor's greeting and kubectl's other.
Tests:
- EditableYamlView.test.tsx covers the flow end to end:
  - a conflict, then the notice;
  - a second failure with no refresh, which shows its own error and no notice;
  - a failure whose refresh finds the same version, which shows no notice;
  - a deleted resource, a switched cluster, and an earlier attempt's late refresh, none of which shows the notice;
  - a late refresh after going back to a newer review, where the next apply still sends the newer version;
  - an unrelated apply error, which stays visible beside the notice.
- Each guard was removed in turn, and its test failed.
- YamlReview.test.tsx covers the notice and the hidden raw error.
client.updateResource.test.tsx runs the real query client: a reviewed apply raises no toast, and a save that isn't from a review does.
make tsc passes. k8s-ui: 4136 tests pass; web: 1845 tests pass.

A reviewed apply's failure no longer also raises the global "Failed to update resource" toast: the review shows it, so a toast would repeat it. Saves that don't come from a review still toast.

An apply refused because the resource changed after review already re-runs the review against the latest version, but the screen kept only the red error, which reads as start over. When the refreshed review carries a newer resource version, a notice now says the diff shows the latest version and to apply again.

When the notice says the resource changed after review, the raw API message under it repeated the same thing; the error toast still carries it.

A retry that fails for another reason, and can't refresh the review, shows its own error instead of the previous attempt's notice.

qodo-free-for-open-source-projects · 2026-10-02T14:10:33Z

PR Summary by Qodo

YAML review: notify when resource changed after review

✨ Enhancement 🧪 Tests 🕐 10-20 Minutes

AI Description

• Shows a clear notice when a refused apply's re-run review detects the resource changed since the
 user's review.
• Hides the raw 409 error message while the new notice is shown, avoiding duplicate messaging.
• Scopes the notice to the attempt that detected the change; subsequent retries show their own
 errors.
• Adds tests covering the conflict/notice flow, retries without refresh, and refreshes matching the
 same version.

Diagram

sequenceDiagram
    participant User
    participant EditableYamlView
    participant onSave as onSave API
    participant onPreview as onPreview API
    participant YamlReview

    User->>EditableYamlView: Apply reviewed changes
    EditableYamlView->>onSave: apply edit
    onSave-->>EditableYamlView: 409 conflict
    EditableYamlView->>onPreview: refresh review
    onPreview-->>EditableYamlView: latest version diff
    EditableYamlView->>EditableYamlView: compare resourceVersion
    EditableYamlView->>YamlReview: changedSinceReview=true
    YamlReview-->>User: show notice, hide raw error

High-Level Assessment

The PR's approach—deriving the notice from a resourceVersion comparison during the existing refresh-on-conflict flow, scoped per attempt via a reset flag—is minimal and well-tested. No alternative architecture (e.g., a separate conflict-state machine) would meaningfully improve this well-bounded UI fix.

Files changed (4) +214 / -1

Enhancement (2) +24 / -1

EditableYamlView.tsxTrack and reset changedSinceReview flag per apply attempt +9/-0

Track and reset changedSinceReview flag per apply attempt
• Adds a changedSinceReview flag to the preview state, sets it when a refreshed review's resourceVersion differs from the prior one, resets it at the start of each apply attempt, and passes it down to YamlReview.
packages/k8s-ui/src/components/shared/EditableYamlView.tsx

YamlReview.tsxRender a warning banner instead of raw error on resource conflict +15/-1

Render a warning banner instead of raw error on resource conflict
• Adds a changedSinceReview prop; when set, renders an AlertBanner explaining the resource changed and hides the raw applyError message to avoid duplicate messaging.
packages/k8s-ui/src/components/ui/YamlReview.tsx

Tests (2) +190 / -0

EditableYamlView.test.tsxAdd end-to-end tests for the conflict notice lifecycle +156/-0

Add end-to-end tests for the conflict notice lifecycle
• New test file verifying the notice appears only for the attempt that detects a resource version change, disappears on an unrelated retry failure, and stays hidden when the refresh finds the same version.
packages/k8s-ui/src/components/shared/EditableYamlView.test.tsx

YamlReview.test.tsxAdd tests for YamlReview's conflict notice rendering +34/-0

Add tests for YamlReview's conflict notice rendering
• New test file asserting the notice text renders and suppresses the raw apply error when changedSinceReview is true, and that nothing extra shows otherwise.
packages/k8s-ui/src/components/ui/YamlReview.test.tsx

qodo-free-for-open-source-projects · 2026-10-02T14:11:52Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Deleted resources get a false notice~~ ✓ Resolved

Description

handleApplyReviewed treats a missing version in a rejected refresh as a version change and sets
changedSinceReview to true. If the resource was deleted after review, the refresh returns a
rejected document without a version, so the notice claims the diff shows the latest resource while
YamlReview hides the apply error.

Code

packages/k8s-ui/src/components/shared/EditableYamlView.tsx[R348-350]

+            changedSinceReview:
+              refreshed.documents[0]?.reviewedResourceVersion !==
+              preview.documents[0]?.reviewedResourceVersion,

Evidence
An accepted update preview stores the live version, but a failed live read makes the refresh return
a rejected document instead. The new comparison evaluates that document's absent version against the
original version, and the new rendering condition suppresses the apply error.
pkg/k8score/workload.go[111-133]
internal/server/yaml_preview.go[160-177]
packages/k8s-ui/src/components/shared/EditableYamlView.tsx[328-351]
packages/k8s-ui/src/components/ui/YamlReview.tsx[359-368]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A rejected refresh has no reviewed resource version. Comparing it with the previous version produces a false changed-resource notice and hides the apply error.
## Fix Focus Areas
- packages/k8s-ui/src/components/shared/EditableYamlView.tsx[345-351]
- packages/k8s-ui/src/components/shared/EditableYamlView.test.tsx[141-156]
## Recommended Fix
Set the notice only when the refreshed document is accepted and both reviews have resource versions that differ. Add a test where the resource disappears and the refresh returns a rejected document.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Cluster switches hide the real apply error~~ ✓ Resolved

Description

handleApplyReviewed refreshes after every failed save and identifies a resource change solely by
comparing versions, without checking why the save failed. If the server refuses an apply because the
reviewed cluster context changed, a successful preview in the new context with a different version
shows the resource-changed notice and hides the cluster-context error.

Code

packages/k8s-ui/src/components/shared/EditableYamlView.tsx[R348-350]

+            changedSinceReview:
+              refreshed.documents[0]?.reviewedResourceVersion !==
+              preview.documents[0]?.reviewedResourceVersion,

Evidence
The update handler returns a distinct conflict error when the reviewed context differs from the
current context, before attempting the resource update. The catch path does not distinguish that
error from a version conflict; it refreshes and sets the new flag from versions alone, after which
YamlReview suppresses the original error.
internal/server/server.go[4291-4301]
packages/k8s-ui/src/components/shared/EditableYamlView.tsx[317-350]
packages/k8s-ui/src/components/ui/YamlReview.tsx[359-368]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A changed version in a refreshed preview does not prove that the failed apply was refused for a resource-version conflict. A cluster-context refusal can therefore be replaced by an incorrect notice.
## Fix Focus Areas
- packages/k8s-ui/src/components/shared/EditableYamlView.tsx[328-351]
- packages/k8s-ui/src/components/ui/YamlReview.tsx[359-368]
## Recommended Fix
Preserve the failure reason from the save operation and show the changed-resource notice only for a verified resource-version conflict. Keep the original apply error visible for cluster-context failures, and cover that case with a test.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~Rapid retries restore an outdated notice~~ ✓ Resolved

Description

handleApplyReviewed lets each failed attempt set changedSinceReview when its asynchronous
refresh completes, without checking whether a newer attempt has started. The Apply button can be
used while the first refresh is pending; if a second attempt fails and its refresh is unavailable,
the first refresh can then finish and display its notice for the second attempt.

Code

packages/k8s-ui/src/components/shared/EditableYamlView.tsx[R348-350]

+            changedSinceReview:
+              refreshed.documents[0]?.reviewedResourceVersion !==
+              preview.documents[0]?.reviewedResourceVersion,

Evidence
The failed-save path awaits onPreview before writing the new notice state. The review passes only
isSaving as isApplying, and its Apply button disables only while isApplying is true, leaving a
window for a second attempt after the save fails but before its refresh finishes.
packages/k8s-ui/src/components/shared/EditableYamlView.tsx[310-357]
packages/k8s-ui/src/components/shared/EditableYamlView.tsx[371-381]
packages/k8s-ui/src/components/ui/YamlReview.tsx[417-430]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A refresh from an earlier failed apply can complete after a newer retry and restore an outdated changed-resource notice.
## Fix Focus Areas
- packages/k8s-ui/src/components/shared/EditableYamlView.tsx[310-357]
- packages/k8s-ui/src/components/shared/EditableYamlView.tsx[371-381]
- packages/k8s-ui/src/components/shared/EditableYamlView.test.tsx[105-139]
## Recommended Fix
Track the current apply attempt and ignore refresh results from superseded attempts, or prevent another apply until the failed attempt's refresh finishes. Test overlapping retries with deferred refresh promises.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

A deleted resource refreshes with no version, and a cluster switch is its own error; neither shows the changed-after-review notice. A slower refresh from an earlier attempt no longer lands over a newer one.

Going back, starting over, or preparing another review now invalidates a refresh still in flight, so it can't restore an earlier draft for the next apply. Only the server's changed-after-review refusal gives way to the notice; a timeout or admission denial still shows beside it.

A reviewed apply's failure shows in the review (the changed-after-review notice, or the error itself), so the global 'Failed to update resource' toast repeated it. Errors from a reviewed apply are marked as shown inline and the mutation handler skips them; other saves still toast.

nadaverell added 3 commits October 2, 2026 16:49

YAML review: one message for a refused, refreshed apply

9917b9b

When the notice says the resource changed after review, the raw API message under it repeated the same thing; the error toast still carries it.

YAML review: the changed-after-review notice covers one attempt

d64aec0

A retry that fails for another reason, and can't refresh the review, shows its own error instead of the previous attempt's notice.

nadaverell requested a review from hisco as a code owner October 2, 2026 14:08

qodo-free-for-open-source-projects Bot reviewed Oct 2, 2026

View reviewed changes

Comment thread packages/k8s-ui/src/components/shared/EditableYamlView.tsx Outdated

Comment thread packages/k8s-ui/src/components/shared/EditableYamlView.tsx Outdated

Comment thread packages/k8s-ui/src/components/shared/EditableYamlView.tsx Outdated

nadaverell added 3 commits October 2, 2026 17:19

YAML review: only a newer version of the same resource is a change

96279c2

A deleted resource refreshes with no version, and a cluster switch is its own error; neither shows the changed-after-review notice. A slower refresh from an earlier attempt no longer lands over a newer one.

nadaverell merged commit a6582e7 into main Oct 3, 2026
10 checks passed

nadaverell deleted the feat/yaml-review-conflict-notice branch October 3, 2026 22:27

Navigation Menu

Repository navigation

YAML review: say when the resource changed after review - #1962

YAML review: say when the resource changed after review#1962
nadaverell merged 6 commits into
mainfrom
feat/yaml-review-conflict-notice

nadaverell commented Oct 2, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 2, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

nadaverell commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Verification

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 2, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

nadaverell commented Oct 2, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Oct 2, 2026 •

edited

Loading