Skip to content

AR-272 add direct relayfile writeback push#282

Merged
khaliqgant merged 4 commits into
mainfrom
ar-272-cli-writeback-fastpath
Jun 15, 2026
Merged

AR-272 add direct relayfile writeback push#282
khaliqgant merged 4 commits into
mainfrom
ar-272-cli-writeback-fastpath

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Jun 15, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds relayfile writeback push <local-path> to send local mount writebacks directly to the cloud /fs/bulk endpoint and record pending/acked/failed receipts in the local outbox audit trail.
  • Adds relayfile workspace status with local sync health, stuck-event count from incrementalReadNotReady state, outbox pending/failed/acked counts, and last error.
  • Adds content-identity dedupe for factory-create/draft writebacks in both CLI push and mountsync bulk writes so a live mount-sync daemon and explicit push converge on the same cloud-side operation.

Scope

This is PR-A for AR-272. It intentionally does not include PR-B fresh-404 cursor drain / skip-stuck or PR-C by-state emitter retraction.

Reviewer Notes

  • Credential path: writeback push bootstraps delegated relayfile credentials through the Agent Relay cloud session, not legacy cloud-credentials.json. The CLI test writes a stale legacy credential file and verifies the cloud session token is used.
  • Scope path: the CLI asks the cloud delegated-token endpoint for provider-scoped join access like fs:write:/linear/**; the returned delegated bundle is required to include the compiled relayfile scope relayfile:fs:write:/linear/** before the push proceeds.
  • Double-dispatch handling: dedupe is cloud-side via content identity mount-writeback-create-draft keyed by workspace, remote path, and content hash for factory-create-*.json and existing draft-path writebacks.
  • Fixtures are synthetic; this PR does not dispatch the operator's real pending factory-create files.

Tests

  • go test ./cmd/relayfile-cli ./internal/mountsync

E2E

Not run against real Linear from this development environment. Direct push, cloud-session credential selection, provider-scoped delegated-token minting, operation polling, outbox receipt writing, and status reporting are covered with synthetic fixtures and fake cloud/relayfile servers.

Review in cubic

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@khaliqgant, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 59 minutes and 46 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 631923f7-b2eb-4160-b7e7-df0868290129

📥 Commits

Reviewing files that changed from the base of the PR and between d96f1d4 and d9c0763.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • cmd/relayfile-cli/main.go
  • cmd/relayfile-cli/main_test.go
  • internal/mountsync/syncer.go
  • internal/mountsync/syncer_test.go
📝 Walkthrough

Walkthrough

Adds five webhook management methods (registerWebhook, listWebhooks, deleteWebhook, getWebhookDeadLetters, replayWebhookDeadLetter) to the TypeScript SDK with matching types and tests. Extends the Go CLI with writeback push and workspace status subcommands, expands the mountsync WriteResult struct, and broadens draft-path detection to include factory-create-*.json filenames. Adds agent trajectory log files for the PR review.

Changes

TypeScript SDK Webhook Methods

Layer / File(s) Summary
Webhook and DLQ type definitions
packages/sdk/typescript/src/types.ts, packages/sdk/typescript/src/index.ts
Adds contentHash to FilesystemEvent, defines ten new webhook/DLQ interfaces (WebhookSubscription, RegisterWebhookInput/Response, list/delete options, DLQ feed/item shapes), and re-exports them from the public index.
RelayFileClient webhook methods
packages/sdk/typescript/src/client.ts, packages/sdk/typescript/src/client.test.ts
Imports webhook types and adds five public methods wired to their HTTP endpoints under /v1/workspaces/{workspaceId}/webhooks and /webhooks/dlq, with Vitest coverage for URL patterns, HTTP methods, request bodies, and query parameters.

Go CLI writeback push and workspace status

Layer / File(s) Summary
WriteResult expansion and factory-create draft detection
internal/mountsync/syncer.go, internal/mountsync/syncer_test.go
Adds opId, status, and a nested writeback object to WriteResult; introduces isMountWritebackCreateDraftPath to recognize factory-create-*.json as draft-eligible; extends the draft-path test.
CLI data models
cmd/relayfile-cli/main.go
Extends bulk-write request/response structs with contentIdentity and per-file result metadata; expands syncStateFile with lastSuccessfulReconcileAt, structured lastError, and incrementalReadNotReadySince; adds the statusError type.
CLI subcommand dispatch and help text
cmd/relayfile-cli/main.go
Updates workspace and writeback help text to document status and push, and wires both into their dispatch switch statements.
writeback push implementation
cmd/relayfile-cli/main.go, cmd/relayfile-cli/main_test.go
Adds runWritebackPush with path resolution, content encoding, content hash, contentIdentity derivation, delegated credential scoping, deterministic command ID, receipt persistence in .relay/outbox/{pending,failed,acked}, bulk-write dispatch, op-completion polling, and JSON/human output; covered by TestWritebackPushPostsBulkAndWritesAckedReceipt.
workspace status implementation
cmd/relayfile-cli/main.go, cmd/relayfile-cli/main_test.go
Adds runWorkspaceStatus and workspaceHealthReport builder: reads mirror state, inspects cursor health, counts outbox receipt files, and renders human or JSON output; covered by TestWorkspaceStatusReportsLocalMountHealth.

Agent Trajectory Logs

Layer / File(s) Summary
Trajectory log files
.agentworkforce/trajectories/active/..., .trajectories/completed/2026-06/*, .trajectories/index.json
Adds active trajectory traj_dzjkresnewd3, completed trajectory traj_2ldfbem11vzl (JSON + markdown), and updates the trajectory index.

Sequence Diagram(s)

sequenceDiagram
  participant CLI as relayfile-cli
  participant Outbox as .relay/outbox
  participant AuthAPI as Delegated Token API
  participant DataPlane as Bulk Write API
  participant OpAPI as Op Status API

  rect rgba(70, 130, 180, 0.5)
    note over CLI,Outbox: Receipt lifecycle
    CLI->>Outbox: write pending receipt (commandId)
    CLI->>AuthAPI: POST bootstrap delegated token
    AuthAPI-->>CLI: delegated credentials
  end

  rect rgba(60, 179, 113, 0.5)
    note over CLI,DataPlane: Bulk write
    CLI->>DataPlane: POST /bulk-write (content + contentIdentity)
    DataPlane-->>CLI: WriteResult (opId, status, writeback.state)
  end

  rect rgba(210, 105, 30, 0.5)
    note over CLI,Outbox: Polling and receipt finalization
    CLI->>OpAPI: GET op/{opId}/status (poll)
    OpAPI-->>CLI: completed
    CLI->>Outbox: move receipt → acked (or failed on error)
    CLI-->>CLI: emit JSON or human output
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • AgentWorkforce/relayfile#253: Modifies the same internal/mountsync/syncer.go writeback draft contentIdentity detection logic that this PR extends with factory-create-*.json path recognition.
  • AgentWorkforce/relayfile#281: Touches the same TypeScript SDK webhook surface (client.ts, types.ts, index.ts, client.test.ts) that this PR extends with webhook DLQ and subscription management methods.

Poem

🐇 Hoppity-hop, new webhooks today,
DLQ items replayed without delay.
A writeback push writes the file with care,
Receipts in outbox — pending, acked — they're there.
workspace status shows the mirror's health bright,
This bunny approves — the code looks just right! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'AR-272 add direct relayfile writeback push' directly describes the main feature added: a writeback push command for the relayfile CLI.
Description check ✅ Passed The description clearly explains the PR's objectives, including the writeback push command, workspace status command, content-identity deduplication, and credential handling, all of which are reflected in the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ar-272-cli-writeback-fastpath

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces new CLI subcommands, including writeback push for pushing local files to a remote workspace and workspace status for reporting workspace health. It also adds TypeScript SDK support for webhook management and dead-letter queues, along with content identity tracking for draft creation. The review feedback suggests ensuring cross-platform correctness on Windows by using path.Base instead of filepath.Base for remote paths, and refactoring the writeback polling mechanism to accept and respect a context.Context for robust timeout and cancellation handling.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread cmd/relayfile-cli/main.go
Comment on lines +3244 to +3248
func isWritebackDraftPath(remotePath string) bool {
base := filepath.Base(normalizeWritebackFailurePath(remotePath))
return strings.HasPrefix(base, "factory-create-") && strings.HasSuffix(base, ".json") ||
relayfile.IsDraftFilePath(remotePath)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

On Windows, filepath.Base expects backslash (\\) as the path separator. Since remotePath is a slash-separated remote path, filepath.Base will fail to correctly extract the base name on Windows (it will return the entire path instead of just the filename).

To ensure cross-platform correctness, use path.Base instead of filepath.Base for remote paths, matching the pattern used in internal/mountsync/syncer.go.

Suggested change
func isWritebackDraftPath(remotePath string) bool {
base := filepath.Base(normalizeWritebackFailurePath(remotePath))
return strings.HasPrefix(base, "factory-create-") && strings.HasSuffix(base, ".json") ||
relayfile.IsDraftFilePath(remotePath)
}
func isWritebackDraftPath(remotePath string) bool {
base := path.Base(normalizeWritebackFailurePath(remotePath))
return strings.HasPrefix(base, "factory-create-") && strings.HasSuffix(base, ".json") ||
relayfile.IsDraftFilePath(remotePath)
}

Comment thread cmd/relayfile-cli/main.go
Comment on lines 5 to 11
"bytes"
"context"
"crypto/rand"
"crypto/sha256"
"encoding/base64"
"encoding/hex"
"encoding/json"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Import the path package to support cross-platform remote path manipulation. Remote paths in this codebase are slash-separated, and using the standard path package is the established pattern for handling them correctly on all operating systems (including Windows).

Suggested change
"bytes"
"context"
"crypto/rand"
"crypto/sha256"
"encoding/base64"
"encoding/hex"
"encoding/json"
"bytes"
"context"
"crypto/rand"
"crypto/sha256"
"encoding/base64"
"encoding/hex"
"encoding/json"
"path"

Comment thread cmd/relayfile-cli/main.go
Comment on lines +3015 to +3033
if opID != "" {
op, err := waitForWritebackOperation(commandClient, opID, *timeout)
if err != nil {
failed := pendingReceipt
failed.OpID = opID
failed.Status = "failed"
failed.LastAttemptAt = time.Now().UTC().Format(time.RFC3339Nano)
failed.AttemptCount = 1
failed.LastError = sanitizeCLIReceiptError(err)
failed.DispatchStatus = "failed"
failed.CorrelationID = firstNonBlank(response.CorrelationID, failed.CorrelationID)
if receiptErr := writeWritebackPushReceipt(resolved.MountRoot, failed); receiptErr != nil {
return fmt.Errorf("%w; additionally failed to write failure receipt: %v", err, receiptErr)
}
return err
}
revision = firstNonBlank(revision, op.Revision)
dispatchStatus = firstNonBlank(op.Status, dispatchStatus)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update the call site of waitForWritebackOperation to pass a context with the configured timeout. This allows the polling loop to respect the timeout and handle cancellations gracefully.

	if opID != "" {
		ctx, cancel := context.WithTimeout(context.Background(), *timeout)
		defer cancel()
		op, err := waitForWritebackOperation(ctx, commandClient, opID)
		if err != nil {
			failed := pendingReceipt
			failed.OpID = opID
			failed.Status = "failed"
			failed.LastAttemptAt = time.Now().UTC().Format(time.RFC3339Nano)
			failed.AttemptCount = 1
			failed.LastError = sanitizeCLIReceiptError(err)
			failed.DispatchStatus = "failed"
			failed.CorrelationID = firstNonBlank(response.CorrelationID, failed.CorrelationID)
			if receiptErr := writeWritebackPushReceipt(resolved.MountRoot, failed); receiptErr != nil {
				return fmt.Errorf("%w; additionally failed to write failure receipt: %v", err, receiptErr)
			}
			return err
		}
		revision = firstNonBlank(revision, op.Revision)
		dispatchStatus = firstNonBlank(op.Status, dispatchStatus)
	}

Comment thread cmd/relayfile-cli/main.go Outdated
Comment on lines +3064 to +3092
func waitForWritebackOperation(commandClient *workspaceCommandClient, opID string, timeout time.Duration) (writebackOperationStatus, error) {
if strings.TrimSpace(opID) == "" {
return writebackOperationStatus{Status: "succeeded"}, nil
}
if timeout <= 0 {
timeout = 90 * time.Second
}
deadline := time.Now().Add(timeout)
for {
var op writebackOperationStatus
err := commandClient.getWorkspaceJSON(context.Background(), func(workspaceID string) string {
return fmt.Sprintf("/v1/workspaces/%s/ops/%s", url.PathEscape(workspaceID), url.PathEscape(opID))
}, &op)
if err != nil {
return op, err
}
status := strings.TrimSpace(op.Status)
switch status {
case "succeeded":
return op, nil
case "failed", "dead_lettered", "canceled":
return op, fmt.Errorf("writeback operation %s %s", opID, status)
}
if time.Now().After(deadline) {
return op, fmt.Errorf("timed out waiting for writeback operation %s", opID)
}
time.Sleep(500 * time.Millisecond)
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The polling loop in waitForWritebackOperation currently uses context.Background() for HTTP requests and a hardcoded time.Sleep(500 * time.Millisecond). This has two drawbacks:

  1. It does not respect context cancellation or timeouts if the CLI is interrupted (e.g., via Ctrl+C).
  2. Any transient network error during polling immediately aborts the entire push operation.

By passing a context.Context to waitForWritebackOperation, we can use it for the HTTP requests, handle timeouts/cancellations gracefully via select, and make the polling resilient to transient network glitches by continuing the loop on transient errors until the context is done.

func waitForWritebackOperation(ctx context.Context, commandClient *workspaceCommandClient, opID string) (writebackOperationStatus, error) {
	if strings.TrimSpace(opID) == "" {
		return writebackOperationStatus{Status: "succeeded"}, nil
	}
	for {
		var op writebackOperationStatus
		err := commandClient.getWorkspaceJSON(ctx, func(workspaceID string) string {
			return fmt.Sprintf("/v1/workspaces/%s/ops/%s", url.PathEscape(workspaceID), url.PathEscape(opID))
		}, &op)
		if err == nil {
			status := strings.TrimSpace(op.Status)
			switch status {
			case "succeeded":
				return op, nil
			case "failed", "dead_lettered", "canceled":
				return op, fmt.Errorf("writeback operation %s %s", opID, status)
			}
		}
		select {
		case <-ctx.Done():
			if err != nil {
				return op, fmt.Errorf("timed out waiting for writeback operation %s: %w", opID, err)
			}
			return op, ctx.Err()
		case <-time.After(500 * time.Millisecond):
		}
	}
}

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6f5eb1eb86

ℹ️ 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".

Comment thread cmd/relayfile-cli/main.go Outdated
return []string{"fs:write:/**"}
}
provider := strings.TrimSpace(parts[0])
return []string{fmt.Sprintf("fs:write:/%s/**", provider)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add ops:read to writeback push credentials

With the default delegated-auth path this requests a token scoped only to fs:write:/<provider>/**, but the same command later polls GET /v1/workspaces/{workspaceId}/ops/{opId} whenever bulk write returns an op id. I checked the workspace route switch in internal/httpapi/server.go, where that ops route requires ops:read, so a normal delegated push can successfully write the file and then fail/record a failed receipt on the status poll. Please request/validate an ops:read relayfile scope for this command before waiting on the operation.

Useful? React with 👍 / 👎.

async registerWebhook(input: RegisterWebhookInput): Promise<RegisterWebhookResponse> {
return this.request<RegisterWebhookResponse>({
method: "POST",
path: `/v1/workspaces/${encodeURIComponent(input.workspaceId)}/webhooks`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove or implement unhandled webhook routes

These new SDK methods point at /v1/workspaces/{id}/webhooks and related /webhooks/dlq routes, but I checked the service route switch in internal/httpapi/server.go and the only workspace webhook route currently handled is /webhooks/ingest; the authoritative OpenAPI file also has no subscription or DLQ paths. SDK consumers calling registerWebhook, listWebhooks, deleteWebhook, or the DLQ helpers against this service will therefore get 404s, so the server/OpenAPI contract needs to be added before exporting these methods.

Useful? React with 👍 / 👎.

agent-relay-code Bot added a commit that referenced this pull request Jun 15, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

Open in Devin Review

Comment thread cmd/relayfile-cli/main.go
Comment on lines +4434 to +4451
for _, path := range []string{
filepath.Join(localDir, ".relayfile-mount-state.json"),
filepath.Join(localDir, mountsync.DefaultMountStateDirName, "state.json"),
} {
payload, err := os.ReadFile(path)
if err != nil {
continue
}
var state struct {
IncrementalReadNotReadySince map[string]string `json:"incrementalReadNotReadySince"`
IncrementalBacklogDraining bool `json:"incrementalBacklogDraining"`
}
if json.Unmarshal(payload, &state) != nil {
continue
}
return len(state.IncrementalReadNotReadySince), state.IncrementalBacklogDraining
}
return 0, false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 readLocalMountCursorHealth second path may never exist in practice

The function checks filepath.Join(localDir, mountsync.DefaultMountStateDirName, "state.json") which resolves to localDir/.relayfile-mount-state/state.json. However, DefaultMountStateDir() at internal/mountsync/state_path.go:43-48 returns $HOME/.relayfile-mount-state/ (not under localDir). The private mount state directory is typically NOT a subdirectory of the local mirror. The first path (localDir/.relayfile-mount-state.json) is the legacy location where the syncer writes internal state and is the one that will match in practice. The second path gracefully falls through via continue but may be dead code.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +1871 to +1938
async registerWebhook(input: RegisterWebhookInput): Promise<RegisterWebhookResponse> {
return this.request<RegisterWebhookResponse>({
method: "POST",
path: `/v1/workspaces/${encodeURIComponent(input.workspaceId)}/webhooks`,
correlationId: input.correlationId,
body: {
url: input.url,
pathGlobs: input.pathGlobs,
secret: input.secret
},
signal: input.signal
});
}

async listWebhooks(
workspaceId: string,
options: ListWebhooksOptions = {}
): Promise<WebhookSubscription[]> {
return this.request<WebhookSubscription[]>({
method: "GET",
path: `/v1/workspaces/${encodeURIComponent(workspaceId)}/webhooks`,
correlationId: options.correlationId,
signal: options.signal
});
}

async deleteWebhook(
workspaceId: string,
subscriptionId: string,
options: DeleteWebhookOptions = {}
): Promise<void> {
await this.performRequest({
method: "DELETE",
path: `/v1/workspaces/${encodeURIComponent(workspaceId)}/webhooks/${encodeURIComponent(subscriptionId)}`,
correlationId: options.correlationId,
signal: options.signal
});
}

async getWebhookDeadLetters(
workspaceId: string,
options: GetWebhookDeadLettersOptions = {}
): Promise<WebhookDeliveryDeadLetterFeedResponse> {
const query = buildQuery({
cursor: options.cursor,
limit: options.limit
});
return this.request<WebhookDeliveryDeadLetterFeedResponse>({
method: "GET",
path: `/v1/workspaces/${encodeURIComponent(workspaceId)}/webhooks/dlq${query}`,
correlationId: options.correlationId,
signal: options.signal
});
}

async replayWebhookDeadLetter(
workspaceId: string,
deliveryId: string,
correlationId?: string,
signal?: AbortSignal
): Promise<QueuedResponse> {
return this.request<QueuedResponse>({
method: "POST",
path: `/v1/workspaces/${encodeURIComponent(workspaceId)}/webhooks/dlq/${encodeURIComponent(deliveryId)}/replay`,
correlationId,
signal
});
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 SDK webhook methods assume server-side endpoints exist without OpenAPI verification

The TypeScript SDK adds registerWebhook, listWebhooks, deleteWebhook, getWebhookDeadLetters, and replayWebhookDeadLetter methods targeting endpoints like POST /v1/workspaces/{id}/webhooks and GET /v1/workspaces/{id}/webhooks/dlq. These are client-side additions only — there are no corresponding changes to internal/httpapi/server.go or openapi/relayfile-v1.openapi.yaml in this PR. This is fine if these endpoints already exist server-side and the SDK is catching up, but if they don't, these methods will fail at runtime.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@agent-relay-code

Copy link
Copy Markdown
Contributor

Findings

  1. packages/sdk/typescript/src/client.ts:1874 adds SDK calls for /v1/workspaces/{workspaceId}/webhooks, /webhooks/{id}, and /webhooks/dlq, but the checked-out server router only handles /webhooks/ingest at internal/httpapi/server.go:228, and the OpenAPI spec only documents /webhooks/ingest at openapi/relayfile-v1.openapi.yaml:1577. These new SDK methods will 404 against the current service unless the server/API contract is added in the same PR.

  2. cmd/relayfile-cli/main.go:3016 persists an accepted writeback operation as failed when operation polling errors or times out, then removes the pending receipt at cmd/relayfile-cli/main.go:3289. A timeout after receiving an opId does not prove the provider operation failed; it may still complete. This needs a human-authored behavior decision, likely preserving pending/unknown state instead of recording failure.

Addressed Comments

  • Devin: packages/relayfile-sdk/tsconfig.json test exclusion removal. Stale/current path differs; current packages/sdk/typescript/tsconfig.json excludes src/**/*.test.ts.
  • Devin: packages/relayfile-sdk/src/types.ts missing BulkWriteResponse.correlationId. Stale/current path differs; current packages/sdk/typescript/src/types.ts includes correlationId.
  • Devin: internal/mountfuse/wsinvalidate.go:134 endpoint concern. Outside this PR diff; current internal/mountfuse/wsinvalidate.go uses /v1/workspaces/%s/fs/ws.
  • Devin: internal/mountfuse/dir.go:130 create/open return concern. Outside this PR diff; current internal/mountfuse/dir.go returns fuse.FOPEN_KEEP_CACHE.
  • Devin: internal/mountfuse/file.go:248 locking concern. Outside this PR diff; current internal/mountfuse/file.go locks before updating handle fields.
  • Devin: internal/mountfuse/file.go:225 operation ordering concern. Outside this PR diff; current internal/mountfuse/file.go invalidates, stores, then updates buffer.
  • Devin: packages/core/src/webhooks.ts:387 webhook handling concern. Outside this PR diff; left unchanged as out of scope.
  • Devin: internal/mountfuse/wsinvalidate.go:71 reconnect/backoff concern. Outside this PR diff; left unchanged as out of scope.
  • Devin: resolved duplicate internal/mountfuse/dir.go:130. Confirmed current code at internal/mountfuse/dir.go.
  • Devin: resolved duplicate internal/mountfuse/wsinvalidate.go:134. Confirmed current code at internal/mountfuse/wsinvalidate.go.
  • Devin: resolved duplicate internal/mountfuse/file.go:248. Confirmed current code at internal/mountfuse/file.go.
  • Devin: resolved duplicate internal/mountfuse/file.go:225. Confirmed current code at internal/mountfuse/file.go.
  • Devin: internal/mountfuse/file.go:231 buffer/error handling concern. Outside this PR diff; left unchanged as out of scope.
  • Devin: internal/mountfuse/wsinvalidate.go:133 endpoint concern. Outside this PR diff; current endpoint verified at internal/mountfuse/wsinvalidate.go.

Verification

  • scripts/check-contract-surface.sh: passed.
  • npm run typecheck --workspace=packages/sdk/typescript: passed.
  • npm run test --workspace=packages/sdk/typescript: passed on rerun.
  • npm run build: TS packages passed, then failed because go is not installed for CLI binary build.
  • npm run typecheck: TS checks passed, then failed because go is not installed for go vet ./....
  • npm run test: core passed; first SDK run hit a flaky WebSocket test, and SDK passed on rerun. Full root test cannot complete here because Go tooling is absent.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/relayfile-cli/main.go`:
- Around line 3016-3029: The error handling in the waitForWritebackOperation
call block incorrectly marks operations as failed when encountering timeout or
transient GET errors. Instead of immediately writing a failed receipt when
waitForWritebackOperation returns an error, you must distinguish between
transient errors (timeout, network issues) and terminal operation states. Only
write a failed receipt for confirmed terminal states like failed, dead_lettered,
or canceled; for timeout or network errors, leave the receipt as pending with
the opID, lastError set to the sanitized error, and a needsAttention flag set to
true. This ensures the local outbox state remains consistent with the cloud
operation state when the final status is unknown. Apply the same logic to the
sibling location at lines 3064-3091, ensuring both sites handle transient vs
terminal errors consistently by checking the operation's actual status before
writing the receipt.
- Around line 4412-4421: The stuck event count is currently only being read from
private cursor files via the readLocalMountCursorHealth function, but the public
sync state field IncrementalReadNotReadySince should be checked first. Modify
the code to read the stuck event count from state.IncrementalReadNotReadySince
as the baseline, then merge or take the maximum of that value with the result
from readLocalMountCursorHealth to ensure the report captures stuck events from
both the public state and private cursor health data.
- Around line 2853-2875: The writebackPushReceipt struct includes Content and
Encoding fields that expose user file contents in persisted and printed receipts
(acked/failed receipts and JSON output), creating a security/privacy risk. Clear
the Content and Encoding fields to empty strings in the writebackPushReceipt
struct before writing acked/failed receipts to disk or outputting as JSON, while
keeping these fields populated only for transient pending receipts. Apply this
redaction at the locations referenced in the comment (lines 3046-3047 and 3286)
where acked/failed receipts are persisted or printed, ensuring that user file
contents are not retained beyond the writeback operation itself.
- Around line 4390-4394: The resolveWorkspaceLikeStatus function currently falls
through to loadCredentials when called with an empty workspace value, but this
fails in Agent Relay scenarios where credentials.json is unavailable. Modify the
workspace resolution logic to check and resolve the workspace in this order
before falling back to legacy credentials: first check the RELAYFILE_WORKSPACE
environment variable, then check for a catalog default workspace, then check for
the active Agent Relay workspace, and only call loadCredentials if none of those
provide a valid local record. This ensures resolveWorkspaceLikeStatus handles
the empty input case by attempting these modern resolution methods before
attempting the legacy credentials fallback.
- Around line 3133-3139: The file validation in the writebackPushResolvedPath
resolution logic only checks if the path is a directory, but allows special
files like FIFOs, device nodes, and sockets which can cause os.ReadFile to hang
or read unintended data. After calling os.Stat and obtaining the info object,
replace the directory check with a validation that requires the file to be a
regular file by checking info.Mode().IsRegular(), which will reject both
directories and all special file types in a single check.
- Around line 3188-3200: The joinRemotePath function produces double slashes in
paths when the normalized root ends with a slash (e.g., `/linear/` concatenated
with `rel` becomes `/linear//file.json`). After calling
normalizeWritebackFailurePath on the root parameter, trim any trailing slashes
from root using strings.TrimSuffix before the logic that checks if root is empty
or constructs the final path. This ensures that the concatenation at the end of
the function (when returning root + "/" + rel or "/" + rel) never produces
double slashes.
- Around line 3094-3101: The ensureWritebackDelegatedCredentials function
validates that an existing delegated bundle has the required
requiredRelayfileScopes before returning, but the bootstrap path does not
perform the same validation after bootstrapDelegatedCredentialsFromAgentRelay
returns. After calling bootstrapDelegatedCredentialsFromAgentRelay to obtain a
new delegated bundle, you must verify that the returned bundle satisfies the
scope requirements by using the delegatedBundleHasScopes check before returning
success. If the bootstrapped bundle lacks the required scopes, the function
should return an appropriate error instead of proceeding.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b66b34cf-8a7c-4224-8262-538434b84770

📥 Commits

Reviewing files that changed from the base of the PR and between fdfe017 and d96f1d4.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (12)
  • .agentworkforce/trajectories/active/traj_dzjkresnewd3/trajectory.json
  • .trajectories/completed/2026-06/traj_2ldfbem11vzl.json
  • .trajectories/completed/2026-06/traj_2ldfbem11vzl.md
  • .trajectories/index.json
  • cmd/relayfile-cli/main.go
  • cmd/relayfile-cli/main_test.go
  • internal/mountsync/syncer.go
  • internal/mountsync/syncer_test.go
  • packages/sdk/typescript/src/client.test.ts
  • packages/sdk/typescript/src/client.ts
  • packages/sdk/typescript/src/index.ts
  • packages/sdk/typescript/src/types.ts

Comment thread cmd/relayfile-cli/main.go
Comment thread cmd/relayfile-cli/main.go Outdated
Comment thread cmd/relayfile-cli/main.go Outdated
Comment thread cmd/relayfile-cli/main.go
Comment thread cmd/relayfile-cli/main.go
Comment thread cmd/relayfile-cli/main.go
Comment thread cmd/relayfile-cli/main.go
Review fixes (Gemini):
- isWritebackDraftPath: use path.Base (slash-separated remote paths) instead
  of filepath.Base for Windows correctness; import "path".
- waitForWritebackOperation: accept context.Context with the configured
  timeout; poll is now cancellable and tolerates transient errors until the
  deadline rather than aborting the push on the first glitch.

Spec completion (AR-272 Parts 2a/2b/3, this repo):
- Part 2a in-cycle stuck-event drain: a read 404 on a provider-layout-alias
  path (by-state/by-id/...) is a stale index event, so the cycle skips it
  immediately and keeps draining instead of holding the events cursor for the
  full read-not-ready TTL and exiting after one event. Canonical (non-alias)
  404s keep the conservative retry behavior.
- Part 2b: add `relayfile writeback skip-stuck [WS] [--max N] [--json]`
  operator escape hatch that walks the cursor past every read-404 event
  immediately (Syncer.SkipStuck) and reports the count skipped.
- Part 3: mount cycle logs a stuck-event drain summary (N skipped; backlog
  remains -> use skip-stuck) instead of exiting silently mid-drain.

Part 2c (by-state index emitter) is writer-side and lives in the cloud repo,
out of scope here.

Tests: alias in-cycle drain, skip-stuck consecutive 404 drain, draft-path
slash handling; full ./cmd/relayfile-cli + ./internal/mountsync suites pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@khaliqgant khaliqgant force-pushed the ar-272-cli-writeback-fastpath branch from d96f1d4 to 1460bc6 Compare June 15, 2026 12:47
@agent-relay-code

Copy link
Copy Markdown
Contributor

Findings

  1. packages/sdk/typescript/src/client.ts exports webhook subscription/DLQ methods for /v1/workspaces/{workspaceId}/webhooks, but the checked-out server only routes /webhooks/ingest in internal/httpapi/server.go, and OpenAPI only documents /webhooks/ingest in openapi/relayfile-v1.openapi.yaml. These SDK methods will 404 against this service unless the server and contract are added.

  2. cmd/relayfile-cli/main.go requests/validates only provider-scoped fs:write relayfile scopes for writeback push, but the command later polls GET /v1/workspaces/{workspaceId}/ops/{opId}; that route requires ops:read in internal/httpapi/server.go. A delegated push can write successfully, then fail while polling the operation.

  3. cmd/relayfile-cli/main.go writes a failed receipt when operation polling errors or times out after an opId was accepted. That can turn an unknown/in-flight provider operation into a local failure and remove the pending receipt. This is semantic writeback behavior, so I left it for a human-authored patch.

  4. cmd/relayfile-cli/main.go uses filepath.Base on slash-separated remote paths. On Windows, factory-create files under nested remote paths will not be recognized correctly. The fix is likely path.Base plus a path import, but I did not auto-edit because it changes runtime behavior.

Addressed comments

  • CodeRabbit: review-in-progress / merge-conflict finishing-touch note. No code change; current GitHub reports mergeable: true, but the live PR state is still unstable.
  • gemini-code-assist: use path.Base for isWritebackDraftPath. Valid at cmd/relayfile-cli/main.go; not auto-edited because it is cross-platform behavior logic.
  • gemini-code-assist: add path import. Valid only with the previous change; not auto-edited for the same reason.
  • gemini-code-assist: pass a timeout context to waitForWritebackOperation. Valid concern at cmd/relayfile-cli/main.go; not auto-edited because polling/cancellation behavior is semantic.
  • gemini-code-assist: make polling resilient to transient errors. Valid concern at cmd/relayfile-cli/main.go; not auto-edited because it changes failure semantics.
  • chatgpt-codex-connector: add ops:read to writeback push credentials. Valid at cmd/relayfile-cli/main.go; not auto-edited because credential scopes are security/behavioral.
  • chatgpt-codex-connector: remove or implement unhandled webhook routes. Valid at packages/sdk/typescript/src/client.ts; not auto-edited because it needs API/server contract work.
  • Devin: readLocalMountCursorHealth second path likely dead. Valid at cmd/relayfile-cli/main.go; no edit because it is not mechanical and current tested path still works.
  • Devin: SDK webhook methods assume server endpoints exist. Valid duplicate of the Codex webhook-route finding; no edit.
  • agent-relay-code: SDK webhook-route finding and writeback timeout-to-failed finding. Both validated in current checkout; no edit because both require semantic API/writeback decisions.

Verification

No files were edited. Local verification passed with a temporary Go 1.22.12 toolchain on PATH:

  • npm run build
  • npm run typecheck
  • npm run test
  • go test ./...
  • go vet ./...
  • CI Go binary build commands
  • ./scripts/check-contract-surface.sh
  • CI=true npx tsx scripts/e2e.ts --ci

I did not print READY because the live PR head has advanced beyond .workforce/context.json, and GitHub currently shows required checks on that live head as failing or in progress.

- ops:read in delegated join scopes: a successful /fs/bulk returns an opID this
  command then polls at GET /ops/{opId} (requires ops:read). [Codex]
- Redact file body from terminal receipts: clear Content/Encoding before
  writing acked/failed receipts and JSON output; pending receipt keeps the body
  and is written 0600. [CodeRabbit]
- Keep dispatched op pending on unknown final status: a poll timeout or
  transient GET error after an opID is "unknown", not "failed" — record
  pending+needsAttention instead of diverging local state with a failed
  receipt. Only failed/dead_lettered/canceled write a failed receipt.
  [CodeRabbit]
- Re-validate delegated scopes after bootstrap: fail loudly if the minted
  bundle omits the required relayfile:fs:write scope. [CodeRabbit]
- resolveWritebackPushPath rejects non-regular files (FIFO/device/socket)
  before os.ReadFile. [CodeRabbit]
- joinRemotePath trims a trailing slash on remoteRoot to avoid //double
  slashes in the bulk-write path. [CodeRabbit]
- workspace status: use public IncrementalReadNotReadySince as the stuck-event
  baseline, max'd with private cursor health; resolve env/agent-relay/catalog
  default before legacy credentials so status works in the delegated flow.
  [CodeRabbit]

SDK webhook-route findings (Codex/Devin on client.ts) are not applicable: that
code merged separately via #281 and is no longer in this PR's diff.

Tests: receipt redaction/perms/routing, isTerminalWritebackOpStatus, updated
push scope assertion. Full ./cmd/relayfile-cli + ./internal/mountsync pass;
contract check passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@khaliqgant

Copy link
Copy Markdown
Member Author

Review addressed + spec completion (pushed b786ed2)

Conflicts

Rebased the branch onto main (v0.8.29). The conflict came from the old base predating the v0.8.28/0.8.29 releases and the AR-267 webhook SDK that already merged via #281 — now resolved.

Inline review comments

Gemini

  • isWritebackDraftPath: path.Base instead of filepath.Base (+ import path) for slash-separated remote paths.
  • waitForWritebackOperation now takes a context.Context with the configured timeout; poll is cancellable and tolerant of transient errors until the deadline.

Codex

  • ops:read added to the delegated join scopes (the push polls GET /ops/{opId} after /fs/bulk).
  • ❌ SDK webhook routes (client.ts): not applicable — that file is no longer in this PR's diff (the webhook SDK merged separately via AR-267 outbound webhook SDK client #281). Per our setup the SDK targets the cloud API, not server.go.

Devin

  • ❌ SDK webhook methods (client.ts): same as above — out of this PR's diff.
  • ℹ️ readLocalMountCursorHealth second path: it's a harmless fallback; the status report now uses the public IncrementalReadNotReadySince set as the stuck-count baseline (max'd with private cursor health), so a missing private file no longer yields a false 0.

CodeRabbit

  • ✅ Redact Content/Encoding from acked/failed receipts and JSON output; pending receipt keeps the body and is written 0600.
  • ✅ Keep a dispatched op pending+needsAttention on poll timeout/transient error; only failed/dead_lettered/canceled write a failed receipt.
  • ✅ Re-validate delegated scopes after bootstrap.
  • ✅ Reject non-regular files before os.ReadFile.
  • ✅ Trim trailing slash in joinRemotePath.
  • workspace status resolves env/agent-relay/catalog default before legacy credentials, and counts stuck events from public state first.

Spec completion (AR-272)

Beyond PR-A, this now also covers the spec's acceptance criteria implementable in this repo:

  • 2a — in-cycle stuck-event drain: a read-404 on a provider-layout-alias path (by-state/by-id/…) is treated as a stale index event and skipped immediately, so one cycle drains the whole stuck run instead of exiting after the first event. Canonical 404s keep the conservative retry.
  • 2b — relayfile writeback skip-stuck [WS] [--max N] [--json]: operator escape hatch that walks the cursor past every read-404 event without the TTL wait.
  • 3 — pull mid-drain summary: the mount cycle logs N stale index events skipped; backlog remains → use 'relayfile writeback skip-stuck' instead of exiting silently.

2c (by-state index emitter) is writer-side and lives in the cloud repo (AR-268 fanout), out of scope here.

Tests added for alias drain, skip-stuck, receipt redaction/routing, terminal-status classification, and draft-path slash handling. go test ./cmd/relayfile-cli ./internal/mountsync and the contract check pass.

The v0.8.29 release bumped @relayfile/mount-* and @relayfile/core to 0.8.29 in
the workspace package.json files but did not update the root package-lock.json,
which still pinned 0.8.28. Because packages/sdk/typescript is a root workspace
member, `npm ci` resolves against the root lockfile and failed the SDK deps
install — breaking the contract, SDK Typecheck, E2E, and provider-eval jobs
(Go Build/Test were unaffected). Regenerated via `npm install
--package-lock-only`; the diff is the 0.8.28->0.8.29 bump with registry
integrity hashes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Relayfile Eval Review

Run: .relayfile/evals/runs/2026-06-15T13-16-13-907Z-HEAD-provider
Mode: provider
Git SHA: ac93dbc

Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0

Human Review Cases

No reviewable human-review cases captured Relayfile output.

@khaliqgant khaliqgant merged commit 6fb0441 into main Jun 15, 2026
9 checks passed
@khaliqgant khaliqgant deleted the ar-272-cli-writeback-fastpath branch June 15, 2026 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant