Skip to content

fix(console): keep the Create draft after a failed Create and across a failed Namespace read - #2886

Merged
freeqaz merged 14 commits into
mainfrom
ux-fixes-122a
Oct 19, 2026
Merged

freeqaz merged 14 commits into
mainfrom
ux-fixes-122a

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 18, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Follow-up to bughunt-122 on #2870 (UX plan A3, "keep every unsaved draft until an explicit discard"):

  • Finding 1934: a failed Create dropped the draft. The submit handler stops draft capture before its first write and never restarted it, so after a refusal such as the duplicate-name 409 the reload warning stopped and leaving and coming back showed the Preset chooser.
  • Finding 1935: one failed or aborted session or /namespaces read called clearDrafts() (loadPage and revalidateMountedAgent). That lost a Configuration edit and a kept Create draft. The owner check after the retry also cleared them, because the failed check had cleared the session.
  • Finding 1937: a pending save held ← Agents and the Namespace selector, but not the sidebar links or Back to another page.
  • Finding 1938: the "does not prompt" and restore halves of the unsaved rules could be removed with every test staying green.
  • Also seen while capturing media: the Create Agent chooser showed a stray null after "← Agents" in every ready Namespace. replaceChildren printed the null child added by fix(console): first-run and not-ready Namespace guidance #2867 as text (finding 1943).

Change

  • agents/create.mjs: after a known refusal, the form becomes a draft again (keepDraftAfterRefusal). The draft also carries the Secret, Credential Source and Configuration that attempt saved, so a restored form shows "Secret saved … Configuration saved …" and the retry reuses them. A password Preset's saved Secret becomes the chosen model credential, because the password field is never kept. Uncertain outcomes, pending credential registration, admitted provisioning jobs and a saved Agent keep today's behaviour: there the form stays uncaptured.
  • console.mjs and drafts.mjs: a failed session or Namespace check no longer clears drafts. Drafts are dropped only when the signed-in account differs from the one that wrote them, or when a successful /namespaces read no longer lists their Namespace (drafts.retainNamespaces).
  • shell.mjs, console.mjs and agents/detail.mjs: while a draft write is in flight, the sidebar links and Back/Forward now stay on the page, restore its URL and show the same note as ← Agents. On a phone, the refusal closes the navigation drawer so the note is visible. An uncertain or stale editor is still kept as a draft, so the sidebar and Back may leave it, as an existing test requires. ← Agents and the Namespace selector keep their wider block.
  • agents/create.mjs: the chooser no longer passes null to replaceChildren.
  • Storybook: Flows → Keep an Agent draft refuses its first Create once, so a reader can leave and return to the refused form. Pages/Create Agent → Provisioning conflict fails the next Namespace check once and shows the draft surviving it. Pages/Agent detail → Keep Configuration edits delays its save by 3 seconds, so a sidebar link visibly holds during the save.
  • Docs: docs/flows/agent-presets.md and docs/reference/console/create-and-deploy.md name the refusals that keep a draft and the ones that do not (repository policy, a saved Agent, an admitted provisioning job, a pending credential registration). docs/contributing/console-storybook.md mentions the refused Create. All three stay under the 2500-word cap (about 2491, 2498, 2498).
  • Not changed: sign-out and session expiry still clear drafts. That is finding 1936, an owner decision.

Verification

New file tests/browser/console-drafts-kept.test.mjs, mapped to checks-browser-2.json (Suite Audit: no issues):

  • With main's console code, the 4 behaviour tests fail: the refused Create, the failed Namespace read, the failed recheck on the Agent page, and the pending save holding the sidebar and Back. With this change, all 9 pass.
  • The 4 rule tests pin finding 1938, and a 9th covers a refusal between the Secret and Configuration writes. Each mutant bughunt-122 listed now fails a test: unsavedWhen("create") removed, shared-ui unsavedWhen("channels") removed, list.mjs search unsavedWhen removed, detail.mjs carried-Secrets unsavedWhen removed, Preset touched = false, and toggleModel markTouched removed.
  • 17 more mutants of this change each fail a test: the Back guard, the sidebar refusal, the pending-only split both ways, the drawer close, retainNamespaces, the refusal re-capture, the saved Configuration and Secret carry, the saved status, the owner check, both removed clearDrafts() calls, and the null child. That makes 23 mutants in all, including the Back-history entry and the locked Secret picker.
  • 20/20 at 2 CPUs (taskset -c 8,9, --test-concurrency=1): 20/20 pass on the final test file (9 tests), again after merging feat(console): tabs with tablist semantics #2878 (tabs now use the tab role).
  • After merging feat(console): tabs with tablist semantics #2878: console-drafts-kept, console-agent-detail, console-agent-draft-guards, console-retained-revalidation, console, console-agent-saves and console-agent-channels pass locally (242/242).
  • Storybook: I walked the three changed stories against the prepared fixture assets in Chromium. The refused form came back with its name, model and Secret, after a failed Namespace check too. During the delayed save, a sidebar link stayed on the page and showed "Wait for Configuration save to finish before leaving the editor."
  • Related files green locally after merging main earlier (450/450): console-agent-draft-guards, console-agent-presets, console-retained-revalidation, console, console-agents, console-agents-2, console-agent-saves, console-agent-save-refusals, console-agent-detail, console-agent-channels, console-agent-detail-2, console-agent-repositories, console-dialogs, console-agent-plugin-refusals, console-create-agent-provisioning-400.
  • Prettier and ESLint are clean on the touched files.

Media (main → this branch):

main this branch
Return to Create Agent after a refused Create, 1440 before after
Same, 390 before after
Sidebar "Agents" while a Slack disable is saving, 1440 before after
Same, 390 (drawer) before after

Media commit: 9f9a46a1cb1f45daf13d30eaa77920d5823416ac. The main screenshot of the chooser also shows the stray null after "← Agents" (finding 1943).

Open PRs

I 3-way merged this head with every open PR that touches these files and compared the result with the same merge from origin/main. It adds no new conflict hunk:

PR conflict hunks vs main vs this head
#2860 0 0
#2833 0 0
#2599 33 33
#2596 0 0
#2524 4 4
#2518 11 11
#2505 2 2
#2502 6 6
#2445 4 4
#2409 15 15
#2268 18 18
#2258 3 3
#2255 5 5
#2253 3 3
#1619 17 17
#906 4 4
#878 2 2
#115 9 9

Risks

Low. These are console-only behaviour changes.

  • A restored Create draft resumes saved resources only after a known refusal. Every uncertain path stays uncaptured, so a restore cannot resend an uncertain write.
  • Drafts now survive a failed admission read. They still render only after a later successful read admits their Namespace, and an account change still clears them.
  • Back/Forward during a write in flight re-adds the page as a new history entry, so the page the reader tried to reach stays reachable once the write ends. Forward entries past it are dropped.
  • A refusal outside the editor tabs (deploy or Credentials saving) is silent, as ← Agents already is. That is unchanged here.
  • Not covered by a new test: the repository-policy and provisioning-with-Agent exclusions. They keep today's uncaptured behaviour.

Internal review: changes needed at first, with 2 medium findings: a restored password-Preset picker stayed editable, and the repository-policy lock was lost. Both are fixed, along with the provisioning exclusion, the reload warning for saved resources, Back keeping the target entry, and the comment and test nits. I kept the history guard after the tab handler, so same-Agent tab changes keep their existing refusal.

…a failed Namespace read

A refused Create now registers its form as a draft again, carrying the
Secret, Credential Source and Configuration the attempt saved, so the
reload warning and a return keep the form and the retry reuses them
(finding 1934). Uncertain outcomes, pending registrations and admitted
provisioning jobs keep today's behaviour.

A failed or aborted session or /namespaces read no longer clears drafts;
the owner check alone drops them on an account change, and a successful
read that no longer lists a Namespace drops that Namespace's drafts
(finding 1935).

The sidebar links and Back/Forward now wait for a draft write in flight,
as ← Agents does; an uncertain or stale editor stays a kept draft they
may leave (finding 1937).

New browser tests cover each case and pin the rules that keep untouched
forms, unchanged drawers, search text and carried Secret choices from
asking before a reload (finding 1938).
…e draft

Keeps the captured model credential lines unchanged, so the open Secret
access PR (#2255) gains no conflict hunk.
…gent chooser

replaceChildren prints a null child as text; the not-ready status line
from #2867 passed null for a ready Namespace.
…Back hold

- A restored password-Preset form keeps the saved Secret fixed, since
  the retry grants the Agent access to that Secret.
- A repository policy refusal and a provisioning job that names its
  Agent keep the form uncaptured, as other uncertain states do.
- Saved resources count as unsaved work for the reload warning.
- A refused Back or Forward re-adds the shown page as a new history
  entry, so the page the reader tried to reach stays for a later Back.
- Owner-check comment states the session rule.
@freeqaz
freeqaz requested a review from a team as a code owner October 18, 2026 23:29
@clawsweeper

clawsweeper Bot commented Oct 18, 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.

@freeqaz
freeqaz temporarily deployed to integration-qa-pr October 18, 2026 23:38 — with GitHub Actions Inactive
@freeqaz
freeqaz temporarily deployed to integration-qa-pr October 18, 2026 23:38 — with GitHub Actions Inactive
@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. clawsweeper:intake:needs-repro 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. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 18, 2026
@clawsweeper clawsweeper Bot added the status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. label Oct 18, 2026
@clawsweeper

clawsweeper Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge.

What this changes

This PR retains refused Create drafts and saved resource references, preserves drafts through failed admission reads, and blocks sidebar/history navigation during pending saves.

Example: Create an Agent named Existing Agent when that name already exists, then leave and return.

  • Before: The 409 refusal stops draft capture, so returning opens the Preset chooser.
  • After: The form returns with Existing Agent and reuses the Secret and Configuration already saved.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Useful, focused recovery work with meaningful browser coverage and inspected screenshots; required Storybook coverage and visual walkthrough evidence remain incomplete.
Proof confidence 🦐 gold shrimp (3/6) Real behavior proof is not required for maintainer- or bot-authored pull requests.
Patch quality 🦐 gold shrimp (3/6) 2 actionable review findings remain.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: A refused Create or failed access check can erase unfinished work, and sidebar/history navigation can abandon a pending save.
Reason: The maintainer-authored repair implements the existing retention direction and protects concrete user work without adding configuration or public contracts.

Merge readiness

⛔ Needs changes before merge - 2 items remain

Keep this PR open: it addresses source-verifiable draft loss on main and follows the existing retention direction, but the required Storybook coverage needs updating.

Priority: P2
Reviewed head: 39d5d9ac0414bcb5309da63e3726b37072505feb

Before merge

  • [P2] Extend Storybook to cover the changed draft failure states (P2) - This adds refused-Create recovery and changes admission-failure and pending-save navigation behavior, but no matching Storybook scenario or workflow instruction changes. AGENTS.md:444–450 explicitly requires those updates for Console UI changes. The existing createExitFlow only covers leaving an ordinary unsaved form and cancelling it. Extend that flow and the relevant Agent-detail scenarios under scripts/console-storybook/ to expose the changed failure states and navigation controls.
  • [P3] Qualify which refused saves survive navigation (P3) - The new reference promises that a refused save stays a draft, but a known Agent-creation refusal with repository bindings calls requireRepositoryReload(), and keepDraftAfterRefusal() explicitly excludes that repository-scoped state. Leaving and returning therefore does not retain that form. Narrow this statement and the corresponding agent-presets flow sentence to eligible known refusals, preserving the documented recovery boundary for excluded states.

Findings

  • [P2] [P2] Extend Storybook to cover the changed draft failure states — apps/controller/src/console/agents/create.mjs:1593
  • [P3] [P3] Qualify which refused saves survive navigation — docs/reference/console/create-and-deploy.md:85-86

Tests

  • Missing end-to-end proof: The supplied screenshots do not demonstrate the full refusal/return, failed-admission recovery, or pending-save navigation sequences required by the repository's UI walkthrough policy.
Agent review details

How this fits together

The Console captures document-local editor state around authenticated API operations and restores it after session and Namespace admission checks.

flowchart TB
  A[Console form] --> B[Create API requests]
  B -->|Known refusal| C[Document-local draft store]
  A -->|Navigation| C
  C --> D[Session and Namespace checks]
  D -->|Admitted| E[Restored form]
  E --> B
Loading

Technical review

Best possible solution:

Keep recovery in the existing draft store, add the required Storybook failure states, and document the retention exceptions.

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

Main shows capture being cleared before Create writes and drafts being cleared after failed admission reads; the PR reports failing-base/passing-head browser regressions, which were not rerun during this read-only review.

Is this the best way to solve the issue?

The implementation extends the existing draft and navigation owners while preserving uncertain-outcome handling, avoiding a second persistence or recovery mechanism.

Full review comments:

  • [P2] [P2] Extend Storybook to cover the changed draft failure states — apps/controller/src/console/agents/create.mjs:1593
    This adds refused-Create recovery and changes admission-failure and pending-save navigation behavior, but no matching Storybook scenario or workflow instruction changes. AGENTS.md:444–450 explicitly requires those updates for Console UI changes. The existing createExitFlow only covers leaving an ordinary unsaved form and cancelling it. Extend that flow and the relevant Agent-detail scenarios under scripts/console-storybook/ to expose the changed failure states and navigation controls.
    Confidence: 0.99
  • [P3] [P3] Qualify which refused saves survive navigation — docs/reference/console/create-and-deploy.md:85-86
    The new reference promises that a refused save stays a draft, but a known Agent-creation refusal with repository bindings calls requireRepositoryReload(), and keepDraftAfterRefusal() explicitly excludes that repository-scoped state. Leaving and returning therefore does not retain that form. Narrow this statement and the corresponding agent-presets flow sentence to eligible known refusals, preserving the documented recovery boundary for excluded states.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 765417d54a15.

Provenance checked

Testing

Proof path: in-process harness.

Security

None.

Evidence

Acceptance criteria:

  • node --test tests/browser/console-drafts-kept.test.mjs
  • pnpm storybook:build
  • pnpm docs:check
  • pnpm docs:check-length
  • pnpm lint

What I checked:

Likely related people:

  • Free Wortley: Raw commit 4a50da4 adds apps/controller/src/console/drafts.mjs:33 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 4a50da4c6695; files: apps/controller/src/console/drafts.mjs)
  • kevinlin-openai: Raw commit ac2f158 adds apps/controller/src/console/drafts.mjs:18 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: ac2f1583821b; files: apps/controller/src/console/drafts.mjs)
  • stevenlee-oai: Raw commit 4a11696 adds apps/controller/src/console/agents/create.mjs:1119 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 4a116967368a; files: apps/controller/src/console/agents/create.mjs)

Review metrics

Metric Value Why it matters
Production and regression coverage Production +115/-14 lines; browser tests +538 lines across 9 cases The tests exercise distinct refusal, admission, navigation, and unload-warning behaviors through the existing Console fixture without adding production test seams.

Labels

Label changes:

  • add P2: The central repair prevents loss of unsaved Console work after ordinary refusals and transient reads, with a limited blast radius.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Label justifications:

  • P2: The central repair prevents loss of unsaved Console work after ordinary refusals and transient reads, with a limited blast radius.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Rank-up moves

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

  • Supply the required short browser walkthrough and native GitHub screenshot/video attachments for the changed behavior.

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.

Reviewed October 18, 2026, 7:51 PM ET / 23:51 UTC.

…s keep them

Keep an Agent draft now refuses its first Create once, so a reader can leave
and return to the refused form. Provisioning conflict fails the next Namespace
check once to show the draft surviving it. Keep Configuration edits delays its
save by three seconds to show the sidebar holding during a pending save.

The create-and-deploy and agent-presets sentences now name the refusals that
do not keep a draft (repository policy, a saved Agent, an admitted job, a
pending credential registration).

This branch was previously deployed

1 inactive deployment
integration-qa-pr — 34a821b9 Deployed Oct 19, 2026 by freeqaz via QA advisory (kubernetes) #2401
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: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant