Skip to content

AR-267 outbound webhook SDK client#281

Merged
khaliqgant merged 4 commits into
mainfrom
ar-267-outbound-webhooks
Jun 15, 2026
Merged

AR-267 outbound webhook SDK client#281
khaliqgant merged 4 commits into
mainfrom
ar-267-outbound-webhooks

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Jun 15, 2026

Copy link
Copy Markdown
Member

Summary

  • add typed webhook subscription registration, listing, deletion, DLQ listing, and DLQ replay methods
  • export outbound webhook subscription and delivery health types
  • include contentHash on FilesystemEvent to match enriched relayfile event payloads

Tests

  • npm run typecheck
  • npm test -- src/client.test.ts

Review in cubic

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The TypeScript SDK gains five outbound webhook management methods (registerWebhook, listWebhooks, deleteWebhook, getWebhookDeadLetters, replayWebhookDeadLetter) on RelayFileClient, backed by new type contracts and barrel exports. The sync layer is extended to propagate contentHash from WebSocket wire events onto emitted FilesystemEvents. Trajectory metadata files record the completion of the PR #281 review run.

Changes

SDK Outbound Webhook Feature and contentHash

Layer / File(s) Summary
Webhook type contracts and FilesystemEvent extension
packages/sdk/typescript/src/types.ts, packages/sdk/typescript/src/index.ts
Nine new webhook interfaces (WebhookSubscriptionHealth, WebhookSubscription, RegisterWebhookInput, RegisterWebhookResponse, ListWebhooksOptions, DeleteWebhookOptions, GetWebhookDeadLettersOptions, WebhookDeliveryDeadLetterItem, WebhookDeliveryDeadLetterFeedResponse) and an optional contentHash field on FilesystemEvent are declared in types.ts and re-exported from the index barrel.
contentHash propagation through sync wire events
packages/sdk/typescript/src/sync.ts, packages/sdk/typescript/src/sync.test.ts
The internal wire event interface gains contentHash?, and normalizeFilesystemEvent maps it onto the emitted event; a new test asserts the field is preserved end-to-end through a WebSocket file.updated payload.
RelayFileClient webhook methods
packages/sdk/typescript/src/client.ts
Five new public async methods are added targeting /v1/workspaces/{workspaceId}/webhooks... REST endpoints; each validates required inputs before dispatching requests via request or performRequest.
Webhook method tests
packages/sdk/typescript/src/client.test.ts
Tests cover all five methods: request body/URL verification for registration, list/delete endpoint routing, DLQ cursor+limit query construction, replay POST response, and negative cases confirming missing identifiers are rejected before any network call.

Trajectory Metadata Updates

Layer / File(s) Summary
traj_4pvrlmqfnzng active-to-completed promotion
.trajectories/active/traj_4pvrlmqfnzng/trajectory.json, .trajectories/completed/2026-06/traj_4pvrlmqfnzng/...
Active trajectory record removed; completed summary.md and trajectory.json added recording PR #281 findings (SDK/server contract mismatch, re-probing risk, inability to run Go checks).
traj_pmzcmccpttxb completed trajectory record
.agentworkforce/trajectories/completed/2026-06/traj_pmzcmccpttxb/...
New completed trajectory, summary, and trace JSON added for the traj_pmzcmccpttxb run, capturing PR #281 review completion status, confidence, and per-file conversation metadata.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant RelayFileClient
  participant REST_API

  Caller->>RelayFileClient: registerWebhook({workspaceId, url, pathGlobs, secret})
  RelayFileClient->>REST_API: POST /v1/workspaces/{id}/webhooks
  REST_API-->>RelayFileClient: RegisterWebhookResponse {subscriptionId}
  RelayFileClient-->>Caller: RegisterWebhookResponse

  Caller->>RelayFileClient: getWebhookDeadLetters(workspaceId, {cursor, limit})
  RelayFileClient->>REST_API: GET /v1/workspaces/{id}/webhooks/dead-letters?cursor&limit
  REST_API-->>RelayFileClient: WebhookDeliveryDeadLetterFeedResponse
  RelayFileClient-->>Caller: WebhookDeliveryDeadLetterFeedResponse

  Caller->>RelayFileClient: replayWebhookDeadLetter(workspaceId, deliveryId)
  RelayFileClient->>REST_API: POST /v1/workspaces/{id}/webhooks/dead-letters/{deliveryId}/replay
  REST_API-->>RelayFileClient: QueuedResponse
  RelayFileClient-->>Caller: QueuedResponse
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 Hop hop, the webhooks are here at last,
With register, list, and delete going fast!
Dead letters replayed with a bouncy POST,
contentHash travels from wire to coast.
The trajectory's done, the rabbit is proud—
New types in the barrel, all exported aloud! 🎉

🚥 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-267 outbound webhook SDK client' directly and clearly describes the main feature added in this PR—webhook management methods in the TypeScript SDK.
Description check ✅ Passed The description explains the changes made (webhook methods, type exports, contentHash addition) and is directly related to the changeset, including test instructions.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ar-267-outbound-webhooks

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 webhook and dead-letter queue (DLQ) management capabilities to the TypeScript SDK, including methods to register, list, and delete webhooks, as well as retrieve and replay webhook dead letters. It also adds corresponding TypeScript types and unit tests. The feedback suggests adding defensive checks to ensure required parameters like workspaceId, url, subscriptionId, and deliveryId are provided and non-empty, preventing malformed requests or unintended API calls.

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 on lines +1871 to +1883
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
});
}

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

Add defensive checks to ensure workspaceId and url are provided and non-empty. This prevents making malformed requests or hitting incorrect endpoints at runtime.

  async registerWebhook(input: RegisterWebhookInput): Promise<RegisterWebhookResponse> {
    if (!input.workspaceId) {
      throw new Error("workspaceId is required");
    }
    if (!input.url) {
      throw new Error("url is required");
    }
    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
    });
  }

Comment on lines +1897 to +1908
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
});
}

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

Add defensive checks to ensure workspaceId and subscriptionId are provided and non-empty. If subscriptionId is empty, the resulting URL path resolves to the collection endpoint, which could lead to accidental bulk deletion or calling the wrong endpoint.

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

Comment on lines +1910 to +1924
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
});
}

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

Add a defensive check to ensure workspaceId is provided and non-empty to prevent malformed request paths.

  async getWebhookDeadLetters(
    workspaceId: string,
    options: GetWebhookDeadLettersOptions = {}
  ): Promise<WebhookDeliveryDeadLetterFeedResponse> {
    if (!workspaceId) {
      throw new Error("workspaceId is required");
    }
    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
    });
  }

Comment on lines +1926 to +1938
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

Choose a reason for hiding this comment

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

medium

Add defensive checks to ensure workspaceId and deliveryId are provided and non-empty to prevent malformed request paths.

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

@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: 48936510d7

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

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.

P1 Badge Don’t publish SDK calls for unhandled webhook routes

These new methods target /v1/workspaces/{workspaceId}/webhooks, but I checked the HTTP router and OpenAPI contract: internal/httpapi/server.go:228-230 only matches /webhooks/ingest, and the spec only documents that ingest path. As a result, registerWebhook, listWebhooks, deleteWebhook, and the /webhooks/dlq... calls all fall through to 404 against the current service, making the outbound webhook SDK feature unusable unless the server/spec routes are added or the SDK paths are changed.

Useful? React with 👍 / 👎.

type: FilesystemEventType;
path: string;
revision: string;
contentHash?: string;

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 Preserve contentHash through RelayFileSync events

When events are consumed through RelayFileSync, this newly advertised field is still stripped: sync.ts reconstructs events in normalizeFilesystemEvent from a fixed set of fields and RelayFileSyncWireEvent has no contentHash, so server WebSocket payloads that include the hash arrive at onEvent with contentHash undefined. Consumers relying on the new typed field for skip-read/dedup logic only get it from raw REST/raw WebSocket paths, not from the higher-level sync API.

Useful? React with 👍 / 👎.

@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 1 potential issue.

Open in Devin Review

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 methods reference server endpoints that don't exist yet

The new methods (registerWebhook, listWebhooks, deleteWebhook, getWebhookDeadLetters, replayWebhookDeadLetter) target API paths like POST /v1/workspaces/{id}/webhooks, GET /v1/workspaces/{id}/webhooks, DELETE /v1/workspaces/{id}/webhooks/{subId}, GET /v1/workspaces/{id}/webhooks/dlq, and POST /v1/workspaces/{id}/webhooks/dlq/{deliveryId}/replay — none of which exist in internal/httpapi/server.go (only webhooks/ingest is implemented). The OpenAPI spec also only defines the /webhooks/ingest path. This means calling these SDK methods against the current server will result in 404 errors. This is likely intentional SDK-first development, but there's no corresponding server PR linked. The AGENTS.md rule about OpenAPI sync applies when adding handlers (not SDK methods), so this isn't a rule violation, but it's worth confirming the server-side implementation is planned.

Open in Devin Review

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

@agent-relay-code

Copy link
Copy Markdown
Contributor

Findings

  • Medium: SDK exposes outbound webhook routes that the current service does not implement.
    client.ts adds calls to POST/GET /v1/workspaces/{workspaceId}/webhooks, DELETE /webhooks/{subscriptionId}, and /webhooks/dlq..., but the current router only recognizes /webhooks/ingest before returning 404 route not found for unmatched routes in server.go and server.go. The OpenAPI contract likewise only documents /webhooks/ingest at relayfile-v1.openapi.yaml. This would ship SDK methods that consistently fail against this server. Fix requires a human-authored contract/server implementation decision, so I did not edit it.

Addressed Comments

  • Devin, packages/relayfile-sdk/tsconfig.json: stale. Current SDK path is packages/sdk/typescript/tsconfig.json, and it already excludes tests at tsconfig.json.
  • Devin, packages/relayfile-sdk/src/types.ts: stale. BulkWriteResponse.correlationId is present at types.ts.
  • Devin, internal/mountfuse/wsinvalidate.go:134: out of scope for this SDK-only PR, and touches mount lifecycle/reconnect behavior, which is safety-critical under the review instructions.
  • Devin, internal/mountfuse/dir.go:130: out of scope for this SDK-only PR; also already shows FOPEN_KEEP_CACHE in current checkout.
  • Devin, internal/mountfuse/file.go:248: out of scope and safety-critical mount file concurrency behavior.
  • Devin, internal/mountfuse/file.go:225: out of scope and safety-critical mount file flush/order behavior.
  • Devin, packages/core/src/webhooks.ts:387: out of scope for this SDK-only PR.
  • Devin, internal/mountfuse/wsinvalidate.go:71: out of scope and safety-critical reconnect/termination behavior.
  • Devin, resolved internal/mountfuse/dir.go:130: verified stale/already handled; no change.
  • Devin, resolved internal/mountfuse/wsinvalidate.go:134: verified stale/already handled; no change.
  • Devin, resolved internal/mountfuse/file.go:248: verified stale/already handled; no change.
  • Devin, resolved internal/mountfuse/file.go:225: verified stale/already handled; no change.
  • Devin, internal/mountfuse/file.go:231: out of scope and safety-critical mount file behavior.
  • Devin, internal/mountfuse/wsinvalidate.go:133: out of scope and safety-critical mount websocket behavior.

Advisory Notes

  • The saved mount/FUSE comments are unrelated to this PR’s .workforce/pr.diff; I left them untouched per scope and safety instructions.
  • No mechanical lint/format/typo fixes were needed, so I made no source edits.

Verification

  • npm run build --workspace=packages/sdk/typescript passed.
  • npm run typecheck --workspace=packages/sdk/typescript passed.
  • npm run test --workspace=packages/sdk/typescript passed.
  • scripts/check-contract-surface.sh passed.
  • npm run test passed all JS workspaces, then failed at go test ./... because go is not installed.
  • npm run typecheck passed TS workspaces, then failed at go vet ./... because go is not installed.
  • npm run build passed TS workspace builds, then failed when CLI binary build tried to spawn go.
  • npm ci is blocked by existing lockfile/package version drift for optional mount packages 0.8.27 vs 0.8.28.

@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: 1

🤖 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 @.trajectories/completed/2026-06/traj_4pvrlmqfnzng/trajectory.json:
- Around line 39-48: The decision event's content field contains a redundant
phrase repeated twice separated by a colon: "Leave outbound webhook SDK/server
contract mismatch as review finding: Leave outbound webhook SDK/server contract
mismatch as review finding". Remove the duplication by keeping only a single
instance of the phrase in the content field, aligning it with the chosen value
in the raw object so both contain the same non-duplicated text.
🪄 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: bef4f194-4bb0-430c-9a32-c2f8420e0b00

📥 Commits

Reviewing files that changed from the base of the PR and between 9059c23 and 2432065.

📒 Files selected for processing (9)
  • .agentworkforce/trajectories/active/traj_pmzcmccpttxb/trajectory.json
  • .trajectories/active/traj_4pvrlmqfnzng/trajectory.json
  • .trajectories/completed/2026-06/traj_4pvrlmqfnzng/summary.md
  • .trajectories/completed/2026-06/traj_4pvrlmqfnzng/trajectory.json
  • .trajectories/index.json
  • 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
💤 Files with no reviewable changes (2)
  • .trajectories/active/traj_4pvrlmqfnzng/trajectory.json
  • .trajectories/index.json

Comment thread .trajectories/completed/2026-06/traj_4pvrlmqfnzng/trajectory.json
@agent-relay-code

Copy link
Copy Markdown
Contributor

Findings

  1. Blocking: SDK exposes outbound webhook routes the server does not serve.
    packages/sdk/typescript/src/client.ts adds registerWebhook, listWebhooks, deleteWebhook, getWebhookDeadLetters, and replayWebhookDeadLetter against /v1/workspaces/{id}/webhooks.... The current router only recognizes POST /v1/workspaces/{id}/webhooks/ingest at internal/httpapi/server.go, and OpenAPI likewise only documents the ingest path at openapi/relayfile-v1.openapi.yaml. As written, these SDK methods will hit 404s against the current service. This needs a human-authored semantic fix: either implement/document the server contract or hold the SDK surface until the API exists.

Addressed Comments

  • CodeRabbit walkthrough comment: informational summary only; no code change needed.
  • CodeRabbit inline comment on .trajectories/completed/2026-06/traj_4pvrlmqfnzng/trajectory.json: redundant duplicated decision text was valid and fixed in .trajectories/completed/2026-06/traj_4pvrlmqfnzng/trajectory.json.
  • CodeRabbit review summary: duplicate aggregate of the inline finding above; fixed at the same line.

Advisory Notes

  • Canonical npm ci is currently blocked outside this PR’s diff: root package-lock.json still contains 0.8.27 package/optional mount entries while manifests require 0.8.28. I did not update lockfiles because that is outside the SDK outbound-webhook change surface.
  • Go is not installed in this sandbox, so Go CI steps cannot run locally here.

Verification

  • node -e 'JSON.parse(...)' for the edited trajectory JSON: passed.
  • packages/sdk/typescript: npm run build: passed.
  • packages/sdk/typescript: npm run typecheck: passed.
  • scripts/check-contract-surface.sh: passed.
  • Root npm run typecheck: TypeScript workspaces passed, then failed at go vet ./... because go is unavailable.
  • SDK/root tests could not run because npm ci is blocked by lock drift and vitest is not installed.

Not ready: the PR still has a blocking SDK/server/OpenAPI contract mismatch and the full CI-equivalent path could not be completed.

- Regenerate root package-lock.json to 0.8.28 mount packages, fixing the
  stale-lock `npm ci` failures across Contract, E2E, SDK Typecheck, and
  Provider-backed evals (lock still pinned mount-* @ 0.8.27 after the
  v0.8.28 release bump).
- Add defensive required-arg guards to registerWebhook/listWebhooks/
  deleteWebhook/getWebhookDeadLetters/replayWebhookDeadLetter so empty
  workspaceId/url/subscriptionId/deliveryId throw instead of issuing a
  malformed request (addresses gemini-code-assist findings).
- Carry contentHash through RelayFileSyncWireEvent and
  normalizeFilesystemEvent so the new typed field survives the higher-level
  sync API, not just raw REST/WS (addresses codex P2 finding).
- Add tests for the validation guards and contentHash passthrough.

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

Copy link
Copy Markdown
Member Author

Pushed 76abab3 addressing CI + review findings:

CI (all four failures) — root cause was a stale root package-lock.json still pinning @relayfile/mount-*@0.8.27 after the v0.8.28 release bump, so npm ci failed in every job (Contract, E2E, SDK Typecheck, Provider-backed evals; the SDK dir resolves against the root workspace lock). Regenerated the lock to 0.8.28 — npm ci, build, typecheck, contract-surface check, and 94 SDK tests all pass locally.

gemini-code-assist (defensive checks) — added required-arg guards to registerWebhook/listWebhooks/deleteWebhook/getWebhookDeadLetters/replayWebhookDeadLetter so empty workspaceId/url/subscriptionId/deliveryId throw before issuing a malformed request. Added a test asserting no fetch is made.

codex P2 (contentHash stripped through sync) — added contentHash to RelayFileSyncWireEvent and normalizeFilesystemEvent, with a test confirming it survives the WebSocket path. Resolved.

codex P1 / Devin (server routes don't exist) — false positive. These reviewers checked the Go reference server (internal/httpapi/server.go), which is not the production API the SDK targets. The production server (Cloudflare Workers / Durable Objects) implements all five routes in the cloud repo on the matching ar-267-outbound-webhooks branch (Implement outbound relayfile webhooks): packages/relayfile/src/routes/webhook-subscriptions.ts + DLQ migration 0007_webhook_delivery_dead_letters.sql. Paths and response shapes match the SDK field-for-field (WebhookSubscription, RegisterWebhookResponse {subscriptionId, secret?}, DLQ feed {items, nextCursor}, replay QueuedResponse at 202). No server work needed here.

@github-actions

Copy link
Copy Markdown

Relayfile Eval Review

Run: .relayfile/evals/runs/2026-06-15T12-00-13-247Z-HEAD-provider
Mode: provider
Git SHA: 8d0e2c4

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.

@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: 1

🤖 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 `@packages/sdk/typescript/src/sync.ts`:
- Line 142: The contentHash field declared with type annotation at line 142 is
being forwarded from an untrusted WebSocket payload at line 258 without runtime
type validation. This can cause type violations if the payload contains a
non-string value (such as an object or number). Add a runtime type guard before
the normalization event is emitted at line 258 to validate that contentHash is
either a string or undefined, ensuring it conforms to the declared type contract
and prevents downstream consumers from receiving invalid types. Only proceed
with emitting the FilesystemEvent if the contentHash passes validation.
🪄 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: 0a8d8682-dd37-4229-8fea-b05c38516a97

📥 Commits

Reviewing files that changed from the base of the PR and between 2432065 and 76abab3.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (8)
  • .agentworkforce/trajectories/completed/2026-06/traj_pmzcmccpttxb.trace.json
  • .agentworkforce/trajectories/completed/2026-06/traj_pmzcmccpttxb/summary.md
  • .agentworkforce/trajectories/completed/2026-06/traj_pmzcmccpttxb/trajectory.json
  • .trajectories/completed/2026-06/traj_4pvrlmqfnzng/trajectory.json
  • packages/sdk/typescript/src/client.test.ts
  • packages/sdk/typescript/src/client.ts
  • packages/sdk/typescript/src/sync.test.ts
  • packages/sdk/typescript/src/sync.ts
✅ Files skipped from review due to trivial changes (4)
  • .agentworkforce/trajectories/completed/2026-06/traj_pmzcmccpttxb/summary.md
  • .agentworkforce/trajectories/completed/2026-06/traj_pmzcmccpttxb.trace.json
  • .agentworkforce/trajectories/completed/2026-06/traj_pmzcmccpttxb/trajectory.json
  • .trajectories/completed/2026-06/traj_4pvrlmqfnzng/trajectory.json
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/sdk/typescript/src/client.ts
  • packages/sdk/typescript/src/client.test.ts

eventId?: string;
path?: string;
revision?: string;
contentHash?: string;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Validate contentHash type before normalization emits events.

At Line 258, contentHash is forwarded from untrusted WebSocket payload without a runtime type guard. If payload sends a non-string (e.g., object/number), emitted FilesystemEvent.contentHash violates the contract and can break downstream consumers expecting string | undefined.

Suggested fix
@@
     if (parsed.ts !== undefined && typeof parsed.ts !== "string") {
       this.emit("error", new Error("Invalid WebSocket payload: 'ts' must be a string."));
       return;
     }
+    if (parsed.contentHash !== undefined && typeof parsed.contentHash !== "string") {
+      this.emit("error", new Error("Invalid WebSocket payload: 'contentHash' must be a string."));
+      return;
+    }
 
     if (parsed.type === "pong") {

Also applies to: 258-258

🤖 Prompt for 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.

In `@packages/sdk/typescript/src/sync.ts` at line 142, The contentHash field
declared with type annotation at line 142 is being forwarded from an untrusted
WebSocket payload at line 258 without runtime type validation. This can cause
type violations if the payload contains a non-string value (such as an object or
number). Add a runtime type guard before the normalization event is emitted at
line 258 to validate that contentHash is either a string or undefined, ensuring
it conforms to the declared type contract and prevents downstream consumers from
receiving invalid types. Only proceed with emitting the FilesystemEvent if the
contentHash passes validation.

@khaliqgant khaliqgant merged commit 654f2e4 into main Jun 15, 2026
9 checks passed
@khaliqgant khaliqgant deleted the ar-267-outbound-webhooks branch June 15, 2026 12:03
khaliqgant added a commit that referenced this pull request Jun 15, 2026
- 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 added a commit that referenced this pull request Jun 15, 2026
* AR-272 add direct writeback push

* AR-272 address review + complete stuck-event spec

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>

* AR-272 address Codex/Devin/CodeRabbit review on writeback push

- 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>

* chore: sync lockfile mount/core deps to 0.8.29

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>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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