Repository navigation
feat(assistant): workspace custom tools that run scripts on dev machines - #103
Conversation
b62127d to
8f6e651
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f6e651aa2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| summary := *run | ||
| summary.Output = outputTail(run.Output, 4096) | ||
| _, err := conversationEvent(ctx, tx, *run.ConversationID, "tool_run.updated", summary) |
There was a problem hiding this comment.
When a tool run changes state while its conversation is streaming a reply, this inserts an event without first locking that conversation. conversation_events.cursor is identity-generated, so a concurrent ConversationRepository.Append can commit a higher cursor before this transaction commits its lower cursor; the SSE reader then advances past the tool-run event and permanently misses it on replay. toolRunTransaction locks the run and operation, not the conversation, and machine-targeted QueueRun also lacks that lock. Acquire the conversation's cursor lock before inserting these events, preserving the established lock ordering. AGENTS/SKILLS/kuayle-backend/SKILL.mdL43-L45
Useful? React with 👍 / 👎.
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| approval, err := s.Repo.CreateApproval(ctx, a, toolCallID, "run_custom_tool", prepared.issue.ID, data, assistantApprovalTTL) |
There was a problem hiding this comment.
For any approval-mode custom tool called from Web chat, the new run_custom_tool approval falls through to the delegation branch in UI/src/lib/features/assistant/ToolCalls.svelte: the prompt says “Start a coding agent” and renders the absent payload.instructions, rather than showing the tool, version, machine and validated inputs. The Approve button still executes the script, so users authorize a different operation from the one described by the confirmation. Add the corresponding conversation API contract and a dedicated approval rendering branch before exposing this action. AGENTS/SKILLS/kuayle-backend/SKILL.mdL35-L36
Useful? React with 👍 / 👎.
| if req.ApprovalID != nil { | ||
| existing := &domain.AssistantToolRun{} | ||
| err = tx.GetContext(ctx, existing, `SELECT `+toolRunColumns+` FROM assistant_tool_runs r WHERE r.approval_id=$1 AND r.workspace_id=$2`, req.ApprovalID, ws) |
There was a problem hiding this comment.
If two requests concurrently approve the same tool run with an explicit machine_id, both can receive the approving claim and find no existing run here, because this path has no task transaction to serialize them. The first request queues the run; the second then acquires the machine lock and receives ErrMachineBusy without checking the approval's run again. AssistantApprovals.runCustomTool treats that error as a terminal approval failure, so if its FinishApproval wins the race, the approval is permanently marked failed even though its script was successfully queued. Serialize requests for the same approval before this lookup, or recheck the existing run after acquiring the machine lock and before rejecting it as busy.
Useful? React with 👍 / 👎.
| labels := machineLabels(machine) | ||
| labels["com.kuayle.service"] = "agent" | ||
| labels["com.kuayle.tool-run-id"] = run.ID.String() |
There was a problem hiding this comment.
When an awaiting-review task on a reused machine is cancelled while its custom tool is running without output, DockerRuntime.CancelAgent misses this container because it selects com.kuayle.agent-run-id, which these labels omit. The manager also excludes run_tool from active-run cancellation, and the queued operation lacks the attempt/checkout identifiers checked by FinishCancellation. Consequently, task cancellation can report runtime cleanup complete while the script continues modifying the checkout until its next output write or timeout. Bind task-targeted tool operations and containers to their attempt and include them in physical cancellation. AGENTS/SKILLS/kuayle-backend/SKILL.mdL54-L56
Useful? React with 👍 / 👎.
Add assistant_custom_tools and assistant_custom_tool_versions (migration 99) and assistant_tool_runs with the run_tool machine operation and the run_custom_tool approval (migration 100). Tools have a strictly validated scalar parameter schema, a script for sh, bash, node or python3, a run mode (auto only when read-only) and versions with drafts, activation and restore; owners and admins write and test-run them through /assistant-tools, members read tools and their own runs. The assistant is offered active, enabled tools as custom_<name>, validates inputs against the schema, asks for approval (or runs read-only auto tools) and reads results with get_tool_run; propose_tool only saves drafts. Runs use the manual check's machine lock and a credential-free check container with a fixed command line: the script and arguments arrive only as environment variables and a JSON file, never interpolated.
8f6e651 to
fb653cb
Compare
Problem
Phase 3 of the orchestrator plan (step S8, backend only): admins should be able to give the assistant workspace-specific tools — scripts that run on a dev machine — and the assistant should be able to draft new ones, without ever running code on the Kuayle server or letting arguments become shell syntax.
This branch is built on S6 (skills, #100) with S2 (run check, #99) merged in, so the diff against
mainincludes them until they merge. The S8 change is the last commit.Change
assistant_custom_tools+assistant_custom_tool_versions. A tool has a snake_case name (unique per workspace; it can't be a built-in name, with or withoutcustom_), a description, a strictly validated parameter schema, a script (≤ 64 KiB), an interpreter (sh/bash/node/python3, all present in the ide and headless images), a run mode (approval, orautoonly whenread_only, enforced in the DB too), a timeout (10–1800 s, default 600), enabled, status draft|active, versions and author kind. Versions, drafts, activation and restore work the same way as skills.domain.ParseToolSchema/ValidateInputs): an object of at most 20 string (min/maxLength ≤ 8192), number/integer (finite bounds), boolean or string-enum properties, with no additional properties, nesting, unknown keywords, duplicate keys or quoted numbers. Schemas are capped at 16 KiB and inputs at 16 KiB.assistant_tool_runs, therun_tooloperation, therun_custom_toolapproval action (the constraint carries the full union, includingrun_check), and tool-call names may now contain digits. Queuing uses the manual check'slockMachineForCheck, and a queued or running tool run holds the machine just like a manual check does.machine/custom_tool.go): each run gets its own check container, with the developer image, the hardened check host config, egress only through the proxy, no credentials, and the attempt's snapshot used only for redaction. The command line is a fixedsh -c <bootstrap> kuayle-tool <interpreter>. The script and inputs arrive only in the environment, the bootstrap writes them to privatemktempfiles (KUAYLE_TOOL_INPUT_FILE), and every input is alsoKUAYLE_ARG_<NAME>. Output goes through the agent output buffer (redaction and caps) and is stored as a 64 KiB tail. The run has a timeout, its container is removed when it finishes, and a run that fails for good is recorded with a safe reason. Every write re-checks the operation lease and generation, the requester's role, that the tool is still active at that version, and that the task or machine still belongs to the requester./assistant-toolsCRUD,/versions,/activate,/versions/:v/restore,/enabled,POST /:id/test-run {inputs, machine_id|identifier, version?}(owners/admins; can run drafts),GET /:id/runs,GET /assistant-tool-runs/:runId. Members read tools; non-admins read only the runs they requested.custom_<name>with{identifier?, machine_id?, inputs: <schema>}. Approval-mode calls create arun_custom_toolcard bound to the issue, version, inputs and task generation, and approving queues one run (idempotent). Auto tools run directly and are awaited for up to 2 minutes. Results go back to the model asuntrusted_data, andget_tool_runreads them later.propose_toolonly saves drafts (always in approval mode) and is limited to owners/admins. Custom tools show up in/assistant-capabilities(custom: true), and their switch is the tool's own enabled flag. Telegram has approval/failure answers and names saved draft tools.UI/src/lib/api/assistant-tools.tsclient types only (the settings UI is S9).BE/docs/conversations.md#custom-tools,BE/docs/agent-tasks.md#custom-tool-runs, TECHNICAL.md.Verification
go build ./... && go vet ./...pass.go test ./... -count=1 -p 2gave 1878 passed (tests and subtests), 9 skipped (the Docker/sudo integration tests and the env-gated issue/analytics Postgres tests), 0 failed.; rm -rf /,$(…)and backticks arrive literally and nothing they contain executes;dev-machine-ideanddev-machine-headlessimages (sh,bash,node,python3,mktemp).npm run check(0 errors) andnode AGENTS/TOOLS/check-harness.mjspass.Limits
identifier, becauseconversation_approvals.issue_idis NOT NULL.machine_id-only calls are for auto tools and test runs.queued/runningand usesget_tool_run.tool_run.updatedconversation events. There are no per-line log chunks and no artifacts page yet.docker runsmoke test wasn't allowed in this environment). The bootstrap itself is covered by the local-shell test.