chore: apply pr-reviewer fixes for #244 (#247)

Co-authored-by: agent-relay-code[bot] <agent-relay-code[bot]@users.noreply.github.com>

Author: kjgbot

cmd/relayfile-cli/main.go

@@ -2753,7 +2753,7 @@ func deadLetterErrorPathFor(localDir, opID string) string {
 
 func runWriteback(args []string, stdout io.Writer) error {
 	if len(args) == 0 {
-		return errors.New("writeback subcommand is required: list, status, or retry")
+		return errors.New("writeback subcommand is required: list, status, retry, or sweep-drafts")
 	}
 	switch args[0] {
 	case "list":

cmd/relayfile-cli/writeback_sweep.go

@@ -66,15 +66,16 @@ func runWritebackSweepDrafts(args []string, stdout io.Writer) error {
 		return errors.New(writebackSweepUsage)
 	}
 
-	record, err := resolveWorkspaceRecord(firstArg(fs))
+	creds, err := loadCredentials()
 	if err != nil {
 		return err
 	}
-	creds, err := loadCredentials()
+	tokenValue := resolveToken(*tokenOverride, creds)
+	workspaceID, err := resolveWorkspaceIDWithToken(firstArg(fs), tokenValue)
 	if err != nil {
 		return err
 	}
-	client, err := newAPIClient(resolveServer(*server, creds), resolveToken(*tokenOverride, creds))
+	client, err := newAPIClient(resolveServer(*server, creds), tokenValue)
 	if err != nil {
 		return err
 	}
@@ -87,7 +88,7 @@ func runWritebackSweepDrafts(args []string, stdout io.Writer) error {
 	var result sweepDraftsResult
 	if err := client.postJSON(
 		context.Background(),
-		fmt.Sprintf("/v1/workspaces/%s/writeback/sweep-drafts", url.PathEscape(record.ID)),
+		fmt.Sprintf("/v1/workspaces/%s/writeback/sweep-drafts", url.PathEscape(workspaceID)),
 		requestBody,
 		&result,
 	); err != nil {

cmd/relayfile-cli/writeback_sweep_test.go

@@ -128,6 +128,31 @@ func TestWritebackSweepDraftsApplyAndJSON(t *testing.T) {
 	}
 }
 
+func TestWritebackSweepDraftsUsesTokenWorkspaceWithoutPositionalArg(t *testing.T) {
+	overrideToken := testJWTWithWorkspace("rw_token")
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/v1/workspaces/rw_token/writeback/sweep-drafts" {
+			t.Fatalf("unexpected request path: %s", r.URL.Path)
+		}
+		if got := r.Header.Get("Authorization"); got != "Bearer "+overrideToken {
+			t.Fatalf("unexpected Authorization: %q", got)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"dryRun": true, "scanned": 0, "removed": [], "skipped": []}`))
+	}))
+	defer server.Close()
+
+	setupSweepWorkspace(t, server.URL)
+	var stdout bytes.Buffer
+	err := run(
+		[]string{"writeback", "sweep-drafts", "--token", overrideToken},
+		strings.NewReader(""), &stdout, &stdout,
+	)
+	if err != nil {
+		t.Fatalf("run writeback sweep-drafts with token override failed: %v", err)
+	}
+}
+
 func TestWritebackSweepDraftsRejectsExtraArgs(t *testing.T) {
 	t.Setenv("HOME", t.TempDir())
 	clearRelayfileEnv(t)
@@ -137,3 +162,11 @@ func TestWritebackSweepDraftsRejectsExtraArgs(t *testing.T) {
 		t.Fatalf("expected usage error, got %v", err)
 	}
 }
+
+func TestWritebackMissingSubcommandMentionsSweepDrafts(t *testing.T) {
+	var stdout bytes.Buffer
+	err := run([]string{"writeback"}, strings.NewReader(""), &stdout, &stdout)
+	if err == nil || !strings.Contains(err.Error(), "sweep-drafts") {
+		t.Fatalf("expected missing subcommand error to mention sweep-drafts, got %v", err)
+	}
+}

internal/httpapi/server.go

@@ -2208,9 +2208,12 @@ func (s *Server) handleWritebackAck(w http.ResponseWriter, r *http.Request, work
 	}
 
 	ack := relayfile.WritebackAck{}
-	if ok, ok2 := payload["success"].(bool); ok2 {
-		ack.Success = ok
+	success, ok := payload["success"].(bool)
+	if !ok {
+		writeError(w, http.StatusBadRequest, "bad_request", "success must be a boolean", correlationID)
+		return
 	}
+	ack.Success = success
 	if e, ok := payload["error"].(string); ok {
 		ack.Error = e
 	}

internal/httpapi/server_test.go

@@ -5847,6 +5847,54 @@ func TestWritebackAckWithExternalIDReconcilesDraft(t *testing.T) {
 	}
 }
 
+func TestWritebackAckRejectsMissingSuccess(t *testing.T) {
+	store := relayfile.NewStoreWithOptions(relayfile.StoreOptions{ExternalWritebackMode: true})
+	defer store.Close()
+	server := NewServer(store)
+	token := mustTestJWT(t, "dev-secret", "ws_1", "Worker1", []string{"fs:write", "sync:read", "sync:trigger"}, time.Now().Add(time.Hour))
+
+	writeResp := doRequest(t, server, request{
+		method: http.MethodPut,
+		path:   "/v1/workspaces/ws_1/fs/file?path=" + url.QueryEscape("/slack/channels/C0ALQ06AAUT/messages/messages 0e89a031-65f0-480e-a823-ab1d94b324ea.json"),
+		headers: map[string]string{
+			"Authorization":    "Bearer " + token,
+			"X-Correlation-Id": "corr_write_1",
+			"If-Match":         "0",
+		},
+		body: map[string]any{
+			"contentType": "application/json",
+			"content":     `{"text":"hi"}`,
+		},
+	})
+	if writeResp.Code != http.StatusAccepted {
+		t.Fatalf("expected 202 on draft write, got %d (%s)", writeResp.Code, writeResp.Body.String())
+	}
+	var writeResult relayfile.WriteResult
+	if err := json.NewDecoder(writeResp.Body).Decode(&writeResult); err != nil {
+		t.Fatalf("failed to decode write response: %v", err)
+	}
+
+	ackResp := doRequest(t, server, request{
+		method: http.MethodPost,
+		path:   "/v1/workspaces/ws_1/writeback/" + writeResult.OpID + "/ack",
+		headers: map[string]string{
+			"Authorization":    "Bearer " + token,
+			"X-Correlation-Id": "corr_ack_1",
+		},
+		body: map[string]any{"error": "missing success"},
+	})
+	if ackResp.Code != http.StatusBadRequest {
+		t.Fatalf("expected 400 on malformed ack, got %d (%s)", ackResp.Code, ackResp.Body.String())
+	}
+	ops, err := store.ListOperations("ws_1", "", "", "", "", 10)
+	if err != nil {
+		t.Fatalf("list operations failed: %v", err)
+	}
+	if len(ops.Items) != 1 || ops.Items[0].Status != "pending" {
+		t.Fatalf("malformed ack must leave operation pending, got %+v", ops.Items)
+	}
+}
+
 // Issue #242: POST /writeback/sweep-drafts drains accumulated draft residue.
 func TestWritebackSweepDraftsEndpoint(t *testing.T) {
 	store := relayfile.NewStoreWithOptions(relayfile.StoreOptions{ExternalWritebackMode: true})

internal/relayfile/draft_reconcile.go

@@ -114,6 +114,9 @@ func isDraftFileBasename(basename string) bool {
 // enqueueWriteback, so the mutation cannot create operations or writebacks.
 func (s *Store) reconcileAckedDraftLocked(workspaceID string, ws *workspaceState, op OperationStatus, ack WritebackAck, correlationID string) DraftDisposition {
 	none := DraftDisposition{Action: "none"}
+	if ws.ProviderIndex == nil {
+		ws.ProviderIndex = map[string]string{}
+	}
 	externalID := strings.TrimSpace(ack.ExternalID)
 	if externalID == "" {
 		return none
@@ -290,6 +293,9 @@ func (s *Store) SweepWritebackDrafts(workspaceID string, req SweepDraftsRequest)
 		if _, err := path.Match(pattern, "probe.json"); err != nil {
 			return SweepDraftsResult{}, ErrInvalidInput
 		}
+		if strings.ContainsAny(pattern, `/\`) {
+			return SweepDraftsResult{}, ErrInvalidInput
+		}
 	}
 
 	s.mu.Lock()

internal/relayfile/draft_reconcile_test.go

@@ -127,6 +127,25 @@ func TestAckWithExternalIDRenamesDraftToCanonicalID(t *testing.T) {
 	}
 }
 
+func TestAckWithExternalIDInitializesLegacyNilProviderIndex(t *testing.T) {
+	store := newExternalStore(t)
+	draftPath := "/slack/channels/C0ALQ06AAUT/messages/messages " + draftUUIDA + ".json"
+	result := writeDraft(t, store, "ws_1", draftPath, `{"text":"hi"}`)
+	store.mu.Lock()
+	store.workspaces["ws_1"].ProviderIndex = nil
+	store.mu.Unlock()
+
+	if _, err := store.AcknowledgeWriteback("ws_1", result.OpID, WritebackAck{
+		Success:    true,
+		ExternalID: "1780018871.351819",
+	}, "corr_ack_1"); err != nil {
+		t.Fatalf("ack with nil provider index failed: %v", err)
+	}
+	if _, err := store.ReadFile("ws_1", "/slack/channels/C0ALQ06AAUT/messages/1780018871.351819.json"); err != nil {
+		t.Fatalf("expected canonical-id file after ack: %v", err)
+	}
+}
+
 func TestAckRenameIsClassificationExempt(t *testing.T) {
 	store := newExternalStore(t)
 	draftPath := "/slack/channels/C0ALQ06AAUT/messages/messages " + draftUUIDA + ".json"
@@ -620,6 +639,15 @@ func TestSweepRejectsBlankWorkspace(t *testing.T) {
 	}
 }
 
+func TestSweepRejectsPatternsWithPathSeparators(t *testing.T) {
+	store := newExternalStore(t)
+	for _, pattern := range []string{"drafts/*.json", `drafts\*.json`} {
+		if _, err := store.SweepWritebackDrafts("ws_1", SweepDraftsRequest{Patterns: []string{pattern}}); err != ErrInvalidInput {
+			t.Fatalf("expected ErrInvalidInput for pattern %q, got %v", pattern, err)
+		}
+	}
+}
+
 func TestDraftBasenameDetection(t *testing.T) {
 	cases := []struct {
 		path  string

packages/sdk/typescript/src/index.ts

@@ -111,6 +111,7 @@ export type {
 export type {
   AckResponse,
   AckWritebackInput,
+  AckWritebackDraftDisposition,
   AckWritebackResponse,
   AdminIngressAlert,
   AdminIngressAlertProfile,
@@ -199,6 +200,8 @@ export type {
   SyncProviderStatusState,
   SyncRefreshRequest,
   SyncStatusResponse,
+  SweepWritebackDraftsInput,
+  SweepWritebackDraftsResponse,
   TreeEntry,
   TreeResponse,
   WritebackActionType,

packages/sdk/typescript/src/types.test.ts

@@ -3,6 +3,9 @@ import type {
   DigestHandler,
   DigestSection,
   LayoutManifest,
+  SweepWritebackDraftsInput,
+  SweepWritebackDraftsResponse,
+  AckWritebackDraftDisposition,
   WritebackDeadLetterError,
   WritebackDeadLetterErrorCode,
   WritebackItem,
@@ -86,6 +89,30 @@ describe("workspace primitive public types", () => {
     expect(item.state).toBe("dead");
   });
 
+  it("exports writeback ack and sweep public types", () => {
+    const draft = {
+      action: "renamed",
+      from: "/slack/channels/C/messages/messages 0e89a031-65f0-480e-a823-ab1d94b324ea.json",
+      to: "/slack/channels/C/messages/1780018871.351819.json"
+    } satisfies AckWritebackDraftDisposition;
+    const input = {
+      workspaceId: "ws_acme",
+      pathPrefix: "/slack/channels/C",
+      patterns: ["wb-*.json"],
+      apply: false
+    } satisfies SweepWritebackDraftsInput;
+    const response = {
+      dryRun: true,
+      scanned: 1,
+      removed: [{ path: draft.from, reason: "space-uuid-draft" }],
+      skipped: []
+    } satisfies SweepWritebackDraftsResponse;
+
+    expectTypeOf(input).toMatchTypeOf<SweepWritebackDraftsInput>();
+    expectTypeOf(response).toMatchTypeOf<SweepWritebackDraftsResponse>();
+    expect(response.removed[0]?.path).toBe(draft.from);
+  });
+
   it("accepts digest handlers that return null or a section", async () => {
     const noActivity: DigestHandler = async () => null;
     const withActivity: DigestHandler = async (ctx) => ({