From f1dd6c19573593e8329ff31fc354c21e28c94123 Mon Sep 17 00:00:00 2001 From: Jiang Bohan Date: Wed, 29 Apr 2026 16:51:12 +0800 Subject: [PATCH] fix(cli,quick-create): no duplicate issue when --attachment fails post-create MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two coordinated fixes for a quick-create case where the agent ended up creating duplicate issues. Repro: user pasted an image into the quick-create prompt; the front-end uploaded it and embedded the URL as markdown in the user input; the agent saw the URL, assumed it was an attachment, and ran `multica issue create … --attachment "https://…"`. The CLI POSTed the issue first, then failed to read the URL as a file (`os.ReadFile("https://…")`) and exited 1. The agent treated exit 1 as "create failed" and retried — but the first issue already existed, so the workspace ended up with two of them. CLI (`server/cmd/multica/cmd_issue.go`): - `runIssueCreate` pre-validates `--attachment` BEFORE POSTing. URLs are warned about and skipped (they are never local files); local-path read errors fail before the issue is created so no half-baked issue lands. Once the POST succeeds, post-create upload failures only print a stderr warning and the issue metadata is still emitted — never a non-zero exit, so callers cannot mistake "attachment upload hiccup" for "create failed" and retry. - `runIssueCommentAdd` already uploads attachments BEFORE the comment is created, so its failure mode is fine; it just gets the same URL-skip behaviour for consistency. Quick-create prompt (`buildQuickCreatePrompt`): - Tells the agent NOT to pass `--attachment` for prompt-embedded image URLs (they are already part of the description as markdown). - Hardens the "no retry" rule: even on a non-zero exit, do not retry `issue create` — the issue may already exist. --- server/cmd/multica/cmd_issue.go | 67 +++++++++++++++++++++++++++----- server/internal/daemon/prompt.go | 5 ++- 2 files changed, 61 insertions(+), 11 deletions(-) diff --git a/server/cmd/multica/cmd_issue.go b/server/cmd/multica/cmd_issue.go index 39d6fdc67..751ea8a03 100644 --- a/server/cmd/multica/cmd_issue.go +++ b/server/cmd/multica/cmd_issue.go @@ -465,6 +465,15 @@ func runIssueGet(cmd *cobra.Command, args []string) error { return cli.PrintJSON(os.Stdout, issue) } +// isHTTPURL reports whether path is an http:// or https:// URL. +// Used to skip URL-shaped values passed to --attachment, which only +// accepts local file paths. Trims surrounding whitespace because +// agent-generated commands sometimes copy URLs with stray spaces. +func isHTTPURL(path string) bool { + p := strings.TrimSpace(path) + return strings.HasPrefix(p, "http://") || strings.HasPrefix(p, "https://") +} + func runIssueCreate(cmd *cobra.Command, _ []string) error { title, _ := cmd.Flags().GetString("title") if title == "" { @@ -529,22 +538,53 @@ func runIssueCreate(cmd *cobra.Command, _ []string) error { body["origin_id"] = taskID } + // Pre-validate attachments BEFORE creating the issue so a bad path + // can never produce a half-created issue (which would otherwise + // trigger callers — especially the agent doing quick-create — to + // retry the whole `issue create` and end up with duplicates). + // + // - http(s) URLs are not local files; the API only accepts local + // paths here. Warn and skip rather than fail — a markdown image + // URL embedded in the prompt should never be re-attached, and + // skipping is the safest outcome for that case. + // - Anything else is treated as a local path and read upfront. + // A read failure here is a real user/agent mistake (typo, + // missing file) and we surface it pre-create so the issue + // never lands. + type pendingAttachment struct { + path string + data []byte + } + pending := make([]pendingAttachment, 0, len(attachments)) + for _, filePath := range attachments { + if isHTTPURL(filePath) { + fmt.Fprintf(os.Stderr, "Skipping --attachment %q: URLs are not supported here, only local file paths.\n", filePath) + continue + } + data, readErr := os.ReadFile(filePath) + if readErr != nil { + return fmt.Errorf("read attachment %s: %w", filePath, readErr) + } + pending = append(pending, pendingAttachment{path: filePath, data: data}) + } + var result map[string]any if err := client.PostJSON(ctx, "/api/issues", body, &result); err != nil { return fmt.Errorf("create issue: %w", err) } // Upload attachments and link them to the newly created issue. + // Failures here are partial-success: the issue exists already, so + // turning a non-zero exit on the caller would invite a retry that + // duplicates the issue. Warn on stderr and continue. issueID := strVal(result, "id") - for _, filePath := range attachments { - data, readErr := os.ReadFile(filePath) - if readErr != nil { - return fmt.Errorf("read attachment %s: %w", filePath, readErr) + for _, att := range pending { + if _, uploadErr := client.UploadFile(ctx, att.data, att.path, issueID); uploadErr != nil { + fmt.Fprintf(os.Stderr, "warning: upload attachment %s failed (issue already created, %s): %v\n", + att.path, strVal(result, "identifier"), uploadErr) + continue } - if _, uploadErr := client.UploadFile(ctx, data, filePath, issueID); uploadErr != nil { - return fmt.Errorf("upload attachment %s: %w", filePath, uploadErr) - } - fmt.Fprintf(os.Stderr, "Uploaded %s\n", filePath) + fmt.Fprintf(os.Stderr, "Uploaded %s\n", att.path) } output, _ := cmd.Flags().GetString("output") @@ -835,9 +875,18 @@ func runIssueCommentAdd(cmd *cobra.Command, args []string) error { ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() - // Upload attachments and collect their IDs. + // Upload attachments and collect their IDs. URLs are skipped with a + // warning — `--attachment` only accepts local file paths, and a + // markdown image URL embedded in agent-supplied content should never + // be re-uploaded as if it were a file. Unlike `issue create`, this + // path uploads BEFORE posting the comment, so a hard failure on a + // real (local) attachment correctly aborts the whole call. var attachmentIDs []string for _, filePath := range attachments { + if isHTTPURL(filePath) { + fmt.Fprintf(os.Stderr, "Skipping --attachment %q: URLs are not supported here, only local file paths.\n", filePath) + continue + } data, readErr := os.ReadFile(filePath) if readErr != nil { return fmt.Errorf("read attachment %s: %w", filePath, readErr) diff --git a/server/internal/daemon/prompt.go b/server/internal/daemon/prompt.go index 8c3fb26cc..979af03c6 100644 --- a/server/internal/daemon/prompt.go +++ b/server/internal/daemon/prompt.go @@ -57,9 +57,10 @@ func buildQuickCreatePrompt(task Task) string { b.WriteString(" - When the user did NOT name an assignee, default to YOURSELF (the picker agent): pass `--assignee `. Never leave the issue unassigned.\n") } b.WriteString("- project: omit. The platform will route the issue to the workspace default.\n") - b.WriteString("- status: omit (defaults to `todo`).\n\n") + b.WriteString("- status: omit (defaults to `todo`).\n") + b.WriteString("- attachments: do NOT pass `--attachment`. The flag only accepts LOCAL file paths, and any image URL embedded in the user input is already part of the description as markdown — keep it inline in `--description` instead of trying to re-attach it. (Trying to pass `https://…` to `--attachment` will fail and look like a create error to you, but the issue may already exist; never retry `issue create` on that signal.)\n\n") b.WriteString("Output format:\n") - b.WriteString("- Run exactly one `multica issue create` invocation.\n") + b.WriteString("- Run exactly one `multica issue create` invocation. Do not retry it for any reason — even on a non-zero exit. The issue may already exist; another attempt would create a duplicate.\n") b.WriteString("- After it succeeds, print exactly one line: `Created MUL-: ` and exit. No commentary, no follow-up tool calls.\n") b.WriteString("- Do NOT call `multica issue get` or `multica issue comment add` for this task — there is no issue to query or comment on prior to creation.\n") b.WriteString("- If the CLI returns an error, exit with that error as the only output. The platform writes a failure notification automatically; do not retry.\n")