mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(panel,video): dead dialog triggers behind tooltips; dotted composition ids render
HelpTip nested inside a Dialog/AlertDialog trigger puts the trigger's click handler on the Tooltip root, which renders no DOM — the agents Spawn item and the KB Reindex-All / Delete-index confirms were dead. Tooltips now wrap the triggers. The video renderer accepts interior single dots in composition ids (release-0.25.0) with '..' still unrepresentable, and propose_video refuses an unrenderable id at authoring time.
This commit is contained in:
@@ -70,18 +70,28 @@ export function SpawnAgentDialog({
|
||||
setInitialPrompt("");
|
||||
};
|
||||
|
||||
// The tooltip must wrap the DialogTrigger, never sit inside it: with
|
||||
// HelpTip as DialogTrigger's asChild child, the dialog's click handler is
|
||||
// cloned onto the Tooltip root (not a DOM element) and silently dropped.
|
||||
const defaultTrigger = (
|
||||
<HelpTip label="Start this agent's container, optionally pre-claiming a task" side="left">
|
||||
<DropdownMenuItem onSelect={(e) => e.preventDefault()}>
|
||||
<Play className="h-4 w-4 mr-2" />
|
||||
Spawn
|
||||
</DropdownMenuItem>
|
||||
</HelpTip>
|
||||
<DropdownMenuItem onSelect={(e) => e.preventDefault()}>
|
||||
<Play className="h-4 w-4 mr-2" />
|
||||
Spawn
|
||||
</DropdownMenuItem>
|
||||
);
|
||||
|
||||
return (
|
||||
<Dialog open={open} onOpenChange={setOpen}>
|
||||
<DialogTrigger asChild>{trigger || defaultTrigger}</DialogTrigger>
|
||||
{trigger ? (
|
||||
<DialogTrigger asChild>{trigger}</DialogTrigger>
|
||||
) : (
|
||||
<HelpTip
|
||||
label="Start this agent's container, optionally pre-claiming a task"
|
||||
side="left"
|
||||
>
|
||||
<DialogTrigger asChild>{defaultTrigger}</DialogTrigger>
|
||||
</HelpTip>
|
||||
)}
|
||||
<DialogContent>
|
||||
<DialogHeader>
|
||||
<DialogTitle>Spawn {agentName}</DialogTitle>
|
||||
|
||||
@@ -555,14 +555,14 @@ function KnowledgeBaseBrowserContent() {
|
||||
<CardTitle className="text-sm flex items-center justify-between">
|
||||
<span>Summary</span>
|
||||
<AlertDialog>
|
||||
<AlertDialogTrigger asChild>
|
||||
<HelpTip label="Rebuilds the Documentation index from the repo's docs/ tree — the other categories (journals, conversations, etc.) are populated live by agent activity, not by this button">
|
||||
<HelpTip label="Rebuilds the Documentation index from the repo's docs/ tree — the other categories (journals, conversations, etc.) are populated live by agent activity, not by this button">
|
||||
<AlertDialogTrigger asChild>
|
||||
<Button size="sm" variant="destructive">
|
||||
<RefreshCw className="h-3 w-3 mr-1" />
|
||||
Reindex All
|
||||
</Button>
|
||||
</HelpTip>
|
||||
</AlertDialogTrigger>
|
||||
</AlertDialogTrigger>
|
||||
</HelpTip>
|
||||
<AlertDialogContent>
|
||||
<AlertDialogHeader>
|
||||
<AlertDialogTitle>Reindex All Data?</AlertDialogTitle>
|
||||
@@ -679,8 +679,8 @@ function KnowledgeBaseBrowserContent() {
|
||||
</Button>
|
||||
</HelpTip>
|
||||
<AlertDialog>
|
||||
<AlertDialogTrigger asChild>
|
||||
<HelpTip label="Delete this index and all its documents">
|
||||
<HelpTip label="Delete this index and all its documents">
|
||||
<AlertDialogTrigger asChild>
|
||||
<Button
|
||||
size="sm"
|
||||
variant="outline"
|
||||
@@ -689,8 +689,8 @@ function KnowledgeBaseBrowserContent() {
|
||||
>
|
||||
<Trash2 className="h-3 w-3" />
|
||||
</Button>
|
||||
</HelpTip>
|
||||
</AlertDialogTrigger>
|
||||
</AlertDialogTrigger>
|
||||
</HelpTip>
|
||||
<AlertDialogContent>
|
||||
<AlertDialogHeader>
|
||||
<AlertDialogTitle>
|
||||
|
||||
@@ -342,6 +342,8 @@ _CURATE_VAULT_ROLES: frozenset[str] = frozenset({"auditor"})
|
||||
# reuses MAX_TWEET_CHARS). No role frozenset here, unlike the sets above:
|
||||
# propose_video is gated on the caller's TEAM at runtime (_caller_team), not
|
||||
# role — Role.DEVELOPER doesn't distinguish a ux-dev from a be-dev/fe-dev.
|
||||
# Kept in lockstep with video-renderer/server.js COMPOSITION_ID_RE.
|
||||
_COMPOSITION_ID_RE = re.compile(r"^[A-Za-z0-9_-]+(\.[A-Za-z0-9_-]+)*$")
|
||||
_VIDEO_PLATFORMS: frozenset[str] = frozenset({"x", "tiktok"})
|
||||
_MAX_TIKTOK_CAPTION_CHARS = 2200
|
||||
|
||||
@@ -1532,6 +1534,20 @@ class ContentActions:
|
||||
statement count under the xenon/PLR0911 budget)."""
|
||||
if rej := cls._reject_soup(composition_id, field="composition_id", min_chars=2):
|
||||
return rej
|
||||
# Mirror the video-renderer sidecar's charset rule so an unrenderable
|
||||
# id is refused at authoring time, not at render time days later.
|
||||
if not _COMPOSITION_ID_RE.fullmatch(composition_id.strip()):
|
||||
return Envelope.invalid_state(
|
||||
message=(
|
||||
f"composition_id {composition_id!r} is not renderable — "
|
||||
"letters, digits, '_' or '-' with optional interior dots"
|
||||
),
|
||||
remediate=(
|
||||
"rename the composition dir to match (e.g. "
|
||||
"'release-0-25-0' or 'release-0.25.0') and call "
|
||||
"propose_video again with that id"
|
||||
),
|
||||
)
|
||||
if rej := cls._reject_caption(
|
||||
x_caption, field="x_caption", max_chars=MAX_TWEET_CHARS
|
||||
):
|
||||
|
||||
@@ -286,3 +286,14 @@ async def test_propose_video_defaults_input_props_to_empty_dict(
|
||||
draft = markers.get_video_draft(authoring)
|
||||
assert draft is not None
|
||||
assert draft["input_props"] == {}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_propose_video_rejects_unrenderable_composition_id() -> None:
|
||||
"""An id the renderer's charset rule would 400 is refused at authoring
|
||||
time, not at render time days later."""
|
||||
env = await _actions("developer", "ux_ui").propose_video(
|
||||
agent_id=uuid4(), **_valid_kwargs(composition_id="release 0.25.0!")
|
||||
)
|
||||
assert env.error == "invalid_state"
|
||||
assert "not renderable" in (env.message or "")
|
||||
|
||||
@@ -24,7 +24,10 @@ const PORT = Number(process.env.PORT ?? 3001);
|
||||
// safe path segment so a ".." / "/" / absolute-path value can't escape the
|
||||
// composition dir (path traversal). The orchestrator only ever sends a real
|
||||
// composition id, but this is the trust boundary — validate here.
|
||||
const COMPOSITION_ID_RE = /^[A-Za-z0-9_-]+$/;
|
||||
// Letters/digits/_/- plus interior single dots (e.g. release-0.25.0).
|
||||
// No leading dot and no adjacent dots, so '.'/'..' path segments and
|
||||
// hidden-file names remain unrepresentable — this stays the trust boundary.
|
||||
const COMPOSITION_ID_RE = /^[A-Za-z0-9_-]+(\.[A-Za-z0-9_-]+)*$/;
|
||||
|
||||
// motion/ source (no node_modules, no build output) is a few hundred KB in
|
||||
// practice; this cap is generous headroom, not a tuned budget.
|
||||
@@ -74,7 +77,7 @@ app.post("/render", renderLimiter, upload.single("source"), async (req, res) =>
|
||||
) {
|
||||
res.status(400).json({
|
||||
error:
|
||||
"'composition_id' must be a non-empty string of letters, digits, '_' or '-'",
|
||||
"'composition_id' must be letters, digits, '_' or '-', with optional interior dots",
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user