Conversation
A fully non-ASCII name (for example a Chinese-only name) cannot produce an agent key under the current slug rule: - AgentCreatePage computed an empty key from the name, which disabled the create button because the form requires a non-empty agent key. - createAgent appended the random suffix to the empty slug and sent "-<uuid8>" as the agent key (leading hyphen). Extract slugifyAgentKey/resolveAgentKey helpers shared by the page and the API layer. The page keeps a temporary agent-<uuid8> key and shows a hint that asks the user to enter a readable key when the name is non-ASCII; createAgent falls back to the same readable agent- prefix instead of the malformed leading-hyphen key.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
oss-maintainer
left a comment
There was a problem hiding this comment.
Summary
Good catch and a clean fix for both symptoms: the disabled submit button and the -<uuid8> key. Extracting slugifyAgentKey / resolveAgentKey with unit tests is the right shape, CI is green, and the non-ASCII hint is a nice touch. Verdict: nearly there — one path in the same bug class is still open.
The advanced "Agent key" input is not covered. It keeps the old inline rule without the leading/trailing-dash strip and without an empty-slug fallback, so typing a fully non-ASCII value there yields -, which passes canSubmit and is forwarded verbatim by resolveAgentKey (an explicit key only gets .trim()). That is the same malformed leading-hyphen key this PR removes from the name path. Reusing slugifyAgentKey in that handler plus a ^[a-z0-9][a-z0-9_-]*$ check on the explicit key closes it.
Details, line anchors and suggested snippets are inline.
Findings
- [Warning]
AgentCreatePage.tsx:286— manual key field bypasses the new slug rule;-can still reach the API. - [Warning]
agents.ts:222—resolveAgentKeyforwards unvalidated explicit keys. - [Info]
AgentCreatePage.tsx:130—fallbackAgentKeyis generated once per mount and reused for later names. - [Info]
AgentCreatePage.tsx:287— hint is only visible inside the collapsed Advanced section. - [Info]
agents.test.ts:3— import/statement ordering, andunstubAllGlobalsbelongs inafterEach.
Suggestions
export function resolveAgentKey(name: string | undefined, explicitKey?: string): string {
const trimmed = (explicitKey || ).trim();
if (/^[a-z0-9][a-z0-9_-]*$/.test(trimmed)) return trimmed;
const slug = slugifyAgentKey(trimmed || name || agent);
return `${slug || agent}-${crypto.randomUUID().slice(0, 8)}`;
}Automated review by github-manager-bot
| <div style={{ ...S.row, paddingLeft: 0, paddingRight: 0 }}> | ||
| <label htmlFor="agent-key" style={S.label}>Agent key</label> | ||
| <div><input id="agent-key" style={S.input} value={agentKey} onChange={e => { setAgentKeyCustomized(true); setAgentKey(e.target.value.toLowerCase().replace(/[^a-z0-9_-]+/g, '-')); }} placeholder="repository-reviewer" /><div style={S.hint}>{scope.selectorVisible ? 'Stable identity inside the current tenant and namespace.' : 'Stable identity for this agent.'}</div></div> | ||
| <div><input id="agent-key" style={S.input} value={agentKey} onChange={e => { setAgentKeyCustomized(true); setAgentKey(e.target.value.toLowerCase().replace(/[^a-z0-9_-]+/g, '-')); }} placeholder="repository-reviewer" /><div style={S.hint}>{scope.selectorVisible ? 'Stable identity inside the current tenant and namespace.' : 'Stable identity for this agent.'}</div> |
There was a problem hiding this comment.
[Warning] The advanced "Agent key" input still uses the old inline rule, and it is the one path the PR does not cover:
setAgentKey(e.target.value.toLowerCase().replace(/[^a-z0-9_-]+/g, '-'))No ^-+|-+$ trim and no empty-slug fallback, so typing a fully non-ASCII value here (e.g. 中文名字) produces the single character -. That passes canSubmit (line 172 only requires a non-empty agentKey.trim()) and reaches the API as -, because resolveAgentKey returns a non-empty explicit key verbatim — i.e. the exact malformed leading-hyphen key this PR removes from the name path. Two small changes close the gap:
// reuse the shared rule instead of the inline copy
setAgentKey(slugifyAgentKey(e.target.value));and in resolveAgentKey, treat an explicit key that is not a valid key as "no explicit key":
const trimmed = (explicitKey || '').trim();
if (/^[a-z0-9][a-z0-9_-]*$/.test(trimmed)) return trimmed;With that, both inputs are normalised by one rule and the generated suffix/fallback kicks in whenever the user's key cannot be used.
| * name contains no ASCII slug characters (e.g. a fully Chinese name). | ||
| */ | ||
| export function resolveAgentKey(name: string | undefined, explicitKey?: string): string { | ||
| const trimmed = (explicitKey || '').trim(); |
There was a problem hiding this comment.
[Warning] resolveAgentKey trusts explicitKey after trimming only, so it forwards whatever the caller puts in the field (see the AgentCreatePage note about -). The doc comment above promises "an explicit key wins", which is fine as a contract, but the function is exported and is now the single choke point for key generation — it is the natural place to enforce the shape the backend stores. Suggest validating against ^[a-z0-9][a-z0-9_-]*$ here and falling through to slug + suffix when it does not match, so the API layer cannot send a key the control plane rejects.
| const [agentKey, setAgentKey] = useState(''); | ||
| const [agentKeyCustomized, setAgentKeyCustomized] = useState(false); | ||
| // Used when the name cannot be turned into an ASCII slug (e.g. a fully Chinese name). | ||
| const [fallbackAgentKey] = useState(() => `agent-${crypto.randomUUID().slice(0, 8)}`); |
There was a problem hiding this comment.
[Info] fallbackAgentKey is created once per page mount, so it is reused for every subsequent non-ASCII name in the same mount (change 中文名字 to 另一个名字 and the visible key stays agent-<same suffix>). It is only cosmetic today because a successful create navigates away, but a failed create that keeps the page mounted would re-submit the same key. Consider regenerating it when the slug comes out empty, e.g. keep the state as string | null and do setFallback(k => k ?? generated) only when the previous value was consumed, or simply let resolveAgentKey own the generation (see the explicit-key validation suggestion) and keep the field empty until submit.
| <label htmlFor="agent-key" style={S.label}>Agent key</label> | ||
| <div><input id="agent-key" style={S.input} value={agentKey} onChange={e => { setAgentKeyCustomized(true); setAgentKey(e.target.value.toLowerCase().replace(/[^a-z0-9_-]+/g, '-')); }} placeholder="repository-reviewer" /><div style={S.hint}>{scope.selectorVisible ? 'Stable identity inside the current tenant and namespace.' : 'Stable identity for this agent.'}</div></div> | ||
| <div><input id="agent-key" style={S.input} value={agentKey} onChange={e => { setAgentKeyCustomized(true); setAgentKey(e.target.value.toLowerCase().replace(/[^a-z0-9_-]+/g, '-')); }} placeholder="repository-reviewer" /><div style={S.hint}>{scope.selectorVisible ? 'Stable identity inside the current tenant and namespace.' : 'Stable identity for this agent.'}</div> | ||
| {!agentKeyCustomized && !!name.trim() && !slugifyAgentKey(name) && ( |
There was a problem hiding this comment.
[Info] Nice touch showing the hint instead of silently generating a key. Two small things: the condition recomputes slugifyAgentKey(name) on every render while the state already holds the derived value, and the wording ("A temporary key was generated") does not tell the user what the current value is. Minor: also consider surfacing the hint outside the collapsed <summary>Advanced settings</summary> block, because a user who never opens that section will still submit agent-xxxxxxxx without knowing it is not derived from their name. The #b45309 color literal should ideally come from the existing theme tokens used elsewhere in this file.
| @@ -0,0 +1,57 @@ | |||
| import { beforeAll, describe, expect, it, vi } from 'vitest'; | |||
|
|
|||
| beforeAll(() => { | |||
There was a problem hiding this comment.
[Info] Test-hygiene nits on an otherwise good suite: (1) beforeAll sits between the two import statements — ESM hoisting means ./agents is still evaluated first, so move it below the imports for readability; (2) vi.unstubAllGlobals() is inside the last it, so a failure in an earlier case leaks the stubbed fetch/localStorage into the rest of the file — put it in afterEach; (3) the createAgent case asserts only the key shape — asserting body.displayName === '中文名字' (already done, good) plus that no leading hyphen can ever be produced would lock the regression this PR fixes. npm test (vitest, node environment, no jsdom) covers this file without a DOM, which matches how the other src/**/*.test.ts suites run here.
AgentScope-Java Version
Current
main(97696a3, 2026-09-15).Description
A fully non-ASCII agent name (for example a Chinese-only name) breaks agent creation in two places:
AgentCreatePagederives the agent key from the name with the slug rulereplace(/[^a-z0-9_-]+/g, '-')and strips leading/trailing dashes. For a fully non-ASCII name the result is the empty string, socanSubmit(which requiresagentKey.trim()) stays false and the "Create & open agent" button never enables.createAgentappends the random suffix to the empty slug and sends-<uuid8>(leading hyphen) as the agent key.This change extracts two shared helpers:
slugifyAgentKey(name)— the existing slug rule, reusable by page and API.resolveAgentKey(name, explicitKey)— explicit key wins; otherwise slug + short random suffix; when the slug is empty it falls back to a readableagent-<uuid8>key (no leading hyphen).AgentCreatePagenow keeps a per-page temporaryagent-<uuid8>key for non-ASCII names, so the form stays submittable, and shows a hint under the Agent key field that asks the user to enter a readable key (a-z,0-9,-,_).How to test
中文名字).-xxxxxxxx).agent-xxxxxxxxkey is generated, the create button enables, a hint asks for a readable key, andcreateAgentsendsagent-xxxxxxxx(verified by the new unit tests).Checklist
mvn spotless:applynpx vitest run src/api/agents.test.ts(7/7),tsc --noEmitandeslintclean for the touched files