Skip to content

fix(dashboard): allow re-adding a disabled ACP runner key - #1824

Open
fengyue-xve wants to merge 2 commits into
TencentCloud:developfrom
fengyue-xve:fix/acp-runner-reenable
Open

fengyue-xve wants to merge 2 commits into
TencentCloud:developfrom
fengyue-xve:fix/acp-runner-reenable

Conversation

@fengyue-xve

Copy link
Copy Markdown

Summary

Fixes #1815. The ACP runner create/edit form rejected any existing runner key with runnerKeyExists, so once a custom runner had been disabled there was no way to add it back under the same name — and disabled custom runners are exactly the state a user is most likely to re-add. Builtin runners cannot be deleted, so for them the form was a dead end too.

The save guard now only treats enabled or builtin keys as collisions:

  • disabled custom key → the save overwrites the stale entry; the create form defaults to enabled, so the runner comes back enabled
  • enabled custom key → still rejected with the same toast as before
  • builtin key (even a disabled one) → still rejected, so builtins can never be shadowed

Target branch

  • Base is develop (feature / fix — default)

Type of change

  • Bug fix

Test plan

  • npx tsc -b — clean

  • npx eslint on both changed files — clean (the two pre-existing react-hooks/exhaustive-deps warnings in ACP/index.tsx are untouched)

  • npx vitest run src/pages/Agent/ACP — 3 passed: new ACPPanel.test.tsx covers re-adding a disabled custom runner (single PUT, re-enabled) plus both rejection paths (enabled custom / disabled builtin → toast, no request at all)

  • Verified the regression test fails on the unpatched guard: with the old code the "re-adds a disabled custom runner" case times out while runnerKeyExists is shown, and passes with the fix

  • Full-suite npx vitest run (local Windows): 1187 passed; the remaining failures are pre-existing on the develop base and unrelated to this diff (same set documented in fix(dashboard): localize remaining hardcoded Chinese UI strings (browser AI panel, skill recording, PWA guides) #1799): DOMMatrix is not defined (pdfjs in jsdom) when collecting docxSanitize / skillMarkdown / SkillDrawer, knowledgeBases / publishedExperts mocks on the base missing the newer bridgeAgentHeaders export, and one rc-notification close-timer error attributed to ChannelsPanel.test.tsx

  • Added/updated tests

  • make all passes locally (the Python harness make all does not apply to a dashboard-only change; the dashboard checks above were run instead)

Checklist

  • Updated CHANGELOG.md (if user-facing)
  • README / docs updated (if needed)

The create/edit guard rejected any existing runner key with
"runnerKeyExists", so a custom runner that had been disabled could never
be added again under the same name.

Only enabled or builtin keys now collide; saving over a disabled custom
entry overwrites it (the create form defaults enabled=true, so the
runner comes back enabled). Covered by ACPPanel.test.tsx.

Fixes TencentCloud#1815
@fengyue-xve

Copy link
Copy Markdown
Author

The Windows job is red on the unrelated flaky test_cold_target_install_budget_preserves_regular_timeout (float wobble in the 900s budget assertion); fix proposed in #1826.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant