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 25 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 25 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, quarantine uncorrelated timeouts, and surface definitive delivery rejections.

  • Discovery and ask: FleetClient.discoverTeammates({ skill | tag | q }) queries a Relaycast directory via RelaycastTeammateDirectory. askTeammate(...) awaits message transport readiness and the socket handshake, ignores pre‑send replies, prevents overlapping asks to the same teammate from the same requester (shared across clients), 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; includes src/cli/teammate-mcp.ts and tests.

  • 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, canonicalize connection stream paths and missing receipt parents, fail closed on unknown streams, and preserve identity across rebinds. createFleet accepts messageStreamScope and forwards it; pass RELAY_WORKSPACE_KEY as both RELAY_WORKSPACE_KEY and RELAY_API_KEY.

  • Tests: 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, reply identity claims, transport readiness/handshake, timeouts/quarantine, 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 f40b482. 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 35 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: 483cdc28-1164-4f98-b6ab-cbe39f94e2f0

📥 Commits

Reviewing files that changed from the base of the PR and between 628bcc6 and f40b482.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (26)
  • .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.reconnect.test.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

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.

@khaliqgant

Copy link
Copy Markdown
Member

@codex review

Please review exact current head 0075364. The previous exact-head request was rejected by the usage limit; this head has not received qualifying Codex coverage.

@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

const PID_RESOLVE_ATTEMPTS = 3

P2 Badge Resolve symlinks before hashing connection scopes

When two clients refer to the same connection.json through a real path and a symlinked path, resolve(path) preserves those distinct spellings even though both clients attach to the same broker and observe the same inbound stream. They consequently receive separate claim registries, allowing concurrent asks for the same requester and teammate to consume the same uncorrelated reply; canonicalize an existing connection file with its real path before hashing it.

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

@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai review

Requested for exact head 1d09656420262fb02a1b98b3342dbfa29260dcc9.

@khaliqgant

Copy link
Copy Markdown
Member

Addressed the exact-head Codex P2 in 1d09656. Connection-file stream identity now resolves an existing path through realpathSync.native before hashing, so a real path and any symlink spelling share one claim registry. Missing startup-time files retain the prior stable absolute-path fallback. The regression constructs a real connection.json plus a symlink and proves both clients expose the same stream identity. Full internal-fleet-client suite: 74/74; build and feature-map (323) pass.

@khaliqgant

Copy link
Copy Markdown
Member

@coderabbitai review

Please review exact current head 1d09656 after the symlink-scope fix.

@khaliqgant

Copy link
Copy Markdown
Member

@codex review

Please review exact current head 1d09656 after the symlink-scope fix.

@coderabbitai

coderabbitai Bot commented Aug 24, 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='1d09656420262fb02a1b98b3342dbfa29260dcc9'

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 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,statusCheckRollup \
  --jq '{headRefOid,baseRefOid,isDraft,mergeable,mergeStateStatus,checks:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}'

echo '=== Requested commit and exact delta ==='
git show --format=fuller --stat "$requested_head"
git diff --unified=70 "${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 \
  src/ports/fleet.ts

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

echo '=== Connection-file stream identity implementation and call sites ==='
rg -n -C 28 --glob '*.ts' \
  '\b(realpathSync|connectionFile|connection-file|messageStreamIdentity|canonical.*[Pp]ath|resolve.*[Pp]ath|createHash|broker.*[Uu][Rr][Ll])\b' \
  src/fleet src/ports src/testing test

echo '=== Claim lifecycle and message stream scope ==='
rg -n -C 18 --glob '*.ts' \
  '\b(askTeammate|inFlightAsks|claim|quarantine|duplicateDeliveryPossible|waitForInjected|messageStreamIdentity|messageStreamScope)\b' \
  src/fleet src/ports src/testing test

echo '=== Symlink identity regression and nearby connection-file tests ==='
rg -n -C 40 --glob '*.test.ts' \
  '\b(symlink|realpath|connection.*file|messageStreamIdentity|missing.*file|stable.*path|reconnect)\b' \
  src/fleet/internal-fleet-client.test.ts \
  src/fleet/internal-fleet-client.reconnect.test.ts \
  src/fleet/teammates.test.ts

echo '=== Focused implementation slices ==='
show src/fleet/internal-fleet-client.ts 1 220
show src/fleet/internal-fleet-client.ts 1040 1335
show src/fleet/teammates.ts 130 370

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

Length of output: 203055


🏁 Script executed:

#!/bin/bash
set -euo pipefail

requested_head='1d09656420262fb02a1b98b3342dbfa29260dcc9'

slice() {
  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 commit identity ==='
printf 'checked_out='; git rev-parse HEAD
printf 'requested='; git rev-parse "$requested_head"
git show --format='%H%n%P%n%s%n%b' --stat "$requested_head"

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

echo '=== Imports and identity implementation ==='
rg -n -C 8 --glob '*.ts' \
  'realpathSync|messageStreamIdentity\s*\(|connectionPath|connectionFilePath|resolve\(' \
  src/fleet/internal-fleet-client.ts

identity_line=$(rg -n 'messageStreamIdentity\s*\(' src/fleet/internal-fleet-client.ts | head -n1 | cut -d: -f1)
if [ -n "${identity_line:-}" ]; then
  start=$(( identity_line > 50 ? identity_line - 50 : 1 ))
  end=$(( identity_line + 110 ))
  slice src/fleet/internal-fleet-client.ts "$start" "$end"
fi

echo '=== Path and connection-file helper implementations ==='
rg -n -C 20 --glob '*.ts' \
  'realpathSync|resolve\(|readFileSync|readFile\(|connection.*[Pp]ath|connection.*[Ff]ile' \
  src/fleet/internal-fleet-client.ts

echo '=== Exact regression ==='
test_line=$(rg -n 'real and symlinked connection paths' src/fleet/internal-fleet-client.test.ts | cut -d: -f1)
if [ -n "${test_line:-}" ]; then
  start=$(( test_line > 25 ? test_line - 25 : 1 ))
  end=$(( test_line + 75 ))
  slice src/fleet/internal-fleet-client.test.ts "$start" "$end"
fi

echo '=== Reconnect identity stability regression ==='
rg -n -C 18 --glob '*.test.ts' \
  'streamIdentityBeforeRebind|messageStreamIdentity\(\).*rebind|rebind.*messageStreamIdentity' \
  src/fleet/internal-fleet-client.reconnect.test.ts

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

Length of output: 71223


@khaliqgant I reviewed exact head 1d09656420262fb02a1b98b3342dbfa29260dcc9.

I found no new blocking issue in the symlink-scope fix.

connectionFileStreamIdentity() now resolves an existing connection-file path with realpathSync.native() before it hashes the path. Clients configured with the real path and a symlink therefore use one message-stream claim scope.

If the file does not exist during startup, the code retains the stable absolute-path fallback. This preserves the prior startup behavior.

The new regression creates connection.json and a symlink. It verifies that both InternalFleetClient instances return the same messageStreamIdentity() value. The existing reconnect regression still verifies that the identity remains stable after a broker rebind.

I cannot approve this head. GitHub currently reports merge conflicts (mergeable: CONFLICTING, mergeStateStatus: DIRTY). Only the request check has completed successfully.

You are interacting with an AI system.

…force-factory-49b6fd48

# Conflicts:
#	src/fleet/relay-fleet-client.ts
#	src/ports/fleet.ts
@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai review

Requested for exact head a4cfb87d1a25393f98dbd4a1bc4ce434bedd2fc7.

@khaliqgant

Copy link
Copy Markdown
Member

@codex review

Please review exact current head a4cfb87 after the main sync and additive conflict resolution.

@khaliqgant

Copy link
Copy Markdown
Member

@coderabbitai review

Please review exact current head a4cfb87 after the main sync and additive conflict resolution.

@khaliqgant

Copy link
Copy Markdown
Member

Current exact head is a4cfb87 after merging origin/main and resolving the two additive overlaps: RelayFleetClient now keeps both exact message-observability readiness and the new fleet-connect health state, and FleetClient exposes both teammate discovery and optional connect status. The symlink canonicalization fix remains unchanged. Post-merge fleet selection passes 182/182; build and feature-map (323) pass; diff check is clean.

@coderabbitai

coderabbitai Bot commented Aug 24, 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

head='a4cfb87d1a25393f98dbd4a1bc4ce434bedd2fc7'

section() {
  printf '\n=== %s ===\n' "$1"
}

section 'Exact commit and pull request state'
printf 'checked_out='; git rev-parse HEAD
printf 'requested='; git rev-parse "$head"
git cat-file -e "${head}^{commit}"
if [ "$(git rev-parse HEAD)" = "$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}]}'

section 'Requested commit and aggregate pull request change'
git show --format=fuller --stat "$head"
git diff --name-status origin/main..."$head"
git diff --stat origin/main..."$head"
git diff --unified=65 "${head}^" "$head" -- \
  src/ports/fleet.ts \
  src/fleet/relay-fleet-client.ts \
  src/fleet/relay-fleet-client.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/teammates.ts \
  src/fleet/teammates.test.ts \
  src/testing/fakes.ts

section 'Relevant TypeScript structure'
for file in \
  src/ports/fleet.ts \
  src/fleet/relay-fleet-client.ts \
  src/fleet/relay-fleet-client.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/teammates.ts \
  src/fleet/teammates.test.ts \
  src/testing/fakes.ts
do
  if [ -f "$file" ]; then
    echo "--- $file"
    ast-grep outline "$file" --items all || true
  fi
done

section 'Fleet contract, construction, observability, connection health, and identities'
rg -n -C 18 --glob '*.ts' \
  '\b(FleetClient|connect|connection|health|whenMessagesObservable|messageStreamIdentity|messageStreamScope|effectiveSender|askTeammate|discoverTeammates|waitForInjected|onAgentMessage|realpathSync|canonical)\b' \
  src test

section 'Focused implementation slices'
nl -ba src/ports/fleet.ts | sed -n '90,310p'
nl -ba src/fleet/relay-fleet-client.ts | sed -n '1,420p'
nl -ba src/fleet/internal-fleet-client.ts | sed -n '1040,1335p'
nl -ba src/fleet/teammates.ts | sed -n '130,390p'

section 'Relevant regression coverage'
rg -n -C 28 --glob '*.test.ts' \
  '\b(whenMessagesObservable|connect.*health|health.*connect|messageStreamIdentity|symlink|realpath|connection.*file|same stream|distinct stream|claim|reconnect)\b' \
  src/fleet test

Length of output: 203258


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='a4cfb87d1a25393f98dbd4a1bc4ce434bedd2fc7'

echo '=== Identity and pull request state ==='
printf 'checked_out='; git rev-parse HEAD
printf 'requested='; git rev-parse "$head"
git cat-file -e "${head}^{commit}"
gh pr view 178 --repo AgentWorkforce/factory \
  --json headRefOid,isDraft,mergeable,mergeStateStatus,statusCheckRollup \
  --jq '{headRefOid,isDraft,mergeable,mergeStateStatus,checks:[.statusCheckRollup[]|{name,status,conclusion}]}'

echo '=== Latest commit ==='
git show --format='%H%n%P%n%s%n%b' --stat "$head"
git diff --unified=35 "${head}^" "$head" -- \
  src/ports/fleet.ts \
  src/fleet/relay-fleet-client.ts \
  src/fleet/relay-fleet-client.test.ts \
  src/fleet/internal-fleet-client.ts \
  src/fleet/internal-fleet-client.test.ts

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

echo '=== RelayFleetClient symbols ==='
ast-grep outline src/fleet/relay-fleet-client.ts --items all || true
rg -n -C 15 \
  'whenMessagesObservable\s*\(|connect(?:ion)?|health|messageStreamIdentity\s*\(|messageStreamScope|effectiveSender\s*\(' \
  src/fleet/relay-fleet-client.ts

echo '=== RelayFleetClient relevant ranges ==='
for range in $(rg -n \
  'whenMessagesObservable\s*\(|messageStreamIdentity\s*\(|effectiveSender\s*\(|connect(?:ion)?' \
  src/fleet/relay-fleet-client.ts | cut -d: -f1 | sort -nu); do
  start=$(( range > 35 ? range - 35 : 1 ))
  end=$(( range + 70 ))
  echo "--- lines ${start}-${end}"
  sed -n "${start},${end}p" src/fleet/relay-fleet-client.ts
done

echo '=== Relevant RelayFleetClient tests ==='
rg -n -C 25 \
  'whenMessagesObservable|message observability|connection.*health|health.*connection|messageStreamIdentity|stream scope' \
  src/fleet/relay-fleet-client.test.ts

echo '=== Connection-file identity implementation ==='
rg -n -C 25 \
  'connectionFileStreamIdentity|realpathSync|messageStreamIdentity\s*\(' \
  src/fleet/internal-fleet-client.ts

echo '=== Symlink identity regression ==='
rg -n -C 35 \
  'symlink|realpath|same.*messageStreamIdentity|messageStreamIdentity.*same' \
  src/fleet/internal-fleet-client.test.ts

echo '=== Direct teammate call sites and claim implementation ==='
rg -n -C 10 \
  '\baskTeammate\s*\(|messageStreamIdentity\s*\(' \
  src/fleet/teammates.ts src/fleet/teammates.test.ts

Length of output: 124150


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='a4cfb87d1a25393f98dbd4a1bc4ce434bedd2fc7'

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

echo '=== Merge commit parents and changed files ==='
git show --format='%H%n%P%n%s' --name-status "$head"
git diff --unified=45 "${head}^1" "$head" -- \
  src/ports/fleet.ts \
  src/fleet/relay-fleet-client.ts \
  src/fleet/relay-fleet-client.test.ts

echo '=== Fleet connection types and public contract ==='
rg -n -A35 -B8 'FleetConnectStatus|fleetConnectStatus|whenMessagesObservable' src/ports/fleet.ts

echo '=== Fleet connection implementation ==='
rg -n -A125 -B25 \
  '`#ensureEventSubscription`\(\)|whenMessagesObservable\(\)|fleetConnectStatus\(\)|`#observeFleetConnectionEvent`\(' \
  src/fleet/relay-fleet-client.ts

echo '=== Fleet connection test names ==='
rg -nP '^\s*it\(' src/fleet/relay-fleet-client.test.ts | rg -i 'connect|connection|observable|disconnect|reconnect|health|event stream'

echo '=== Fleet connection test bodies ==='
for line in $(rg -nP '^\s*it\(.*(?:connect|connection|observable|disconnect|reconnect|health|event stream)' \
  src/fleet/relay-fleet-client.test.ts | cut -d: -f1); do
  start=$((line - 8))
  end=$((line + 95))
  echo "--- lines ${start}-${end}"
  sed -n "${start},${end}p" src/fleet/relay-fleet-client.test.ts
done

Length of output: 145487


@khaliqgant I reviewed exact head a4cfb87d1a25393f98dbd4a1bc4ce434bedd2fc7.

I found no new blocking issue in the main sync and additive conflict resolution.

The merge preserves whenMessagesObservable(). It still waits for event-subscription setup before askTeammate() can send a question.

RelayFleetClient now also preserves the fleet connection health state. It subscribes before it calls messaging.events.connect(). This keeps the fast-reply protection intact.

The new tests cover these connection states:

  • never-attempted
  • dialed before the first event
  • connected
  • disconnect and recovery
  • synchronous connection events
  • subscription failure with redacted error information

The connection-file symlink canonicalization remains present. The regression still verifies that real and symlinked connection.json paths share one message-stream identity.

I cannot approve yet. The package, kubernetes-provider-e2e, load-e2e, verification-gate-e2e, and verification-stack-e2e checks 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: a4cfb87d1a

ℹ️ 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
@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai review

Requested for exact head 7d339326f6a184c0268285cae7acfcee2a4317f8.

@khaliqgant

Copy link
Copy Markdown
Member

@coderabbitai review

Please review exact current head 7d33932 after the socket-handshake observability fix.

@khaliqgant

Copy link
Copy Markdown
Member

@codex review

Please review exact current head 7d33932 after the socket-handshake observability fix.

@coderabbitai

coderabbitai Bot commented Aug 24, 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

head='7d339326f6a184c0268285cae7acfcee2a4317f8'

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 '=== Exact commit and pull request state ==='
printf 'checked_out='; git rev-parse HEAD
printf 'requested='; git rev-parse "$head"
git cat-file -e "${head}^{commit}"
if [ "$(git rev-parse HEAD)" = "$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 PR diff ==='
git show --format=fuller --stat "$head"
git diff --name-status origin/main..."$head"
git diff --stat origin/main..."$head"
git diff --unified=70 "${head}^" "$head" -- \
  src/fleet/relay-fleet-client.ts \
  src/fleet/relay-fleet-client.test.ts \
  src/ports/fleet.ts \
  src/fleet/teammates.ts \
  src/fleet/teammates.test.ts \
  src/fleet/internal-fleet-client.ts \
  src/fleet/internal-fleet-client.test.ts \
  src/testing/fakes.ts \
  test/e2e/ask-a-teammate.test.ts

echo '=== Target file outlines ==='
for file in \
  src/fleet/relay-fleet-client.ts \
  src/fleet/relay-fleet-client.test.ts \
  src/ports/fleet.ts \
  src/fleet/teammates.ts \
  src/fleet/teammates.test.ts \
  src/fleet/internal-fleet-client.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 '=== Handshake and observability implementation/call sites ==='
rg -n -C 22 --glob '*.ts' \
  '\b(socket|handshake|whenMessagesObservable|events\.connect|connect\(|connection|observable|subscribe|onAgentMessage|askTeammate|sendMessage|waitForInjected|effectiveSender)\b' \
  src/fleet src/ports src/testing test

echo '=== Focused implementation and tests ==='
show src/fleet/relay-fleet-client.ts 1 460
show src/ports/fleet.ts 90 300
show src/fleet/teammates.ts 110 420
show src/fleet/relay-fleet-client.test.ts 1 900

echo '=== Relevant test inventory ==='
rg -n -C 30 --glob '*.test.ts' \
  '\b(socket|handshake|observable|observability|connect|connection|early reply|fast reply|event subscription|askTeammate)\b' \
  src/fleet test

echo '=== Public exports and construction paths ==='
rg -n -C 10 --glob '*.ts' \
  '\b(RelayFleetClient|createFleet|whenMessagesObservable|askTeammate)\b' \
  src/index.ts src/fleet src/node src/cli src/ports

Length of output: 203359


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='7d339326f6a184c0268285cae7acfcee2a4317f8'

echo '=== Exact revision and pull request state ==='
printf 'checked_out='; git rev-parse HEAD
printf 'requested='; git rev-parse "$head"
if [ "$(git rev-parse HEAD)" = "$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,isDraft,mergeable,mergeStateStatus,statusCheckRollup \
  --jq '{headRefOid,isDraft,mergeable,mergeStateStatus,checks:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}'

echo '=== Exact commit metadata and patch ==='
git show --format='%H%n%P%n%s%n%b' --stat "$head"
git diff --unified=90 "${head}^" "$head" -- \
  src/fleet/relay-fleet-client.ts \
  src/fleet/relay-fleet-client.test.ts \
  src/ports/fleet.ts \
  src/fleet/teammates.ts \
  src/fleet/teammates.test.ts

echo '=== Socket observability implementation ==='
sed -n '1320,1520p' src/fleet/relay-fleet-client.ts

echo '=== Disposal implementation and call sites ==='
rg -n -C 18 --glob '*.ts' \
  '\b(dispose\s*\(|`#disposed`|settleMessageObservability|rejectMessageObservability|whenMessagesObservable)\b' \
  src/fleet/relay-fleet-client.ts src/fleet/relay-fleet-client.test.ts src/fleet/teammates.ts

echo '=== Direct handshake regression and adjacent lifecycle tests ==='
sed -n '920,990p' src/fleet/relay-fleet-client.test.ts
sed -n '2020,2150p' src/fleet/relay-fleet-client.test.ts

echo '=== askTeammate readiness wait and deadline handling ==='
rg -n -C 35 'whenMessagesObservable|askTeammate|Promise\.race|timeoutMs|deadline' src/fleet/teammates.ts

echo '=== All Relay event variants from local type definitions ==='
rg -n -C 8 --glob '*.ts' \
  '(type|interface).*RelayMessagingEvent|RelayMessagingEvent.*=|type:\s*.(connected|disconnected|reconnecting|error)' \
  src

Length of output: 104637


@khaliqgant I reviewed exact head 7d339326f6a184c0268285cae7acfcee2a4317f8.

P2 — Reject a handshake wait that races dispose()

In src/fleet/relay-fleet-client.ts Line 1379, whenMessagesObservable() creates #messageObservabilityReady after it awaits #eventSubscriptionReady.

If dispose() runs during that await, Line 734 calls #settleMessageObservability() before a promise exists. When #eventSubscriptionReady later resolves, whenMessagesObservable() creates a new observability promise. The client is already disposed, its event listener has been removed, and no handshake event can settle the promise.

askTeammate() then waits for its full request timeout instead of receiving a prompt disposal error.

Check #disposed after await this.#eventSubscriptionReady and reject if disposal has started. Add a regression where a delayed event subscription races dispose(). Assert that whenMessagesObservable() rejects promptly.

The handshake test correctly verifies that a successful events.connect() call alone does not make replies observable.

The request check passed. The package, kubernetes-provider-e2e, load-e2e, verification-gate-e2e, and verification-stack-e2e checks 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: 7d339326f6

ℹ️ 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
@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai review

Requested for exact head f40b4823977164b9c89f92170e525c4931d20e07.

@khaliqgant

Copy link
Copy Markdown
Member

@coderabbitai review

@khaliqgant

Copy link
Copy Markdown
Member

@codex review

@coderabbitai

coderabbitai Bot commented Aug 24, 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: f40b482397

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

// Every still-viable event id predates this resend response. Once the
// resend is accepted, either that earlier send or the resend can deliver
// the same uncorrelated question, so callers must quarantine the pair.
pending.duplicateDeliveryPossible ||= pending.eventIds.size > 0

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 Clear duplicate risk when the original delivery fails

When the readiness resend returns before the original event later emits a correlated delivery_failed, this sets duplicateDeliveryPossible permanently while both event IDs are still viable. Removing the failed original ID never recomputes the flag, so confirmation of the sole surviving resend is reported as a possible duplicate and askTeammate quarantines that requester/teammate pair for the lifetime of the stream, even though the definitive failure proves only one delivery can reply. Recompute or clear the duplicate marker when failures reduce the viable sends to one.

Useful? React with 👍 / 👎.

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