Skip to content

fix(console): ask for model credentials in the slack drawer only when none are set - #2986

Merged
freeqaz merged 3 commits into
mainfrom
fix-2063
Oct 20, 2026
Merged

freeqaz merged 3 commits into
mainfrom
fix-2063

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Dogfood D658 (board finding 2063, low): the Configure Slack drawer on an already deployed Agent with a bound API key says "Configure model credentials in the Credentials tab before first deploy." Neither half applies to that Agent.

Change

The Agent page now passes the channel drawer its notice (the copy.drawerNotice hook Create Agent already uses). Every deployment needs a harness authentication source (Deploy is disabled without agent.harnessAuth), so:

  • Agent with a model authentication source (API key, credential source or runtime): "New version. Changes affect future deployments using this Configuration."
  • Agent without one: the same, plus "Configure model credentials in the Credentials tab before deploying."

Create Agent's own notice is unchanged. The drawer's fallback notice in channels/shared-ui.mjs, now used by neither page, says "before deploying" too. No other copy changes.

Verification

Screenshots (real controller; Agent with its API key bound, Edit Slack open)

Before 1440 After 1440 Before 390 After 390

Storybook: new Components/Channels → Slack editor, no model authentication story (auth: null) shows the long notice; the existing Slack editor story (keyed Agent) now shows the short one. Checked by loading the built frames locally.

Risks

Copy only, chosen from agent.harnessAuth as the page already reads it. An Agent whose authentication source is removed while the tab is open shows the short notice until the tab rerenders.

… none are set

The Configure Slack drawer on an Agent with its API key bound, deployed
or not, said to configure model credentials in the Credentials tab
before first deploy (finding 2063, dogfood D658). Every deployment needs
a harness authentication source, so the Agent page now passes the
drawer a notice that names the Credentials tab only while the Agent has
none.
@freeqaz
freeqaz requested a review from a team as a code owner October 20, 2026 01:15
@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. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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. clawsweeper:intake:needs-repro 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

The Slack drawer requests model credentials only when the Agent has no authentication source.

Example: Open Edit Slack for an Agent with an API key already bound.

  • Before: The notice asks the user to configure model credentials before first deploy.
  • After: The notice only explains that Configuration changes affect future deployments.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The focused patch is correct and the prior finding is fixed; supplied visual proof covers only one authentication state.
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: An Agent with model authentication already configured receives an unnecessary instruction to configure it before first deployment.
Reason: The maintainer-authored direction corrects a demonstrated contradiction with a small presentation change.

Merge readiness

✅ Ready for maintainer review

This PR makes a useful, focused correction and resolves the prior Storybook finding; no actionable code defect remains.

Priority: P3
Reviewed head: 91d350559de2b545e4d19dea36b5ec611739d804

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: The submitted visual evidence does not show the no-authentication notice or a walkthrough covering both authentication states.
Agent review details

How this fits together

The Agent detail page reads authentication metadata and supplies notice text to the shared channel drawer.

flowchart LR
  A[Agent API metadata] --> B[Agent detail page]
  B --> C[Authentication presence check]
  C --> D[Shared channel drawer]
  D --> E[Slack setup notice]
Loading

Technical review

Best possible solution:

Keep the existing notice hook and authentication-presence check as the complete fix.

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

Current-main source and supplied before screenshots establish the unconditional reminder; the PR reports a base-failing, head-passing browser regression, which this read-only review did not run.

Is this the best way to solve the issue?

The patch uses the existing presentation owner and matches the deployment prerequisite without adding state or changing credential handling.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 14a04fbb64c0.

Provenance checked

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

Likely related people:

  • Free Wortley: Raw commit 9abddf7 adds apps/controller/src/console/channels/shared-ui.mjs:553 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 9abddf791b7b; files: apps/controller/src/console/channels/shared-ui.mjs)
  • kevinlin-openai: Raw commit 835b1cc adds apps/controller/src/console/channels/shared-ui.mjs:293 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 835b1cc9b775; files: apps/controller/src/console/channels/shared-ui.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:

  • P3: This corrects misleading setup copy without changing deployment, authentication, or saved settings.
  • 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 two-state visual evidence and native screenshot/video attachments described in realBehaviorProof.

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-20T01:30:49.619Z sha 78a0e2a :: needs changes before merge. :: [P3] Add the missing-authentication Slack drawer Storybook state

Reviewed October 19, 2026, 10:05 PM ET / October 20, 2026, 02:05 UTC (Revision 2).

@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. clawsweeper:intake:needs-repro 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. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 20, 2026
@freeqaz
freeqaz merged commit 384fe53 into main Oct 20, 2026
71 of 73 checks passed
@freeqaz
freeqaz deleted the fix-2063 branch October 20, 2026 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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