-
Notifications
You must be signed in to change notification settings - Fork 0
feat: migrate agents to @relayfile/relay-helpers #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| .DS_Store | ||
| node_modules/ | ||
| */.workforce/build/ | ||
| .claude/ |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,13 +12,9 @@ | |
| */ | ||
| import { | ||
| defineAgent, | ||
| draftFile, | ||
| encodeSegment, | ||
| resolveMountRoot, | ||
| writeJsonFile, | ||
| type IntegrationClientOptions, | ||
| type WorkforceCtx | ||
| } from '@agentworkforce/runtime'; | ||
| import { githubClient, slackClient } from '@relayfile/relay-helpers'; | ||
|
|
||
| interface Pr { | ||
| owner: string; | ||
|
|
@@ -29,20 +25,6 @@ interface Pr { | |
| headSha?: string; | ||
| } | ||
|
|
||
| function vfsClient(): IntegrationClientOptions { | ||
| return { relayfileMountRoot: resolveMountRoot({}) }; | ||
| } | ||
|
|
||
| /** Draft path for a PR-level comment (PRs are issues on the github side). */ | ||
| function prCommentPath(pr: Pr): string { | ||
| return `/github/repos/${encodeSegment(pr.owner)}/${encodeSegment(pr.repo)}/issues/${pr.number}/comments/${draftFile('comment')}`; | ||
| } | ||
|
|
||
| /** Slack channel message draft path. */ | ||
| function slackPostPath(channel: string): string { | ||
| return `/slack/channels/${encodeSegment(channel)}/messages/${draftFile('message')}`; | ||
| } | ||
|
|
||
| export default defineAgent({ | ||
| // Re-review on every PR change (open, new commits, review comments, finished | ||
| // CI), and merge when you approve. Every `on` value autocompletes from | ||
|
|
@@ -112,19 +94,19 @@ async function reviewAndFix(ctx: WorkforceCtx, pr: Pr): Promise<void> { | |
| if (!body) { | ||
| await failReviewRun(ctx, pr, 'The review harness produced no review output.'); | ||
| } | ||
| const client = vfsClient(); | ||
| if (body) { | ||
| await writeJsonFile(client, 'github', 'comment', prCommentPath(pr), { body }); | ||
| await githubClient().comment({ owner: pr.owner, repo: pr.repo, number: pr.number }, body); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Guard this outbound comment write with idempotency/deduplication. On handler redelivery, this path can post duplicate PR comments. Prompt for AI agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledged, but deferring out of this PR: this is a mechanical migration (same writes, just via relay-helpers) — the duplicate-on-redelivery behavior is pre-existing, not introduced here. Also the runtime dedupes redeliveries on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the feedback. |
||
| } | ||
|
Comment on lines
+98
to
99
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: No evidence found that Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Repo file tree (focus) =="
ls -R
echo "== Find references to relay-helpers / githubClient / slackClient =="
rg -n "relay-helpers|githubClient\(|slackClient\(|failReviewRun|post\(|comment\(" -S .
echo "== Show review/agent.ts around referenced lines =="
sed -n '70,180p' review/agent.ts
echo "== Locate package.json/lock to identify where `@relayfile/relay-helpers` comes from =="
ls
fd -a "package.json" .
fd -a "package-lock.json" .
fd -a "pnpm-lock.yaml" .
fd -a "yarn.lock" .Repository: AgentWorkforce/agents Length of output: 8940 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Check if node_modules exists (so we can inspect actual relay-helpers code)
if [ -d "node_modules/@relayfile/relay-helpers" ]; then
echo "Found local node_modules/@relayfile/relay-helpers"
ls -R node_modules/@relayfile/relay-helpers | head -n 200
echo "== Search for idempotency/dedupe markers in relay-helpers =="
rg -n "Idempotency|idempotency|dedupe|dedup|X-Request-ID|requestId|retry|deduplication" node_modules/@relayfile/relay-helpers -S || true
echo "== Search for GitHub comment/post call wrappers =="
rg -n "comment\\(|slackClient\\(\\)|post\\(|mergePullRequest\\(" node_modules/@relayfile/relay-helpers -S || true
else
echo "node_modules/@relayfile/relay-helpers not present in this checkout."
fi
echo "== Search for retry/at-least-once hints in review harness entrypoints =="
rg -n "retry|retries|redeliver|at-?least-?once|idempotent|dedup" review/agent.ts . -S || true
echo "== Show any local docs about idempotency for relay-helpers in repo =="
rg -n "`@relayfile/relay-helpers`|relay-helpers" -S README.md review/README.md . || trueRepository: AgentWorkforce/agents Length of output: 1197 🌐 Web query:
💡 Result: I couldn’t find any public, authoritative evidence for the specific internal chain you referenced: Citations:
🌐 Web query:
💡 Result: I couldn’t find any authoritative public source that shows the actual “@relayfile/relay-helpers githubClient comment” implementation (i.e., the code for how Citations: 🌐 Web query:
💡 Result: I couldn’t find any authoritative or directly relevant information about the package name “@relayfile/relay-helpers” (including any mention of “idempotency” or “slackClient”). The web results I found around “relay-helpers” appear to be unrelated packages (e.g., Relay GraphQL helpers/network-layer, or other “relay” libraries), not “@relayfile/relay-helpers”. So, I can’t confidently answer how (or whether) “@relayfile/relay-helpers” implements idempotency for a “slackClient” from evidence. If you paste the repository link (GitHub URL) or the relevant file path(s) (e.g., where slackClient/idempotency are implemented), I can re-run the research specifically on that code/docs and give a grounded answer. Citations:
Make GitHub/Slack write operations idempotent under handler retries.
No accessible evidence shows await githubClient().comment({ owner: pr.owner, repo: pr.repo, number: pr.number }, body);Require idempotent behavior (e.g., provider/client-side idempotency keys + retry-safe semantics, or local/per-PR-per-run dedupe) or ensure these handler paths cannot be retried/redelivered. 🤖 Prompt for AI Agents |
||
|
|
||
| const channel = input(ctx, 'SLACK_CHANNEL'); | ||
| if (channel) { | ||
| const who = `<https://github.com/${pr.author}|@${pr.author}>`; // the PR opener | ||
| await writeJsonFile(client, 'slack', 'post', slackPostPath(channel), { | ||
| text: ready | ||
| await slackClient().post( | ||
| channel, | ||
| ready | ||
| ? `:white_check_mark: ${who} — PR #${pr.number} in *${pr.owner}/${pr.repo}* is ready for your review: ${pr.url}` | ||
| : `:eyes: ${who} — reviewing PR #${pr.number} in *${pr.owner}/${pr.repo}*, still working on it: ${pr.url}` | ||
| }); | ||
| ); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -140,36 +122,33 @@ async function failReviewRun(ctx: WorkforceCtx, pr: Pr, reason: string): Promise | |
| number: pr.number, | ||
| reason, | ||
| }); | ||
| const client = vfsClient(); | ||
| await writeJsonFile(client, 'github', 'comment', prCommentPath(pr), { body: message }); | ||
| await githubClient().comment({ owner: pr.owner, repo: pr.repo, number: pr.number }, message); | ||
| const channel = input(ctx, 'SLACK_CHANNEL'); | ||
| if (channel) { | ||
| await writeJsonFile(client, 'slack', 'post', slackPostPath(channel), { | ||
| text: `:warning: pr-reviewer failed for PR #${pr.number} in *${pr.owner}/${pr.repo}*: ${reason}` | ||
| }); | ||
| await slackClient().post( | ||
| channel, | ||
| `:warning: pr-reviewer failed for PR #${pr.number} in *${pr.owner}/${pr.repo}*: ${reason}` | ||
| ); | ||
| } | ||
| throw new Error(message); | ||
| } | ||
|
|
||
| async function mergePr(ctx: WorkforceCtx, pr: Pr): Promise<void> { | ||
| const client = vfsClient(); | ||
| const mergePath = `/github/repos/${encodeSegment(pr.owner)}/${encodeSegment(pr.repo)}/pulls/${pr.number}/merge.json`; | ||
| const result = await writeJsonFile(client, 'github', 'merge', mergePath, { | ||
| const result = await githubClient().mergePullRequest({ | ||
| owner: pr.owner, | ||
| repo: pr.repo, | ||
| number: pr.number, | ||
| method: 'squash', | ||
| ...(pr.headSha ? { sha: pr.headSha } : {}) | ||
| }); | ||
| // The writeback worker reports the merge outcome on the receipt. A missing | ||
| // receipt means fire-and-forget didn't confirm — surface that loudly rather | ||
| // than pretend the merge landed. | ||
| const merged = result.receipt?.merged; | ||
| if (merged !== true && merged !== 'true') { | ||
| // mergePullRequest surfaces the writeback worker's merge outcome as `merged`. | ||
| // A false/unconfirmed result means we shouldn't pretend the merge landed. | ||
| if (!result.merged) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Merge confirmation uses a truthiness check, which can falsely accept non-boolean values like Prompt for AI agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Declining: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the feedback! I've saved this as a new learning to improve future reviews. |
||
| throw new Error(`GitHub did not confirm PR #${pr.number} in ${pr.owner}/${pr.repo} was merged.`); | ||
| } | ||
| const channel = input(ctx, 'SLACK_CHANNEL'); | ||
| if (channel) { | ||
| await writeJsonFile(client, 'slack', 'post', slackPostPath(channel), { | ||
| text: `:tada: Merged PR #${pr.number} in ${pr.owner}/${pr.repo}.` | ||
| }); | ||
| await slackClient().post(channel, `:tada: Merged PR #${pr.number} in ${pr.owner}/${pr.repo}.`); | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To prevent potential runtime crashes, we should defensively check if the issue was successfully retrieved before passing it to
parseRepo. If the issue is not found or the API call returnsundefined/null, callingparseRepo(issue)will throw aTypeErrorwhen attempting to accessissue.title.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Declining:
linearClient().getIssue<LinearIssue>(issueId)returns a non-nullableLinearIssue— on a missing/unreadable file it throws (readJsonFile→RelayfileWritebackError), it never resolves toundefined/null. Soif (!issue)is dead code, and the behavior is unchanged from the pre-migrationreadJsonFile<LinearIssue>(...)which threw the same way.