Skip to content

fix(console): keep the new version notice in view on Edit current - #3004

Merged
freeqaz merged 3 commits into
mainfrom
fix-1020b-edit
Oct 20, 2026
Merged

freeqaz merged 3 commits into
mainfrom
fix-1020b-edit

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

#2992 made Edit current Configuration open the draft with its editor in view and focused. It scrolled the editor field to the very top of the window. The draft's "New version. Changes affect future deployments…" notice and the version selector were then 370 px (1440) or 590 px (390) above, out of view. At 390 the phone top bar had scrolled away too, so nothing on screen said the reader was now on the draft and not the current version (finding 2086).

Two of the routes #2992 threaded edit through were never exercised (finding 2089). A kept draft brought back by Edit current went untested. So did the in-place tab change, which Edit current cannot reach: the button shows only on a version, and that hand-off needs the draft already selected.

Change

  • agents/detail.mjs: Edit current Configuration brings the draft's version notice to the top, with the focused editor below it.
    • If the window is too short for the notice plus the editor's label and first lines (about 160 px), it brings the editor itself to the top, as before.
    • Focus, caret and unsaved-draft handling are unchanged.
  • tests/browser/console-agent-saves.test.mjs:
    • The existing 390 px test now requires the "New version." notice at the top with the focused editor below it, all in view. This covers the first open, the open whose draft read lands late, and the return with unsaved edits.
    • New at 1440 px: Edit current returning to a kept draft (Back, then Edit current again). The restored node is the same and its editor is focused in view. This exercises the onResume hand-off.
    • New: a short window (1440×420) brings the editor to the top.

Verification

Screenshots right after Edit current Configuration, before | after:

width before after
1440 before after
390 before after

Storybook: the existing Pages/Agent detail Edit current Configuration story's description and walkthrough steps now check the notice at the top with the editor below at desktop and phone widths, plus the editor-first fallback in a short window. After internal review, the short-window test also requires the notice to be above the window.

Risks

Low. Only the scroll target changes: the notice instead of the editor, with a fallback to the editor on short windows. The 160 px allowance is a scroll heuristic, not a style value.

Edit current Configuration scrolled the editor flush to the top of the
window, with the draft's 'New version' notice 370 px (1440) or 590 px (390)
above it, so nothing on screen said this was now the draft (finding 2086).
The notice now comes to the top with the editor below it; a window too short
for both brings the editor to the top as before.

Tests also cover Edit current returning to a kept draft (finding 2089).
@freeqaz
freeqaz requested a review from a team as a code owner October 20, 2026 04:07
@clawsweeper

clawsweeper Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. labels Oct 20, 2026
@clawsweeper

clawsweeper Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR keeps the draft notice above the focused Configuration editor when the window has enough room.

Example: Choose Edit current Configuration at 390 × 844.

  • Before: The editor appears at the top while the New version notice is offscreen.
  • After: The notice and the beginning of the focused editor appear together.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The patch is focused and supported by meaningful browser coverage; static screenshots establish the improvement but leave the repository's required UI media incomplete.
Proof confidence 🦐 gold shrimp (3/6) Real behavior proof is not required for maintainer- or bot-authored pull requests.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: After choosing Edit current Configuration, users can lose the visible indication that they are editing the draft rather than the current version.
Reason: The collaborator-authored direction addresses demonstrated UI confusion with a narrow change and no new public contract.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

✅ Ready for maintainer review

Keep open: this PR addresses a concrete loss of draft context with a focused change. The previous Storybook finding is resolved, and no introduced code defect was found.

Priority: P2
Reviewed head: 1e1b420972fc8264be6d40181bc37887f1a4fe28

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: Supply the required final-UI walkthrough showing the Edit current action and resulting positioning, including the short-window fallback.
Agent review details

How this fits together

The Console Agent detail page receives version-navigation intent and Configuration data, then opens, focuses, and positions the draft editor.

flowchart TD
  A[Version Configuration tab] --> B[Edit current Configuration]
  B --> C[Existing draft navigation]
  C --> D[Open and focus editor]
  D --> E[Measure available window space]
  E --> F[Reveal notice or editor]
Loading

Technical review

Best possible solution:

Keep the existing navigation and editor owners, revealing the draft notice when space permits and preserving editor-first positioning in short windows.

Do we have a high-confidence way to reproduce the issue?

Current-main source scrolls the editor above the draft notice, consistent with the supplied desktop and phone before screenshots; no target code was executed during this review.

Is this the best way to solve the issue?

The geometry check is a small correction in the existing scroll owner and preserves editor visibility when the notice and editor cannot fit together.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 7f44e5f4c91e.

Provenance checked

  • Editor-opening scroll target changes intended behavior with a stated reason (33132e1: Open and focus the draft editor immediately without requiring a second Edit Configuration click.)
  • Browser positioning assertions and Storybook walkthrough changes intended behavior with a stated reason (33132e1: Protect immediate editor visibility across initial opening, delayed reads, and retained edits; the revised expectations additionally preserve visible draft context.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Likely related people:

  • Free Wortley: Raw commit 33132e1 adds apps/controller/src/console/agents/detail.mjs:1692 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 33132e12df10; files: apps/controller/src/console/agents/detail.mjs)

Labels

Label changes:

  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: Losing draft context creates avoidable editing confusion, but the editor remains usable and the notice is reachable by scrolling.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Complete the native GitHub UI media attachments and walkthrough required by AGENTS.md.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (1 earlier review cycle)
  • reviewed 2026-10-20T04:22:29.805Z sha d4db370 :: needs changes before merge. :: [P3] Update the existing Storybook scenario for the new scroll behavior

Reviewed October 20, 2026, 12:45 AM ET / 04:45 UTC (Revision 2).

The Edit current Configuration story's description and steps now check the
'New version' notice at the top with the editor below it at desktop and
phone widths, and the editor-first fallback in a short window. The short
window test also requires the notice above the window, so it proves the
fallback rather than any scroll that leaves the editor near the top.
@freeqaz

freeqaz commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. labels Oct 20, 2026
@freeqaz
freeqaz merged commit e615044 into main Oct 20, 2026
68 of 69 checks passed
@freeqaz
freeqaz deleted the fix-1020b-edit branch October 20, 2026 04:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant