Skip to content

Route to the coworker somebody named, and stop building a Bot four ways - #296

Open
jerelvelarde wants to merge 1 commit into
CopilotKit:mainfrom
jerelvelarde:jerel/coworker-resolver
Open

Route to the coworker somebody named, and stop building a Bot four ways#296
jerelvelarde wants to merge 1 commit into
CopilotKit:mainfrom
jerelvelarde:jerel/coworker-resolver

Conversation

@jerelvelarde

Copy link
Copy Markdown
Contributor

Route to the coworker somebody named

The problem

Four things in this deployment now build a Bot for a person: a chat request through
mountCopilotRuntime, a routine's headless turn through buildAgentFor, a hop delivered to another
Bot through agentFor, and whatever comes next. They have to build the same Bot, and today each
one is handed the same eleven collaborators positionally, in the same order, by hand. index.ts
already says so in a comment block in capitals — "they have to build the SAME Bot" — which is a
comment doing the work a type should do. One caller passing undefined where agentFetch goes is a
Bot that runs, answers, and quietly holds different tools or a different role from the one the person
is talking to. Nothing fails; the answer is just worse, and there is nothing to point at.

Choosing which coworker has the same shape of problem for a different reason. The model call, the
visibility rule, the preferred-coworker fallback and the channel.routed row are all written inside
a Hono handler, so nothing that is not an HTTP request can route. A routine cannot. A hop cannot.

And routing is paying for something it already knows. "ask Risk Analyst to review the Q3 filing"
names the coworker in the first four words, and the deployment sends the sentence to a model to be
told what the person already said — a model call and its latency on every such message, and
occasionally a different answer than the one asked for.

The approach

ActorAgentResolver binds the collaborators once. It is constructed at boot from the named
constants that already exist for this reason, and every surface asks it for a coworker instead of
re-assembling the wiring. resolveAgentsForActor gives the whole roster for a request;
resolveAgentForActor gives one, and passes the id down to resolveRuntimeAgents so the other Bots
are neither built nor asked what they hold. The roster is still read in full, so a coworker somebody
cannot see is still absent — that check does not move.

CoworkerRoutingService owns the decision, and the route owns the status codes. The service
returns selected, ambiguous or none; routing/routes.ts turns those into 200, 409 and 404. It
also re-applies canAccessAgent at its own boundary rather than trusting the store's SQL alone,
because a broader store implementation must not be able to leak a coworker into routing.

A name that matches exactly one coworker is honoured without a model call. Matching is against
the asking person's own roster, on NFKC-normalised names with Unicode token boundaries, so Riskier
does not match Risk. Suffix aliases are indexed too, and a longer alias that fully contains a
shorter one suppresses it — otherwise "Risk Analyst" would always read as ambiguous with a coworker
called "Analyst". Two coworkers that genuinely answer to one name is a 409 carrying both display
labels, not a guess, because silently redirecting a message somebody addressed by hand is the worst
answer available.

The cost is real. Name matching is string work on every untagged message, and it can be wrong in
the direction of refusing: a deployment with two coworkers whose names overlap will see 409s where it
used to see a model's pick. That is deliberate — the 409 names both, so the person resolves it in one
reply — but it is a behaviour change, not a pure addition, and it is in CHANGELOG.md as one.

A failed reachability read now stops the turn. Which systems a coworker can reach is weighed by
the router. Treating a failed read as "reaches nothing" is a false statement about the deployment
rather than an absence of one, and it quietly routed work away from the coworker that could do it.
CoworkerReachabilityUnavailableError refuses instead.

What is not covered

  • Routing still runs per request. Nothing is cached, including the alias index, which is rebuilt from
    the roster on every untagged message.
  • The duplicate-name label is the coworker's name plus its id in hex. It is unambiguous and it is not
    pretty. A friendlier disambiguator needs something the roster does not carry yet.
  • resolveAgentForActor throws for a coworker the person cannot reach and the two existing callers
    both turn that into null. The distinction between "not registered" and "not yours" is not
    exposed, because neither caller should tell the difference to a person.
  • No surface changed. The composer's own picker path, the response shape, and the audit payload are
    the same as before apart from the new named by the person asking reason.

Verification

Full suite on this branch, against a live PostgreSQL: 2094 pass, 0 fail, 23 skip, 2117 tests
across 172 files. main in the same environment runs 2084 across 170; the two new files are the
difference. bun run format:check, bun run lint, bun run typecheck and bun run build all clean. No schema change,
so drizzle-kit check and the unwritten-migration probe are untouched.

The recording drives CoworkerRoutingService against a real database, a real AgentProfileStore and
the real audit store. Only the intent model is a stand-in, and only so its calls can be counted — the
whole claim is that a named coworker never reaches it. It ends on the channel.routed rows, which
carry the reason and the candidates and never the message text.

  • server/tests/routing-service.test.ts — the matching rules, one case per way a name can be
    ambiguous or contained; the visibility filter; the reachability refusal; and that the audit row is
    written exactly once and never carries the message.
  • server/tests/agent-resolver.test.ts — that both entry points build through the same collaborators,
    and that asking for one coworker builds only that one.

Four callers now build a Bot for a person — a chat request, a routine's
headless turn, a hop delivered to another Bot, and the boundary's own
lookup — and each passed the same eleven collaborators positionally. One
of them getting an argument wrong is a Bot that runs and quietly holds
different tools or a different role from the one the person is talking
to. ActorAgentResolver binds them once.

Choosing a coworker moves out of the HTTP route for the same reason: it
was the routing model call, the visibility rule, and the channel.routed
row all written inside a Hono handler, so nothing that is not an HTTP
request could route. CoworkerRoutingService owns the decision, and the
route turns its outcome into status codes.

That move makes an explicit name cheap enough to honour: a message that
names exactly one coworker on the asking person's roster no longer pays
a model call to be told what the person already said. Two matches are
refused with both names rather than guessed at.
@davidmckayv

Copy link
Copy Markdown
Contributor

Verified and want this (named-coworker routing + the 4→1 resolver consolidation). Holding for a rebase: both #290 (index.ts/copilot.ts) and #309 (copilot.ts) landed after this branched and touch the same regions your ActorAgentResolver refactor consolidates. Please rebase onto current main so the resolver seam sits on top of #290's handoff wiring and #309's generative-UI middleware. CI verify was green pre-conflict. Thanks.

@Hotragn

Hotragn commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Read this closely because it rewrites routing/routes.ts, which I touched in #248, and I wanted to
check my field survived. It does — undecided is carried through routeDetailed and into the
channel.routed payload, and auditReason builds fallback: <cause> from it. Nothing was lost in
the move, which is the thing I most expected to find in a refactor this size.

The service extraction is the right shape. Four call sites building a Bot positionally from eleven
collaborators, with a capitalised comment doing a type's job, is exactly the kind of thing that goes
wrong quietly, and agent-resolver.ts fixes it in the only way that stays fixed.

One thing I would not merge as it stands.

Every suffix of a coworker's name becomes a standalone mention

buildAliasIndex adds the full normalized name and then every suffix of it, unconditionally:

addAlias(aliases, normalized, profile);
for (const suffix of suffixes(normalized)) addAlias(aliases, suffix, profile);

suffixes("risk analyst") is ["analyst"]. So on the example roster this repo ships:

Coworker Aliases it registers
Risk Analyst risk analyst, analyst
General Assistant general assistant, assistant
Company Knowledge company knowledge, knowledge

Those are ordinary English words, and hasTokenBoundaries will match them as whole tokens anywhere in
a sentence. So:

"I need some knowledge about the refund policy"

matches knowledge, and the result is kind: "selected", viaMention: true, with
reason: "named by the person asking".

Nobody named anybody. The person used a common noun, and the deployment records that they addressed a
coworker by name.

Two things follow, and the second is the one I care about:

The model is skipped, so nothing catches it. The whole value of the short-circuit is that a named
coworker is not a question — but this path is now reached by sentences that are very much a question,
with no confidence score and no fallback, because those only exist on the branch this one avoids.
"what does the agent field mean in this API" on a roster with a Travel Agent is the same shape.

The trail says something that is not true. channel.routed will carry
reason: "named by the person asking" and viaMention: true for a message that named nobody. That
row is the answer to "why did this go to Knowledge", and #134 already moved its wording once
specifically because an administrator reads it and the person who typed did not. A row asserting an
intent the person did not have is worse than one saying "inferred" — and it is now un-countable too,
since viaMention was the field that told inference from choice.

Ambiguity does not save it: two coworkers ending in analyst collide and go to ambiguous, but a
roster with exactly one ... Knowledge has no collision, so the single-coworker case is precisely
the one that silently wins.

What I would do

Any of these, and I have no strong preference between the first two:

  • Require a suffix alias to be more than one token. suffixes("head of risk analysis") still
    gives of risk analysis and risk analysis, which are genuinely names people shorten to. Dropping
    the last single-token suffix removes analyst, assistant, knowledge and keeps everything that
    reads like a name.
  • Only treat a suffix as a mention when it is addressed — preceded by @, or by ask/tell, or
    at the start of the message. The PR's own example, "ask Risk Analyst to review the Q3 filing", is
    addressed; "some knowledge about" is not.
  • Or keep the match and stop calling it a mention. If a bare suffix should still route, it is an
    inference, so viaMention: false and a reason that says which alias matched. That keeps the trail
    honest even if the routing stays as it is.

The full-name match needs none of this — risk analyst appearing as whole tokens really is somebody
naming them, and that half is good.

Smaller notes, none blocking

  • occurrencesOf advances by alias.length after a match, so overlapping occurrences are skipped. It
    cannot matter here, because an overlapping second occurrence has a word character before it and
    fails hasTokenBoundaries anyway — but it is worth a word, since the loop reads as though it means
    to find all of them.
  • hasTokenBoundaries returns a nested ternary that computes "both sides are non-word". It is
    correct; (!before || !WORD.test(before)) && (!after || !WORD.test(after)) says the same thing and
    is one read rather than three.
  • codePointCompare returns -1/1 for the prefix cases and a code-point difference otherwise,
    which is fine for a sort but means the magnitude is meaningless. Worth a line saying it is a
    comparator and not a distance, since the difference is tempting to reuse.

Happy to be wrong about the suffix case if the intent is that a bare knowledge should route there
— but then I would still argue it is not a mention, and the row should not say it was.

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.

3 participants