Skip to content

139: "Who can I ask?" — discover teammate agents by skill and engage them; publish cards for hosted personas - #178

Open
agent-relay-code[bot] wants to merge 21 commits into
mainfrom
factory/139-agentworkforce-factory-49b6fd48
Open

139: "Who can I ask?" — discover teammate agents by skill and engage them; publish cards for hosted personas#178
agent-relay-code[bot] wants to merge 21 commits into
mainfrom
factory/139-agentworkforce-factory-49b6fd48

Conversation

@agent-relay-code

@agent-relay-code agent-relay-code Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Part of the cross-repo epic #2785. This is the consumer issue — the one that makes the ecosystem come alive. Depends on the directory from relaycast#294 and the shared card contract from workforce#296.

Why

The Factory turns issues into reviewed PRs by spawning worker agents. Today, when a worker hits a question it can't answer, it has nowhere to turn — Factory only discovers fleet nodes/capabilities for placement (FleetClient.roster()), not agents by skill, and there's no path to actually ask another agent. Once every proactive agent publishes an A2A card and registers with the Relaycast directory, a Factory worker should be able to: "who can answer X?" → pick an agent by its card skills → send it the question and get a reply.

What

1. Directory-backed peer discovery

Add a card-aware lookup alongside roster() in FleetClient (src/ports/fleet.ts), implemented in RelayFleetClient (src/fleet/relay-fleet-client.ts) — query Relaycast's GET /v1/a2a/directory filtered by skill/tag, returning { name, skills[], url, kind } per candidate. Optionally cache via an in-package index like RelayflowPolicyRegistry (src/dispatch/relayflow-registry.ts).

2. "Ask a teammate" capability

Give a running worker a way to engage a discovered peer: resolve target via #1, then send over Relaycast — A2A POST /a2a/rpc message/send or the broker seam (FleetClient.sendMessage/waitForInjected/onAgentMessage). Reuse the existing DM/relay path; don't invent a new transport. Surface it as a discrete capability/tool the worker invokes mid-task, with a bounded wait for the reply.

3. Publish cards for factory-hosted personas

Generate an A2A card for factory-hosted personas (e.g. .agentworkforce/agents/factory-feature-guardian/persona.json) via @agentworkforce/persona-kit deriveAgentCard ([workforce#296]) and publish it when the persona-backed node comes online — createFactoryNodeDefinition (src/node/factory-node.ts) already assembles capabilities/tags/metadata and calls defineNode. Export the new surface from src/index.ts.

Acceptance

  • A FleetClient method returns teammate agents matching a skill/tag from the Relaycast directory (with how to reach each).
  • A worker can, mid-task, ask a discovered teammate a question and receive a reply (bounded wait).
  • Factory-hosted personas publish a deriveAgentCard-derived card when their node comes online; they appear in the directory.
  • New surface exported from src/index.ts; card schema consumed from @relaycast/a2a (no hand-rolled shape).

✅ End-to-end verification — Factory success criteria (REQUIRED)

This issue IS the "does the ecosystem come alive?" proof. "Compiles" is nowhere near done — the Factory must run a real discover → ask → reply round trip and observe the reply arrive, then check the harness in. Use agent-relay's in-process broker so this runs locally with no external services (src/fleet/internal-fleet-client.ts, src/fleet/ensure-relay-broker.ts).

Run it

npm run build
npm test                                   # vitest run (add the E2E below)

The E2E (test/e2e/ask-a-teammate.test.ts, runnable via npm test) must:

  1. Stand up a local directory (mock the Relaycast /v1/a2a/directory HTTP response, or seed the in-process broker) with two agents whose cards declare distinct skills (e.g. ["infra-watch"], ["code-review"]).
  2. Call the new Factory discovery method with skill: "infra-watch" → assert it returns only the infra agent, with an address to reach it.
  3. Stand up a stub teammate agent on the in-process broker (InternalFleetClient + ensureRelayBroker) that replies to an inbound question with a canned answer.
  4. From a simulated worker, invoke the new "ask a teammate" capability against the discovered agent → assert the question is delivered and the canned reply is received within the bounded wait.
  5. For persona cards: bring a factory-hosted persona node online via createFactoryNodeDefinition, then assert its deriveAgentCard-derived card was published/registered and shows up in the directory lookup from step 2.

Observe (must pass)

  • Discovery returns the right agent for a skill and [] for an unknown skill.
  • The worker actually receives the teammate's reply over the relay path (the round trip completes, not just "message sent").
  • A factory-hosted persona is discoverable by its skill after its node comes online.

Deliverable

  • test/e2e/ask-a-teammate.test.ts running the full discover→ask→reply round trip against the in-process broker + mock directory, one command (npm test).
  • The E2E fails if discovery returns everything unfiltered, if the reply never arrives, or if persona-card publishing is removed (no false green).

Anchor files

  • src/ports/fleet.ts (FleetClient, RosterEntry, sendMessage/waitForInjected/onAgentMessage)
  • src/fleet/relay-fleet-client.ts, src/fleet/internal-fleet-client.ts, src/fleet/ensure-relay-broker.ts
  • src/dispatch/relayflow-registry.ts (in-package registry precedent)
  • src/node/factory-node.ts (createFactoryNodeDefinition), src/index.ts (public barrel)
  • .agentworkforce/agents/factory-feature-guardian/persona.json (proto card / template)
  • planning/linear-issue-cloud-proactive-runtime-fleet-unification.md, planning/factory-unified-node-architecture-linear-issue.md

Fixes #139


Summary by cubic

Workers can discover teammate agents by skill and ask them; Factory‑hosted personas publish canonical A2A cards. Previously there was no skill discovery or persona publishing; now asks are bounded, correlate replies to the real sender, and report definitive delivery rejections.

  • Discovery and ask: FleetClient.discoverTeammates({ skill | tag | q }) queries a Relaycast directory via RelaycastTeammateDirectory. askTeammate(...) awaits transport readiness, blocks overlapping asks to the same teammate from the same requester, drains late replies, quarantines timed‑out asks, matches replies using the backend’s effectiveSender(), and throws FleetDeliveryRejectedError on rejection. Adds discoverTeammates, effectiveSender(), and whenMessagesObservable() to FleetClient; exports askTeammate and RelaycastTeammateDirectory.

  • Teammate tools via MCP: packaged entrypoint bin/factory.mjs teammate-mcp extends the injected Agent Relay MCP server so workers keep existing Relay tools and identity while gaining discover/ask. InternalFleetClient resolves this command by default.

  • Persona cards: derive with @agentworkforce/persona-kit’s canonical mapper, validate with @relaycast/a2a, and publish via RelaycastAgentCardPublisher with retry and 409 handling. startFactoryNode({ cardPublisher }) publishes on node registration and exposes cardPublished. The default @agent-relay/factory/node hosts no persona; cloud personas are unchanged.

  • Relay workspace scoping: identify message streams by Relay workspace ID and fail closed on unknown streams; preserve identity across rebinds. createFleet accepts messageStreamScope and forwards it; pass RELAY_WORKSPACE_KEY as both RELAY_WORKSPACE_KEY and RELAY_API_KEY.

  • Verification: end‑to‑end test/e2e/ask-a-teammate.test.ts exercises discover → ask → reply and persona card publication; unit tests cover MCP tools, directory filtering, concurrency claims, and relay/internal backends.

  • Migration

    • Upgrade to Node 22+.
    • Set RELAY_WORKSPACE_KEY in worker environments (and use it as RELAY_API_KEY) when using the Relay backend.

Written for commit 0075364. Summary will update on new commits.

Review in cubic

@kjgbot
kjgbot marked this pull request as draft July 21, 2026 14:16
@kjgbot

kjgbot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

End-to-end merge gate: this PR is not production-ready yet.

Validated locally at 3e12fc676c6389b254310cd6e979a35d39cacedb:

  • focused discovery, ask/reply, and persona-card tests: 3 files / 5 tests pass
  • build, feature map, and 9 packed-install checks pass
  • all five hosted Factory E2Es pass

Real integration blockers:

Unblock/merge criteria:

  1. publish/deploy the Relaycast directory and canonical persona-kit deriveAgentCard
  2. remove the compatibility mapper and consume the published shared API
  3. integrate current Factory main
  4. run a real deployed discover → ask → reply → persona publish/directory lookup, in addition to the checked-in mock E2E
  5. rerun all local and five hosted E2E gates and clear review threads

Marking draft so the passing mocked checks cannot be mistaken for full ecosystem proof.

Copy link
Copy Markdown
Member

Addressed the branch-integration finding from the merge-gate review on the current PR head.

  • Merged current origin/main at 0c70c64 (Factory 0.1.52) into this branch in commit 90b4837; the branch is conflict-free and GitHub reports it mergeable.
  • Kept the reply-matching, timeout, and idempotent card-registration hardening in 66a1fa2.
  • Exact-head local validation passed: npm run build; focused teammate/persona tests (3 files / 6 tests); full npm test (72 files / 1,313 tests); feature-map check; CLI help; and the tier-2 fixture cycle.
  • Fresh GitHub Actions CI run #456 passed all five jobs on 90b48377e7b24f0dc2d748a8777a92f1973e671c: package, verification-gate-e2e, load-e2e, verification-stack-e2e, and kubernetes-provider-e2e.

The PR remains draft. The external merge gates are still blocked exactly as identified: Relaycast #294 is not deployed, Workforce #296 has not published deriveAgentCard, the compatibility mapper therefore remains, and the required real deployed discover → ask → reply → publish/directory proof cannot yet be run. I am not treating the mocked/in-process E2E as production-readiness evidence.

@khaliqgant

Copy link
Copy Markdown
Member

Stale-PR triage: REVIVE — the blocker that stalled this has since cleared

Triage sweep of the 6 open PRs, evaluated against origin/main @ f13600f.

#139's closure is a false positive — this work is not done

#139 shows CLOSED / COMPLETED, which would normally end the discussion. It should not be trusted here:

$ gh api repos/AgentWorkforce/factory/issues/139/timeline
commented 2026-08-13 "Factory observed pull request #2891 merge and completed this issue."
closed    2026-08-13

Factory has no PR #2891 — its numbering is in the ~270s. PR #2891 is in AgentWorkforce/cloud: "fix(relayauth): add dormant emergency source mint gate", which has nothing to do with teammate discovery. This is a cross-repo issue/PR number collision in Factory's reconciliation (#146 was closed the same way by cloud#2895). I am reporting that defect separately.

Nothing on main implements this

$ git grep -i -l "teammate\|personaCard\|deriveAgentCard\|discoverBySkill" origin/main -- src
(no matches)

No src/fleet/teammates.ts, no src/node/factory-persona-card.ts, no equivalent under another name. The capability is absent from main, so there is nothing that superseded this PR.

The question that stalled it has been answered by events

This PR stopped on 2026-07-22 with an open human-input request:

Which published @agentworkforce/persona-kit version containing deriveAgentCard should PR #178 target so I can remove the compatibility mapper and complete the shared-contract acceptance gate?

That was never answered in-thread, but upstream resolved it two days later. AgentWorkforce/workforce#296"deriveAgentCard(personaSpec): canonical persona → A2A agent card in persona-kit" — closed COMPLETED on 2026-07-24. I verified it actually shipped rather than trusting the issue state, by unpacking the published tarball:

$ npm pack @agentworkforce/persona-kit@4.1.43 && tar xzf *.tgz
$ grep -rl deriveAgentCard package/
package/dist/spec.d.ts
package/dist/agent-card.d.ts
package/dist/agent-card.js
package/dist/index.d.ts
...

deriveAgentCard is present in the published 4.1.43. This branch pins ^4.1.34 and carries an in-tree compatibility mapper with the comment "workforce#296 owns the canonical mapper; persona-kit 4.1.34 predates that". Reviving this should bump the dependency and delete that mapper — the revival is a net scope reduction, not just a rebase.

Rebase cost: moderate, mostly mechanical

Tested in an isolated detached worktree (never the shared checkout, no force-push). 7 files conflict, one block each:

package-lock.json                  <- regenerate, not hand-merge
package.json                       <- dependency bump anyway
vitest.config.ts                   <- 16 lines
src/index.ts                       <- export barrel
src/fleet/internal-fleet-client.ts
src/fleet/relay-fleet-client.ts
.agentworkforce/agents/factory-feature-guardian/persona.json

The lockfile and package.json blocks resolve by regeneration, and the persona-kit bump has to happen regardless. The two src/fleet/*-client.ts blocks are the only ones needing real judgment. mergeable: CONFLICTING overstates this.

CI. gh run list --branch <this-branch> shows CI success across all 8 head shas including the current d93aaa5. CI is the only workflow on this branch.

Still a draft — it would need the dependency bump, mapper deletion, and un-drafting before review.

Recommendation only — I do not close or merge. Posted by an automated triage lane.

@khaliqgant

Copy link
Copy Markdown
Member

Triage: REBASE-AND-FINISH — unlanded, blocker cleared, and the conflicts are mostly mechanical

Stale-PR triage pass. Recommendation only; no ruling has been made yet. Evidence against origin/main at f6050e0, fetched today.

This is the healthiest of the four stale Factory PRs and the one I'd revive first.

1. Is the work already on main by another route? — No.

Checked origin/main directly rather than trusting issue state:

  • origin/main:src/fleet/ contains create-fleet.*, ensure-relay-broker.*, internal-fleet-client.*, relay-fleet-client.*, relay-workspace-key.ts. There is no teammates.ts.
  • origin/main:src/node/ contains factory-node.*, factory.node.ts, preview-process.*, tailscale-preview.*. There is no factory-persona-card.ts and no factory-node-runtime.ts.

None of this PR's 300-line teammate directory or 298-line persona-card publisher exists on main.

2. Does the parent issue still describe something we want? — Yes. And #139 is wrongly closed; it should be reopened.

⚠️ Correction to the triage brief, which listed #139 as open: it is CLOSED, as of 2026-08-16T23:27:42Z, stateReason: COMPLETED. That closure is invalid, and it is the second invalid closure of this same issue:

The second closure is not the same defect, and #278's fix would not have prevented it. #278 is a real merged PR in this repository, so repo-qualification passes. But #278's file list is exactly src/orchestrator/factory.ts and src/orchestrator/factory.test.ts — it has nothing whatever to do with "Who can I ask?" teammate discovery. So a same-repo PR with no connection to the issue still advanced it to Done. Repo-qualification was necessary but is not sufficient; the record-less fallback still resolves merged PRs to unrelated issues within the same repo.

(I have not established whether the running Factory had picked up #278's build by 23:27 — merged is not released is not deployed — so this may be the old binary re-firing rather than a surviving hole in the new logic. Either way the closure is invalid and worth a separate look. Flagging, not filing, pending your ruling.)

Recommendation: reopen #139. I have not done so — that is outside my brief.

3. Is the original blocker cleared? — Yes. Verified independently against the registry, not against upstream issue state.

#139's open human-input request from 2026-07-22 was: "Which published @agentworkforce/persona-kit version containing deriveAgentCard should PR #178 target so I can remove the compatibility mapper?"

I unpacked the published tarball rather than trusting a version claim:

npm pack @agentworkforce/persona-kit@4.1.43
→ package/dist/agent-card.js, package/dist/agent-card.d.ts   (deriveAgentCard present)

4.1.43 is also the current latest. This PR pins "@agentworkforce/persona-kit": "^4.1.34", which resolves to 4.1.43. The blocker is genuinely cleared and the compatibility mapper can come out. This independently confirms the earlier REVIVE verdict recorded on #139 on 2026-08-16.

4. How big is the conflict? — 7 files, 82 behind — but 4 of the 7 are mechanical.

git merge-tree --write-tree origin/main origin/factory/139-agentworkforce-factory-49b6fd48 → exit 1, conflicting:

.agentworkforce/agents/factory-feature-guardian/persona.json
package.json
package-lock.json
src/fleet/internal-fleet-client.ts
src/fleet/relay-fleet-client.ts
src/index.ts
vitest.config.ts

rev-list --left-right --count82 behind, 9 ahead.

Seven conflicts sounds worse than it is. Inspected individually:

file nature cost
src/index.ts pure export-list append (two blocks) trivial — take both sides
vitest.config.ts one line: adds 'test/e2e/**/*.test.ts' to include trivial
package.json dependency block alphabetical reorder + 2 additions trivial
package-lock.json regenerate from resolved package.json mechanical
.agentworkforce/.../persona.json +10 / −0 small
internal-fleet-client.ts + relay-fleet-client.ts +59 lines total, purely additive small, real

So the substantive merge work is roughly 60 additive lines across two fleet clients. Rebase, don't redo — the 300-line teammates.ts, the 298-line persona-card publisher, and the 306-line E2E all apply cleanly and are worth well more than they'd cost to rewrite.

Recommendation

REBASE-AND-FINISH. Concretely, on a ruling:

  1. Reopen [factory] "Who can I ask?" — discover teammate agents by skill and engage them; publish cards for hosted personas #139 (its closure is invalid, twice over).
  2. Cut a worktree, rebase factory/139-...49b6fd48 onto origin/main, taking both sides on the four mechanical conflicts and merging the two fleet clients by hand.
  3. Bump the pin to @agentworkforce/persona-kit@^4.1.43 and remove the compatibility mapper — the thing the PR was blocked on since 2026-07-22.
  4. Regenerate package-lock.json, run the suite, take it out of draft.
  5. Keep factory-ready off [factory] "Who can I ask?" — discover teammate agents by skill and engage them; publish cards for hosted personas #139 while this is in flight, so Factory doesn't dispatch a duplicate agent against a PR that already implements it (this exact duplication was caught and prevented on 2026-08-16).

Holding for a ruling — not rebasing, reopening, or closing anything on my own.

@khaliqgant
khaliqgant force-pushed the factory/139-agentworkforce-factory-49b6fd48 branch from d93aaa5 to 996b7bc Compare August 17, 2026 14:05
@khaliqgant

Copy link
Copy Markdown
Member

Rebase-and-finish update

Rebased this PR onto current origin/main (c79d564) and force-updated the existing branch with an exact-old-SHA lease. Current head: 996b7bca0665aaf050613f343fee758b39274510.

The July blocker in Workforce is resolved in code:

  • moved @agentworkforce/persona-kit to the production dependency set at ^4.1.43;
  • verified the root install resolves @agentworkforce/persona-kit@4.1.43;
  • removed the in-tree compatibility mapper;
  • now imports the published canonical deriveAgentCard from @agentworkforce/persona-kit/spec and still validates the result through A2aAgentCardSchema.

Verification at the rebased content:

  • npm run build — exit 0
  • focused teammate/persona/round-trip E2E: 3 files, 8 tests — exit 0
  • full npm test: 96 files passed / 1 skipped; 1,730 tests passed / 1 skipped — exit 0
  • npm run featuremap:check: 319 features, no advisories — exit 0
  • cost-accounting E2E: 6 tests — exit 0
  • packed consumer E2E under Node 22.22.2: 11 checks bound to exact head 996b7bc — exit 0
  • git diff --check — exit 0

The PR remains draft for one external reason that the rebase cannot fix: AgentWorkforce/relaycast#294 is still open and an unauthenticated probe of the required directory route still returns HTTP 404. Therefore the required real deployed discover → ask → reply → persona publish/directory proof is not available yet. The checked-in mock round trip is green, but I am not presenting it as production proof.

No merge performed.

@khaliqgant

Copy link
Copy Markdown
Member

Hosted CI is green at exact head 996b7bc: run 32037774187 completed with all five jobs successful (package, kubernetes-provider-e2e, verification-gate-e2e, verification-stack-e2e, load-e2e). The package job included the full test suite, feature-map validation, packed lifecycle verification, attestation upload, and package-content verification.

The PR intentionally remains draft only for the live Relaycast directory proof recorded above.

khaliqgant added a commit that referenced this pull request Aug 22, 2026
…y title (#327)

Refs #313. Orphan recovery demanded the scope label alone while isInFactoryScope accepts title prefix OR label, so title-scoped orphans were never recovered.

Issue reference deliberately omitted: the PR title cited (#139), which is unrelated teammate-discovery work implemented in PR #178 and must not be auto-closed by this change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kjgbot and others added 6 commits August 22, 2026 18:53
Session-Id: ded1b209-9eee-4893-9e62-15b85847e24f

Session-Id: ded1b209-9eee-4893-9e62-15b85847e24f
Session-Id: ded1b209-9eee-4893-9e62-15b85847e24f

Session-Id: ded1b209-9eee-4893-9e62-15b85847e24f
Session-Id: ded1b209-9eee-4893-9e62-15b85847e24f

Session-Id: ded1b209-9eee-4893-9e62-15b85847e24f
Session-Id: ded1b209-9eee-4893-9e62-15b85847e24f

Session-Id: ded1b209-9eee-4893-9e62-15b85847e24f
Session-Id: ded1b209-9eee-4893-9e62-15b85847e24f

Session-Id: ded1b209-9eee-4893-9e62-15b85847e24f
Session-Id: ded1b209-9eee-4893-9e62-15b85847e24f

Session-Id: ded1b209-9eee-4893-9e62-15b85847e24f
@khaliqgant
khaliqgant force-pushed the factory/139-agentworkforce-factory-49b6fd48 branch from 996b7bc to 5a53da6 Compare August 22, 2026 16:54
@khaliqgant
khaliqgant marked this pull request as ready for review August 22, 2026 16:54
@khaliqgant

Copy link
Copy Markdown
Member

Taken out of draft and rebased onto current main (0d566a3). Head is now 5a53da6.

Why this sat still. It went quiet on 2026-08-17, not because it was abandoned but because nobody owned it. It was checked for supersession against #327, which also referenced #139 — the two share zero files (src/fleet/teammates.ts et al here, src/orchestrator/factory.ts there), so the teammate-discovery work in this PR is still unshipped. Nothing on main provides src/fleet/teammates.ts, src/node/factory-persona-card.ts, or the directory-backed peer lookup.

Rebase. All 6 commits replayed onto 0d566a3 with zero conflicts, including across #327, which landed in src/orchestrator/factory.ts — no overlap with this diff.

Local verification at 5a53da6:

  • tsc -p tsconfig.build.json --noEmit — exit 0, clean
  • test/e2e/ask-a-teammate.test.ts, src/fleet/teammates.test.ts, src/node/factory-persona-card.test.ts — 3 files, 8 tests, all pass
  • full src/fleet + src/node suites (the subsystems this touches) — 11 files, 238 tests, all pass

Dependency change, called out explicitly: @agentworkforce/persona-kit moves from devDependencies to dependencies (^4.1.43), and @relaycast/a2a ^6.2.0 is added. Both are runtime requirements — persona cards are derived at node startup, not at build time. npm ci resolves both cleanly.


@coderabbitai review

@codex review

Requesting both explicitly rather than waiting for an automatic pass. CodeRabbit is an incremental reviewer and does not re-review already-reviewed commits on its own, and cubic is currently returning neutral / "AI review line limit reached" repo-wide until 1 September — so a PR can sit at zero unresolved threads with no reviewer having actually read its head. This head has not been reviewed by anyone yet; please treat the whole diff as new.

@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown

@khaliqgant I will review the complete current diff at 5a53da6. I will treat it as new work and check the teammate discovery, relay request/reply flow, persona-card publication, public API exports, and test coverage.

⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 22 minutes.

@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 50 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d6140bfd-20af-483a-8749-79cf2a046798

📥 Commits

Reviewing files that changed from the base of the PR and between ddf6486 and a6d5b7c.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (25)
  • .agentworkforce/agents/factory-feature-guardian/persona.json
  • README.md
  • package.json
  • src/cli/fleet.ts
  • src/cli/teammate-mcp.test.ts
  • src/cli/teammate-mcp.ts
  • src/fleet/create-fleet.test.ts
  • src/fleet/create-fleet.ts
  • src/fleet/internal-fleet-client.test.ts
  • src/fleet/internal-fleet-client.ts
  • src/fleet/relay-fleet-client.test.ts
  • src/fleet/relay-fleet-client.ts
  • src/fleet/teammates.test.ts
  • src/fleet/teammates.ts
  • src/index.ts
  • src/node/factory-node-runtime.ts
  • src/node/factory-node.test.ts
  • src/node/factory-node.ts
  • src/node/factory-persona-card.test.ts
  • src/node/factory-persona-card.ts
  • src/ports/fleet.ts
  • src/ports/index.ts
  • src/testing/fakes.ts
  • test/e2e/ask-a-teammate.test.ts
  • vitest.config.ts

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.

@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: 5a53da6192

ℹ️ 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 src/fleet/teammates.ts
Comment thread src/fleet/teammates.ts
Addresses both codex findings on #178.

P1 -- askTeammate could never resolve on RelayFleetClient. That backend
cannot represent `SendInput.from` (documented at its own sendMessage) and
stamps every inbound message's `target` with the authenticated identity.
The listener required `target === from`, the caller's worker name, so a
valid reply was discarded and the ask timed out. Add the optional
`FleetClient.effectiveSender()`, implement it on RelayFleetClient, and
match replies against it, falling back to `from` on a backend that
carries the requested sender faithfully.

P2 -- the same backend drops `SendInput.data`, so `requestId` never
reaches the teammate and cannot be echoed back. Two open questions to one
teammate were therefore indistinguishable, and either reply could resolve
either waiter -- including a late reply from a question that had already
timed out. Refuse the overlap on backends that cannot correlate, and
leave faithful backends unrestricted.

Verified by mutation, not by colour: restoring `target === from` fails
both new tests on a 1s timeout, which is the reported bug; removing the
overlap guard fails the P2 test alone. The pre-existing tests stay green
under the first mutant, which is why this was not caught before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Session-Id: ded1b209-9eee-4893-9e62-15b85847e24f
@khaliqgant

Copy link
Copy Markdown
Member

New head cc9436c — both codex findings fixed, each answered in its own thread with the mutation evidence.

  • P1 (teammates.ts:153): askTeammate could never resolve on RelayFleetClient, which cannot represent SendInput.from and stamps every inbound target with its authenticated identity. Added FleetClient.effectiveSender?(), implemented it on RelayFleetClient, and match replies against it. CI missed this because the e2e drives InternalFleetClient, the one backend where the bug cannot occur.
  • P2 (teammates.ts:147): requestId was sent but never checked. Echo-based correlation is structurally impossible there (data is dropped too), so overlapping asks to one teammate are now refused rather than resolved ambiguously. Faithful backends keep unrestricted concurrency.

Mutation-verified: restoring target === from fails both new tests on a 1s timeout; disabling the guard fails only the P2 test. The five pre-existing tests stay green under the first mutant — which is why this gap survived.

Local at cc9436c: tsc clean, src/fleet + src/node + src/ports + the e2e = 12 files, 242 tests, all pass.

@coderabbitai review

@codex review

CodeRabbit — your previous attempt on this PR returned "Review rate limited" with a green commit status and no review, so this head has had no CodeRabbit pass at all. Please treat the whole diff as new.

@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== PR metadata ==='
gh pr view 178 --repo AgentWorkforce/factory --json headRefOid,baseRefOid,files \
  --jq '{headRefOid, baseRefOid, files: [.files[].path]}'

echo '=== Changed-file status ==='
git diff --stat origin/main...HEAD
git diff --name-status origin/main...HEAD

echo '=== TypeScript outlines for changed source and test files ==='
fd -e ts -e json . src test .agentworkforce | sort | while read -r f; do
  case "$f" in
    src/fleet/teammates.ts|src/fleet/relay-fleet-client.ts|src/fleet/internal-fleet-client.ts|src/ports/fleet.ts|src/node/factory-persona-card.ts|src/node/factory-node.ts|src/index.ts|test/e2e/ask-a-teammate.test.ts|*.test.ts)
      echo "--- $f"
      if [[ "$f" == *.ts ]]; then ast-grep outline "$f" --items all || true; fi
      ;;
  esac
done

echo '=== Full PR diff ==='
git diff --find-renames --find-copies origin/main...HEAD -- \
  package.json package-lock.json \
  src/ports/fleet.ts src/fleet/teammates.ts src/fleet/relay-fleet-client.ts src/fleet/internal-fleet-client.ts \
  src/node/factory-persona-card.ts src/node/factory-node.ts src/index.ts \
  test/e2e/ask-a-teammate.test.ts

echo '=== Relevant symbol definitions/usages ==='
rg -n -C 4 --glob '*.ts' \
  'effectiveSender|askTeammate|discoverTeammates|sendMessage\s*\(|waitForInjected\s*\(|onAgentMessage\s*\(|deriveAgentCard|RelaycastAgentCardPublisher|startFactoryNode' \
  src test

echo '=== Dependency and test config context ==='
sed -n '1,220p' package.json
if [ -f vitest.config.ts ]; then sed -n '1,220p' vitest.config.ts; fi

Length of output: 139677


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Changed paths and diff summary ==='
git diff --name-status origin/main...HEAD
git diff --stat origin/main...HEAD

echo '=== askTeammate implementation with line numbers ==='
nl -ba src/fleet/teammates.ts | sed -n '105,215p'

echo '=== askTeammate tests with line numbers ==='
nl -ba src/fleet/teammates.test.ts | sed -n '55,255p'

echo '=== Relay sender, send, and inbound-message mapping ==='
nl -ba src/fleet/relay-fleet-client.ts | sed -n '500,575p'
rg -n -C 8 '`#send`\(|`#emitAgentMessage`|authenticatedAgentName|data:' src/fleet/relay-fleet-client.ts

echo '=== Node runtime and card-publication tests ==='
nl -ba src/node/factory-node-runtime.ts | sed -n '1,180p'
nl -ba src/node/factory-persona-card.test.ts | sed -n '1,260p'

echo '=== Directory tests ==='
nl -ba src/fleet/teammates.test.ts | sed -n '1,65p'
rg -n -C 5 'discoverTeammates|effectiveSender' src/fleet/relay-fleet-client.test.ts src/fleet/internal-fleet-client.test.ts

echo '=== Exports and package public paths ==='
git diff origin/main...HEAD -- src/index.ts package.json

Length of output: 3175


🏁 Script executed:

#!/bin/bash
set -euo pipefail

numbered() {
  awk -v start="$2" -v end="$3" 'NR >= start && NR <= end { printf "%6d\t%s\n", NR, $0 }' "$1"
}

echo '=== askTeammate implementation ==='
numbered src/fleet/teammates.ts 105 215

echo '=== askTeammate tests ==='
numbered src/fleet/teammates.test.ts 55 255

echo '=== Relay sender, send, and inbound mapping ==='
numbered src/fleet/relay-fleet-client.ts 500 575
rg -n -C 8 '`#send`\(|`#emitAgentMessage`|authenticatedAgentName|data:' src/fleet/relay-fleet-client.ts

echo '=== Factory node runtime ==='
numbered src/node/factory-node-runtime.ts 1 180

echo '=== Persona-card tests ==='
numbered src/node/factory-persona-card.test.ts 1 260

echo '=== Directory tests and client coverage ==='
numbered src/fleet/teammates.test.ts 1 65
rg -n -C 5 'discoverTeammates|effectiveSender' src/fleet/relay-fleet-client.test.ts src/fleet/internal-fleet-client.test.ts

echo '=== Supporting port and fake changes ==='
git diff origin/main...HEAD -- src/ports/index.ts src/testing/fakes.ts

Length of output: 36917


@khaliqgant I found one blocking issue in the updated reply-correlation logic.

P1 — Guard discovered targets after resolution
src/fleet/teammates.ts Line 137 enables inFlightAsks only when input.teammate is present. Calls that use the normal skill, tag, or q path do not enter the guard. Each call then resolves its target at Line 174.

On RelayFleetClient, two concurrent calls with the same query can resolve the same teammate. The transport drops requestId, and both reply listeners match the same sender and effectiveSender() target. The first reply can therefore resolve both calls with the same answer.

Resolve the teammate before creating the guard key, then guard by the resolved teammate address for every uncorrelated backend. Add a regression test that uses relayLike() with two concurrent skill: 'infra-watch' calls. The test should verify that the second call rejects and that only one message is sent.

You are interacting with an AI system.

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

ℹ️ 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 src/fleet/relay-fleet-client.ts Outdated
Comment thread src/fleet/teammates.ts Outdated
Comment thread src/fleet/teammates.ts Outdated
Second-pass codex review found three defects in the first fix. All three
were correct.

P1 -- `effectiveSender()` returned `#authenticatedAgentName`, which holds
the CONFIGURED `agentName` until something calls `agents.me()`. An
injected `messaging` or an existing `agentToken` can authenticate as a
different name, so the synchronous accessor handed out a pre-auth guess
and replies were matched against a name the server never used. Make the
port method async and resolve the identity for real before returning.

P2a -- the guard exempted "correlatable" backends, defined as those where
`replyTarget === from`. That reasoning was wrong: `AgentMessage` echoes no
request field on ANY backend, so the same requester asking one teammate
twice is ambiguous even where `from` is carried faithfully. The exemption
is removed; the claim is unconditional.

P2b -- the claim was keyed on `input.teammate` and skipped entirely when
the caller passed a skill/tag/query, so two discovery-based asks resolving
to the same teammate both armed listeners. The comment claimed the
discovery leg was covered; it was not. The claim is now taken after
discovery, against the resolved teammate.

Mutation-verified per finding: dropping the await fails 4 tests including
the async-identity case; restoring the correlatable exemption fails
exactly the same-requester and discovery cases; keying on the
caller-supplied teammate fails only the discovery-contention case.

The previous concurrency test used two DIFFERENT requesters, which is
precisely the shape that dodged P2a. It is kept, renamed for what it
actually proves, and joined by a same-requester test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Session-Id: ded1b209-9eee-4893-9e62-15b85847e24f
@khaliqgant

Copy link
Copy Markdown
Member

@codex review

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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

ℹ️ 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 src/fleet/internal-fleet-client.ts Outdated
Comment thread src/node/factory-node-runtime.ts
@khaliqgant
khaliqgant force-pushed the factory/139-agentworkforce-factory-49b6fd48 branch from a6d5b7c to ac4eb0a Compare August 23, 2026 21:31
@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai review

Requested for exact head ac4eb0a61069c01b2c9c0e3b961b258920ef97a3.

@khaliqgant

Copy link
Copy Markdown
Member

@codex review\n\nPlease review exact current head ac4eb0a61069c01b2c9c0e3b961b258920ef97a3 after the stream-rebind and node-termination fixes.

@khaliqgant

Copy link
Copy Markdown
Member

@coderabbitai review\n\nPlease review exact current head ac4eb0a61069c01b2c9c0e3b961b258920ef97a3 after the stream-rebind and node-termination fixes.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

requested_head='ac4eb0a61069c01b2c9c0e3b961b258920ef97a3'

show() {
  local file="$1" start="$2" end="$3"
  printf '=== %s:%s-%s ===\n' "$file" "$start" "$end"
  nl -ba "$file" | sed -n "${start},${end}p"
}

echo '=== Commit and PR identity ==='
printf 'checked_out='; git rev-parse HEAD
git cat-file -e "${requested_head}^{commit}"
printf 'requested='; git rev-parse "$requested_head"
if [ "$(git rev-parse HEAD)" = "$requested_head" ]; then
  echo 'exact_head_checked_out=true'
else
  echo 'exact_head_checked_out=false'
fi
gh pr view 178 --repo AgentWorkforce/factory \
  --json headRefOid,baseRefOid,isDraft,mergeable,mergeStateStatus,statusCheckRollup \
  --jq '{headRefOid,baseRefOid,isDraft,mergeable,mergeStateStatus,checks:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}'

echo '=== Requested commit and aggregate diff ==='
git show --format=fuller --stat "$requested_head"
git diff --name-status origin/main..."$requested_head"
git diff --stat origin/main..."$requested_head"
git diff --unified=60 "${requested_head}^" "$requested_head"

echo '=== Outline changed TypeScript files ==='
git diff --name-only "${requested_head}^" "$requested_head" -- '*.ts' | sort | while read -r file; do
  echo "--- $file"
  ast-grep outline "$file" --items all || true
done

echo '=== Stream identity, rebind, and node termination references ==='
rg -n -C 12 --glob '*.ts' \
  '\b(messageStreamIdentity|messageStreamScope|rebind|rebinds?|stream|askTeammate|inFlightAsks|onAgentMessage|whenMessagesObservable|terminate|termination|onRegistered|startFactoryNode|dispose|close)\b' \
  src test

echo '=== Relevant implementation files ==='
for file in \
  src/fleet/teammates.ts \
  src/fleet/internal-fleet-client.ts \
  src/fleet/relay-fleet-client.ts \
  src/ports/fleet.ts \
  src/node/factory-node-runtime.ts \
  src/node/factory-node.ts \
  src/node/factory-persona-card.ts \
  src/fleet/teammates.test.ts \
  src/fleet/internal-fleet-client.test.ts \
  src/node/factory-node-runtime.test.ts \
  src/node/factory-persona-card.test.ts \
  test/e2e/ask-a-teammate.test.ts
do
  if [ -f "$file" ]; then
    echo "--- $file"
    ast-grep outline "$file" --items all || true
  fi
done

Length of output: 198546


🏁 Script executed:

#!/bin/bash
set -euo pipefail

requested_head='ac4eb0a61069c01b2c9c0e3b961b258920ef97a3'

show() {
  local file="$1" start="$2" end="$3"
  printf '=== %s:%s-%s ===\n' "$file" "$start" "$end"
  nl -ba "$file" | sed -n "${start},${end}p"
}

echo '=== Exact latest commit ==='
git show --format='%H%n%P%n%s%n%b' --stat "$requested_head"
git diff --unified=45 "${requested_head}^" "$requested_head" -- \
  src/fleet/internal-fleet-client.ts \
  src/fleet/internal-fleet-client.reconnect.test.ts \
  src/fleet/internal-fleet-client.test.ts \
  src/fleet/teammates.ts \
  src/ports/fleet.ts

echo '=== Reconnect implementation ==='
rg -n -C 22 \
  '`#callBroker`|`#reconnectIfBrokerChanged`|`#readConnectionFile`|`#connectionFilePid`|messageStreamIdentity|dispose\(\)|`#ensureEventSubscription`|`#connectedBroker`|`#disposed`' \
  src/fleet/internal-fleet-client.ts

echo '=== Exact implementation slices ==='
show src/fleet/internal-fleet-client.ts 130 285
show src/fleet/internal-fleet-client.ts 580 810
show src/fleet/internal-fleet-client.ts 940 1120
show src/fleet/internal-fleet-client.ts 1120 1335

echo '=== Reconnect test file ==='
show src/fleet/internal-fleet-client.reconnect.test.ts 1 520

echo '=== Node termination regression tests ==='
rg -n -C 20 \
  'markAgentTerminal|terminal|dispose\(\)|owned broker|spawned agents to exit|release.*terminal' \
  src/fleet/internal-fleet-client.test.ts

echo '=== Exact-head checks ==='
gh pr view 178 --repo AgentWorkforce/factory --json headRefOid,statusCheckRollup \
  --jq '{headRefOid,checks:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}'

Length of output: 71756


🏁 Script executed:

#!/bin/bash
set -euo pipefail

requested_head='ac4eb0a61069c01b2c9c0e3b961b258920ef97a3'

show() {
  file="$1"; start="$2"; end="$3"
  printf '=== %s:%s-%s ===\n' "$file" "$start" "$end"
  awk -v start="$start" -v end="$end" \
    'NR >= start && NR <= end { printf "%6d  %s\n", NR, $0 }' "$file"
}

echo '=== Node runtime latest delta ==='
git diff --unified=55 "${requested_head}^" "$requested_head" -- \
  src/node/factory-node-runtime.ts \
  src/node/factory-persona-card.test.ts

echo '=== Node runtime implementation and all call sites ==='
show src/node/factory-node-runtime.ts 1 220
rg -n -C 14 --glob '*.ts' \
  '\b(startFactoryNode|onRegistered|onTerminated|onTerminate|cardPublished|publishAgentCard|RunningFactoryNode|dispose)\b' \
  src test

echo '=== Persona runtime regressions ==='
show src/node/factory-persona-card.test.ts 1 360

echo '=== Internal disposal and rebind implementations ==='
show src/fleet/internal-fleet-client.ts 530 890
show src/fleet/internal-fleet-client.ts 1170 1325

echo '=== Rebind and stream-identity regression sections ==='
show src/fleet/internal-fleet-client.reconnect.test.ts 300 470
show src/fleet/internal-fleet-client.test.ts 1650 1785

echo '=== Exact-head checks ==='
gh pr view 178 --repo AgentWorkforce/factory --json headRefOid,statusCheckRollup \
  --jq '{headRefOid,checks:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}'

Length of output: 167588


🏁 Script executed:

#!/bin/bash
set -euo pipefail

requested_head='ac4eb0a61069c01b2c9c0e3b961b258920ef97a3'

show() {
  file="$1"; start="$2"; end="$3"
  printf '=== %s:%s-%s ===\n' "$file" "$start" "$end"
  awk -v start="$start" -v end="$end" \
    'NR >= start && NR <= end { printf "%6d  %s\n", NR, $0 }' "$file"
}

echo '=== Complete node runtime ==='
show src/node/factory-node-runtime.ts 1 220

echo '=== Exact node-runtime delta ==='
git diff --unified=75 "${requested_head}^" "$requested_head" -- src/node/factory-node-runtime.ts

echo '=== Node runtime test cases ==='
rg -n -A75 -B12 \
  'rejects card publication when node startup terminates|rejects card publication on a pre-registration stop|derives a shared-schema card|retries a transient card-publication' \
  src/node/factory-persona-card.test.ts

echo '=== RunningNode and startServeNode type contract ==='
rg -n -C 8 --glob '*.ts' \
  '\binterface RunningNode\b|\btype RunningNode\b|\bstartServeNode\b|\bdone:\b|\bstop:\b' \
  src/node src --glob '!src/node/factory-persona-card.test.ts'

echo '=== Current status check summary ==='
gh pr view 178 --repo AgentWorkforce/factory --json headRefOid,statusCheckRollup \
  --jq '{headRefOid,checks:[.statusCheckRollup[]|{name,status,conclusion}]}'

Length of output: 28896


@khaliqgant I reviewed exact head ac4eb0a61069c01b2c9c0e3b961b258920ef97a3.

P1 — Reject cardPublished when node termination occurs during a hung publication

In src/node/factory-node-runtime.ts Line 44, markNodeTerminated() rejects cardPublished only when publicationInFlight is false.

If publishAgentCard() never settles, and then running.done settles or running.stop() runs, cardPublished remains pending forever. The node cannot receive another registration edge, so the operation cannot retry. Consumers that await cardPublished hang after node termination.

Reject cardPublished immediately when the node terminates, including when publication is in flight. Ignore a later publisher completion because the node is already terminal.

Add a regression where publishAgentCard() stays pending, then running.done resolves or running.stop() completes. Assert that cardPublished rejects promptly.

The stream-rebind change is otherwise sound. The connection-file scope remains stable across broker replacement, and the stale-call retry guard stops broker activity after dispose() begins.

The exact-head package, kubernetes-provider-e2e, load-e2e, verification-gate-e2e, and verification-stack-e2e jobs are still in progress.

You are interacting with an AI system.

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

ℹ️ 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 src/fleet/internal-fleet-client.ts Outdated
Comment thread src/fleet/teammates.ts
@khaliqgant
khaliqgant force-pushed the factory/139-agentworkforce-factory-49b6fd48 branch from ac4eb0a to d6c3605 Compare August 23, 2026 21:54
@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai review

Requested for exact head d6c3605e11bfc2c5d23be7f0e4970149bf62edf1.

@khaliqgant

Copy link
Copy Markdown
Member

@codex review\n\nPlease review exact current head d6c3605e11bfc2c5d23be7f0e4970149bf62edf1 after the explicit-scope precedence and pending-delivery claim-retention fixes.

@khaliqgant

Copy link
Copy Markdown
Member

@coderabbitai review\n\nPlease review exact current head d6c3605e11bfc2c5d23be7f0e4970149bf62edf1 after the explicit-scope precedence and pending-delivery claim-retention fixes.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

requested_head='d6c3605e11bfc2c5d23be7f0e4970149bf62edf1'

show() {
  local file="$1" start="$2" end="$3"
  printf '=== %s:%s-%s ===\n' "$file" "$start" "$end"
  nl -ba "$file" | sed -n "${start},${end}p"
}

echo '=== Commit and PR identity ==='
printf 'checked_out='; git rev-parse HEAD
git cat-file -e "${requested_head}^{commit}"
printf 'requested='; git rev-parse "$requested_head"
if [ "$(git rev-parse HEAD)" = "$requested_head" ]; then
  echo 'exact_head_checked_out=true'
else
  echo 'exact_head_checked_out=false'
fi
gh pr view 178 --repo AgentWorkforce/factory \
  --json headRefOid,baseRefOid,isDraft,mergeable,mergeStateStatus \
  --jq '{headRefOid,baseRefOid,isDraft,mergeable,mergeStateStatus}'

echo '=== Requested commit and aggregate diff ==='
git show --format=fuller --stat "$requested_head"
git diff --name-status origin/main..."$requested_head"
git diff --stat origin/main..."$requested_head"
git diff --unified=60 "${requested_head}^" "$requested_head" -- \
  src/fleet/teammates.ts \
  src/fleet/teammates.test.ts \
  src/fleet/internal-fleet-client.ts \
  src/fleet/internal-fleet-client.test.ts \
  src/fleet/relay-fleet-client.ts \
  src/fleet/relay-fleet-client.test.ts \
  src/ports/fleet.ts \
  src/testing/fakes.ts \
  test/e2e/ask-a-teammate.test.ts

echo '=== Target file outlines ==='
for file in \
  src/fleet/teammates.ts \
  src/fleet/teammates.test.ts \
  src/fleet/internal-fleet-client.ts \
  src/fleet/internal-fleet-client.test.ts \
  src/fleet/relay-fleet-client.ts \
  src/ports/fleet.ts \
  src/testing/fakes.ts \
  test/e2e/ask-a-teammate.test.ts
do
  if [ -f "$file" ]; then
    echo "--- $file"
    ast-grep outline "$file" --items all || true
  fi
done

echo '=== Claim scope, delivery wait, and failure lifecycle ==='
rg -n -C 14 --glob '*.ts' \
  '\b(askTeammate|messageStreamIdentity|messageStreamScope|inFlightAsks|claim|pending|waitForInjected|onDeliveryFailed|FleetDeliveryRejectedError|delivery_failed|message_delivery_failed|sendMessage|onAgentMessage|whenMessagesObservable)\b' \
  src test

echo '=== Focused implementations ==='
show src/fleet/teammates.ts 80 380
show src/fleet/internal-fleet-client.ts 600 780
show src/ports/fleet.ts 100 280

echo '=== Relevant regression tests ==='
rg -n -C 22 --glob '*.test.ts' \
  '\b(explicit.*scope|scope.*precedence|pending.*delivery|delivery.*pending|claim.*retention|messageStreamIdentity|waitForInjected|concurrent|reject)\b' \
  src/fleet src test

echo '=== Exact-head checks ==='
gh pr view 178 --repo AgentWorkforce/factory --json headRefOid,statusCheckRollup \
  --jq '{headRefOid,checks:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}'

Length of output: 203386


🏁 Script executed:

#!/bin/bash
set -euo pipefail

requested_head='d6c3605e11bfc2c5d23be7f0e4970149bf62edf1'

echo '=== Identity ==='
printf 'checked_out='; git rev-parse HEAD
printf 'requested='; git rev-parse "$requested_head"
if [ "$(git rev-parse HEAD)" = "$requested_head" ]; then
  echo 'exact_head_checked_out=true'
else
  echo 'exact_head_checked_out=false'
fi
gh pr view 178 --repo AgentWorkforce/factory \
  --json headRefOid,mergeable,mergeStateStatus,isDraft \
  --jq '{headRefOid,mergeable,mergeStateStatus,isDraft}'

echo '=== Exact latest delta ==='
git show --format='%H%n%P%n%s%n%b' --stat "$requested_head"
git diff --unified=45 "${requested_head}^" "$requested_head" -- \
  src/fleet/teammates.ts \
  src/fleet/teammates.test.ts \
  src/fleet/internal-fleet-client.ts \
  src/fleet/internal-fleet-client.test.ts \
  src/ports/fleet.ts \
  src/testing/fakes.ts

echo '=== Target outlines ==='
for file in src/fleet/teammates.ts src/fleet/teammates.test.ts src/fleet/internal-fleet-client.ts src/ports/fleet.ts; do
  echo "--- $file"
  ast-grep outline "$file" --items all || true
done

echo '=== askTeammate implementation ==='
sed -n '90,330p' src/fleet/teammates.ts

echo '=== Claim scope helpers ==='
sed -n '330,460p' src/fleet/teammates.ts

echo '=== Internal client delivery and stream identity ==='
rg -n -C 14 \
  'messageStreamIdentity\s*\(|messageStreamScope|waitForInjected\s*\(|FleetDeliveryRejectedError|delivery_failed|message_delivery_failed|pendingInjected' \
  src/fleet/internal-fleet-client.ts
sed -n '620,760p' src/fleet/internal-fleet-client.ts

echo '=== FleetClient contract ==='
sed -n '120,260p' src/ports/fleet.ts

echo '=== Targeted teammate regressions ==='
rg -n -C 20 \
  'explicit.*scope|scope.*precedence|pending.*delivery|delivery.*pending|claim.*retain|claim.*release|waitForInjected|concurrent|second ask' \
  src/fleet/teammates.test.ts

echo '=== Targeted internal-client regressions ==='
rg -n -C 18 \
  'messageStreamIdentity|messageStreamScope|pending.*delivery|delivery.*pending|waitForInjected' \
  src/fleet/internal-fleet-client.test.ts

echo '=== Exact-head checks ==='
gh pr view 178 --repo AgentWorkforce/factory --json headRefOid,statusCheckRollup \
  --jq '{headRefOid,checks:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}'

Length of output: 148979


@khaliqgant I reviewed exact head d6c3605e11bfc2c5d23be7f0e4970149bf62edf1.

P1 — Keep the claim after an early reply if delivery confirmation fails ambiguously

src/fleet/teammates.ts releases claimed in the finally block after an early reply, when waitForInjected() later rejects with an ordinary confirmation timeout.

A waitForInjected() timeout does not prove that the first send, or a readiness-triggered resend, was not delivered. The teammate can still send a later answer to the first question. A second askTeammate() can then start and accept that late uncorrelated answer.

Quarantine the pair when an early reply has settled the ask but the delivery operation ends with any non-FleetDeliveryRejectedError failure. Release the pair only after confirmed delivery or a definitive correlated delivery failure.

Add a regression with this sequence:

  1. The first ask receives an early matching reply while waitForInjected() remains pending.
  2. waitForInjected() rejects with Timed out waiting for delivery_injected ....
  3. A second ask to the same requester-teammate pair is rejected and sends no message.

The explicit messageStreamScope precedence is correct. The exact-head CI jobs are still in progress, so I cannot approve.

You are interacting with an AI system.

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

ℹ️ 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 src/fleet/teammates.ts Outdated
@khaliqgant
khaliqgant force-pushed the factory/139-agentworkforce-factory-49b6fd48 branch from d6c3605 to 0e8801a Compare August 23, 2026 22:08
@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai review

Requested for exact head 0e8801a0b33fbf95f0f827c8dd79667361d1574e.

@khaliqgant
khaliqgant force-pushed the factory/139-agentworkforce-factory-49b6fd48 branch from 0e8801a to 5ed5157 Compare August 23, 2026 22:08
@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai review

Requested for exact head 5ed51578a8304f9c9bf7b7899d96b98e4be4c480.

@khaliqgant

Copy link
Copy Markdown
Member

@codex review

Please review exact current head 5ed5157, including the in-flight readiness-resend settlement fix and regression.

@khaliqgant

Copy link
Copy Markdown
Member

@coderabbitai review

Please review exact current head 5ed5157, including the in-flight readiness-resend settlement fix and regression.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

requested_head='5ed51578a8304f9c9bf7b7899d96b98e4be4c480'

show() {
  local file="$1" start="$2" end="$3"
  printf '=== %s:%s-%s ===\n' "$file" "$start" "$end"
  nl -ba "$file" | sed -n "${start},${end}p"
}

echo '=== Commit and pull request identity ==='
printf 'checked_out='; git rev-parse HEAD
git cat-file -e "${requested_head}^{commit}"
printf 'requested='; git rev-parse "$requested_head"
if [ "$(git rev-parse HEAD)" = "$requested_head" ]; then
  echo 'exact_head_checked_out=true'
else
  echo 'exact_head_checked_out=false'
fi
gh pr view 178 --repo AgentWorkforce/factory \
  --json headRefOid,baseRefOid,isDraft,mergeable,mergeStateStatus \
  --jq '{headRefOid,baseRefOid,isDraft,mergeable,mergeStateStatus}'

echo '=== Requested commit delta ==='
git show --format=fuller --stat "$requested_head"
git diff --unified=70 "${requested_head}^" "$requested_head" -- \
  src/fleet/teammates.ts \
  src/fleet/teammates.test.ts \
  src/fleet/internal-fleet-client.ts \
  src/fleet/internal-fleet-client.test.ts \
  src/ports/fleet.ts \
  src/testing/fakes.ts \
  test/e2e/ask-a-teammate.test.ts

echo '=== Relevant source structure ==='
for file in \
  src/fleet/teammates.ts \
  src/fleet/teammates.test.ts \
  src/fleet/internal-fleet-client.ts \
  src/fleet/internal-fleet-client.test.ts \
  src/ports/fleet.ts \
  src/testing/fakes.ts \
  test/e2e/ask-a-teammate.test.ts
do
  if [ -f "$file" ]; then
    echo "--- $file"
    ast-grep outline "$file" --items all || true
  fi
done

echo '=== askTeammate implementation and helpers ==='
show src/fleet/teammates.ts 80 440

echo '=== Resend, readiness, delivery, and claim references ==='
rg -n -C 16 --glob '*.ts' \
  '\b(askTeammate|waitForInjected|whenMessagesObservable|sendMessage|readiness|resend|delivery|FleetDeliveryRejectedError|claim|quarantine|settle|settled|onAgentMessage|messageStreamIdentity)\b' \
  src/fleet src/ports src/testing test

echo '=== Focused regression tests ==='
rg -n -C 28 --glob '*.test.ts' \
  '\b(readiness|resend|in.flight|in-flight|early reply|delivery.*timeout|timeout.*delivery|claim.*retain|second ask|concurrent)\b' \
  src/fleet/teammates.test.ts src/fleet/internal-fleet-client.test.ts test/e2e/ask-a-teammate.test.ts

echo '=== Exact-head checks ==='
gh pr view 178 --repo AgentWorkforce/factory --json headRefOid,statusCheckRollup \
  --jq '{headRefOid,checks:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}'

Length of output: 92329


🏁 Script executed:

#!/bin/bash
set -euo pipefail

requested_head='5ed51578a8304f9c9bf7b7899d96b98e4be4c480'

show() {
  local file="$1" start="$2" end="$3"
  printf '=== %s:%s-%s ===\n' "$file" "$start" "$end"
  awk -v start="$start" -v end="$end" \
    'NR >= start && NR <= end { printf "%6d  %s\n", NR, $0 }' "$file"
}

echo '=== Exact current delta ==='
git diff --unified=45 "${requested_head}^" "$requested_head" -- \
  src/fleet/internal-fleet-client.ts \
  src/fleet/internal-fleet-client.test.ts \
  src/fleet/teammates.ts \
  src/fleet/teammates.test.ts

echo '=== waitForInjected creation and resend implementation ==='
rg -n -C 18 \
  'async waitForInjected|`#triggerReadyResend`|async `#resendPendingInjected`|`#resolvePendingInjected`|`#settlePendingInjected`|`#rejectPendingInjected`|`#failPendingInjectedEvent`|`#markAgentReady`' \
  src/fleet/internal-fleet-client.ts

echo '=== Full delivery waiter region ==='
show src/fleet/internal-fleet-client.ts 600 780
show src/fleet/internal-fleet-client.ts 920 1075

echo '=== askTeammate settlement and claim lifecycle ==='
show src/fleet/teammates.ts 155 320

echo '=== Regression tests for a blocked resend and claim retention ==='
rg -n -C 35 \
  'keeps a confirmed injection pending until its readiness re-send returns|retains the pair claim after an early reply until delivery retries have stopped|disposes a logical waiter while its readiness re-send response is blocked|keeps waiting when the first event fails' \
  src/fleet/internal-fleet-client.test.ts src/fleet/teammates.test.ts

echo '=== All readiness-resend test names ==='
rg -nP '^\s*it\(' src/fleet/internal-fleet-client.test.ts | rg -i -C2 'resend|injection|delivery'

echo '=== Exact-head GitHub checks ==='
gh pr view 178 --repo AgentWorkforce/factory --json headRefOid,statusCheckRollup \
  --jq '{headRefOid,checks:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}'

Length of output: 119408


@khaliqgant I reviewed exact head 5ed51578a8304f9c9bf7b7899d96b98e4be4c480.

I found no new blocking issue in the in-flight readiness-resend settlement change.

InternalFleetClient now stores a positive injection confirmation while the readiness resend is in flight. It clears the delivery timeout and keeps waitForInjected() pending. When the resend returns or fails, it settles with the stored confirmation.

askTeammate() retains the pair claim after an early reply until the delivery operation settles. This prevents a second ask while the resend can still deliver the first question.

The new regressions cover both conditions:

  • A confirmed original injection does not settle before the blocked resend returns.
  • A reply that arrives before delivery settlement retains the pair claim until delivery retries stop.

The request check passed. The package and five E2E-related checks are still in progress. I cannot approve until those checks complete successfully.


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

You are interacting with an AI system.

@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: 5ed51578a8

ℹ️ 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 src/fleet/internal-fleet-client.ts
Comment thread src/fleet/internal-fleet-client.ts Outdated
@khaliqgant
khaliqgant force-pushed the factory/139-agentworkforce-factory-49b6fd48 branch from 5ed5157 to 0075364 Compare August 23, 2026 22:24
@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai review

Requested for exact head 0075364ec248a2c0a5db5ebdc96297a949e854e2.

@khaliqgant

Copy link
Copy Markdown
Member

@coderabbitai review

Please review exact current head 0075364, especially permanent claim quarantine after a possible duplicate resend and canonical broker URL stream identities.

@khaliqgant

Copy link
Copy Markdown
Member

@codex review

Please review exact current head 0075364, especially permanent claim quarantine after a possible duplicate resend and canonical broker URL stream identities.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

requested_head='0075364ec248a2c0a5db5ebdc96297a949e854e2'

show() {
  local file="$1" start="$2" end="$3"
  printf '\n=== %s:%s-%s ===\n' "$file" "$start" "$end"
  nl -ba "$file" | sed -n "${start},${end}p"
}

echo '=== Commit and PR identity ==='
printf 'checked_out='; git rev-parse HEAD
git cat-file -e "${requested_head}^{commit}"
printf 'requested='; git rev-parse "$requested_head"
if [ "$(git rev-parse HEAD)" = "$requested_head" ]; then
  echo 'exact_head_checked_out=true'
else
  echo 'exact_head_checked_out=false'
fi
gh pr view 178 --repo AgentWorkforce/factory \
  --json headRefOid,baseRefOid,isDraft,mergeable,mergeStateStatus,statusCheckRollup \
  --jq '{headRefOid,baseRefOid,isDraft,mergeable,mergeStateStatus,checks:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}'

echo '=== Requested commit and latest delta ==='
git show --format=fuller --stat "$requested_head"
git diff --unified=65 "${requested_head}^" "$requested_head" -- \
  src/fleet/teammates.ts \
  src/fleet/teammates.test.ts \
  src/fleet/internal-fleet-client.ts \
  src/fleet/internal-fleet-client.test.ts \
  src/fleet/internal-fleet-client.reconnect.test.ts \
  src/fleet/relay-fleet-client.ts \
  src/fleet/relay-fleet-client.test.ts \
  src/ports/fleet.ts \
  src/testing/fakes.ts \
  test/e2e/ask-a-teammate.test.ts

echo '=== Relevant source outlines ==='
for file in \
  src/fleet/teammates.ts \
  src/fleet/teammates.test.ts \
  src/fleet/internal-fleet-client.ts \
  src/fleet/internal-fleet-client.test.ts \
  src/fleet/internal-fleet-client.reconnect.test.ts \
  src/fleet/relay-fleet-client.ts \
  src/fleet/relay-fleet-client.test.ts \
  src/ports/fleet.ts \
  src/testing/fakes.ts \
  test/e2e/ask-a-teammate.test.ts
do
  if [ -f "$file" ]; then
    echo "--- $file"
    ast-grep outline "$file" --items all || true
  fi
done

echo '=== Claim, resend, and stream-scope references ==='
rg -n -C 14 --glob '*.ts' \
  '\b(askTeammate|inFlightAsks|claim|quarantine|permanent|timedOut|resend|readiness|waitForInjected|delivery|FleetDeliveryRejectedError|messageStreamIdentity|messageStreamScope|brokerUrl|canonical|URL|onAgentMessage|whenMessagesObservable)\b' \
  src/fleet src/ports src/testing test

echo '=== Focused implementations ==='
show src/fleet/teammates.ts 80 460
show src/fleet/internal-fleet-client.ts 1 360
show src/fleet/internal-fleet-client.ts 580 1110
show src/fleet/relay-fleet-client.ts 1 260
show src/ports/fleet.ts 100 290

echo '=== Focused regression tests ==='
rg -n -C 26 --glob '*.test.ts' \
  '\b(duplicate|resend|readiness|permanent|quarantine|claim|canonical|broker.*URL|URL.*broker|messageStreamIdentity|same.stream|distinct.stream)\b' \
  src/fleet src test

Length of output: 198236


🏁 Script executed:

#!/bin/bash
set -euo pipefail

requested_head='0075364ec248a2c0a5db5ebdc96297a949e854e2'

show() {
  local file="$1" start="$2" end="$3"
  printf '\n=== %s:%s-%s ===\n' "$file" "$start" "$end"
  awk -v start="$start" -v end="$end" \
    'NR >= start && NR <= end { printf "%6d  %s\n", NR, $0 }' "$file"
}

echo '=== Exact identity and checks ==='
printf 'checked_out='; git rev-parse HEAD
printf 'requested='; git rev-parse "$requested_head"
gh pr view 178 --repo AgentWorkforce/factory --json headRefOid,statusCheckRollup \
  --jq '{headRefOid,checks:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}'

echo '=== Exact commit delta ==='
git diff --unified=75 "${requested_head}^" "$requested_head" -- \
  src/fleet/teammates.ts \
  src/fleet/teammates.test.ts \
  src/fleet/internal-fleet-client.ts \
  src/fleet/internal-fleet-client.test.ts \
  src/fleet/relay-fleet-client.ts \
  src/fleet/relay-fleet-client.test.ts

echo '=== askTeammate claim and delivery lifecycle ==='
show src/fleet/teammates.ts 163 332

echo '=== Internal broker stream identity implementation ==='
rg -n -C 35 \
  'messageStreamIdentity\s*\(|canonical.*[Bb]roker|[Bb]roker.*[Uu][Rr][Ll]|baseUrl|connection.*fingerprint|createHash' \
  src/fleet/internal-fleet-client.ts

echo '=== URL canonicalization implementations ==='
rg -n -C 16 \
  'function canonical|canonical.*[Uu][Rr][Ll]|new URL\(|toString\(\).*replace|baseUrl' \
  src/fleet/internal-fleet-client.ts src/fleet/relay-fleet-client.ts

echo '=== New and nearby claim regressions ==='
show src/fleet/teammates.test.ts 450 555
show src/fleet/internal-fleet-client.test.ts 1760 1855

echo '=== URL identity test inventory ==='
rg -n -C 12 \
  'broker URL|broker.*url|canonical|trailing|default port|localhost|127\.0\.0\.1|messageStreamIdentity' \
  src/fleet/internal-fleet-client.test.ts src/fleet/relay-fleet-client.test.ts

Length of output: 231480


@khaliqgant I reviewed exact head 0075364ec248a2c0a5db5ebdc96297a949e854e2.

I found no new blocking issue.

InternalFleetClient marks a delivery as duplicateDeliveryPossible after a readiness resend can have been accepted. askTeammate() then keeps a permanent duplicate-uncorrelated claim after the first reply. A late duplicate reply cannot open the requester-teammate pair. The regression verifies that later asks are rejected and no second message is sent.

InternalFleetClient.messageStreamIdentity() now hashes the canonical new URL(baseUrl).href value. The regression confirms that broker URLs with and without a trailing slash share one claim scope.

The exact-head request check passed. The package, kubernetes-provider-e2e, load-e2e, verification-gate-e2e, and verification-stack-e2e checks were still in progress. I cannot approve until they pass.

You are interacting with an AI system.

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.

[factory] "Who can I ask?" — discover teammate agents by skill and engage them; publish cards for hosted personas

2 participants