Ralph triage: silent empty routing tables + unbounded substring matching + unconditional Lead fallback + ungated, non-idempotent re-triage #1591

Summary

packages/squad-sdk/src/ralph/triage.ts (and its vendored twin .squad/templates/ralph-triage.js) mis-assigns issues in a way that is silent, systematic, and hard to undo. In one private repository running Squad it produced six wrong squad:{name} assignments in a row, and the wrong labels could not be cleared by hand because the automation re-applied them.

Four independent defects compound. Each is small; together they turn "no confident match" into "confidently wrong, repeatedly".

All line references are against dev as of today.

1. Both routing-table parsers fail to [] in silence

parseRoutingRules locates its table with

/^##\s*work\s*type\s*(?:→|->)\s*agent\b/i

and parseModuleOwnership with

/^##\s*module\s*ownership\b/i

parseTableSection returns null when no heading matches, and both callers coalesce that to []. Nothing is logged. So a routing.md whose headings read ## Routing Table, ## Routing, ## Work Types, or which simply has no module-ownership section at all — none of which is unreasonable, and a hand-edited or hand-scaffolded routing.md easily lands there — produces a router with zero rules and zero modules that still runs, still returns confident decisions, and reports no problem whatsoever.

In the repository above, routing.md had headed its table ## Routing Table since the day it was created. Both parsers had been returning [] for months. Nobody could have known: there is no output at any level that says so.

Ask: warn when either table parses to zero rows, and when a table names an owner who does not resolve against the roster. A router with no data should be loud, not confident.

2. Keyword and path matching is unbounded String.prototype.includes()

findBestRuleMatch:

issueText.includes(keyword)

findBestModuleMatch:

normalizedIssueText.includes(modulePath)

findRoleKeywordMatch, e.g.:

(role.includes('backend') || role.includes('api') || role.includes('server')) &&
(issueText.includes('api') || issueText.includes('backend') || issueText.includes('database'))

findMember is bidirectional-substring too:

normalizedTarget.includes(memberName) || memberName.includes(normalizedTarget)

There is no token or path boundary anywhere. Consequences we hit in practice:

A domain acronym in a regulatory issue is enough to route it to the backend engineer. Every regulatory issue in that repository contained APIA, so every one of them landed on Backend or Frontend.

Ask: match on token boundaries for keywords, and on path-segment boundaries for module paths (scripts/ must not match docs/scripts/ or myscripts/). A short fix: build one anchored RegExp per keyword with lookarounds on a character class, rather than calling includes.

3. The Lead fallback is unconditional, and a wrong label is sticky

const lead = findLeadFallback(roster);
if (!lead) return null;

return {
  agent: lead,
  reason: 'No module, routing, or role keyword match → routed to Lead/Architect',
  source: 'lead-fallback',
  confidence: 'low',
};

confidence: 'low' is recorded and then ignored — the caller applies the squad:{name} label either way.

This is the defect that makes the others expensive rather than merely annoying. Because the workflow re-triages (see §4), a wrong label is not something a human can remove: it comes back. Clearing one costs a workflow edit, not a click. So the cost of a wrong assignment is far higher than the cost of no assignment — and the fallback exists precisely to convert "no assignment" into "some assignment".

Ask: return null when nothing matches, or at minimum make the fallback opt-in via config. Do not label on confidence: 'low'.

Related, in the same area: findBestModuleMatch / findBestRuleMatch return the single best row, and the caller gives up on that whole layer if the winning row's named owner does not resolve against the roster. A stale row therefore swallows the issue instead of yielding to the runner-up. Ranking and falling through fixes it.

4. The triage path is ungated and non-idempotent

Two problems in the shipped workflow (squad-issue-automation.yml, previously squad-heartbeat.yml):

It re-triages on unrelated events. The workflow fires on issues: [labeled, closed] and pull_request: [closed], and the triage sweep re-decides every open squad issue that carries no squad:{name} label. So the sequence is:

1. A human removes an incorrect squad:{name} label.
2. Any label or close event anywhere in the repository — including one the bot itself caused — starts a new run.
3. The sweep now sees that issue as untriaged and re-applies the same wrong label.

Observed: a label removed at 06:38:10Z was restored by the bot at 06:52:04Z. The human never gets to win.

Its comment write has no idempotency key. The workflow deliberately carries no concurrency: block (correctly — cancelling pending runs drops load-bearing label events), so label bursts start parallel runs. addLabels is naturally idempotent, but createComment is not: four identical auto-triage comments landed on one issue within one second.

Note that a previous fix already made label removal idempotent by absorbing 404 from removeLabel. The write path never got the same treatment.

Ask: gate the sweep so it runs on admission to the queue (the squad label being applied) or on manual dispatch, rather than on every repository event — and scope it to the triggering issue on the label path. Put an idempotency marker in the triage comment and skip a decision whose marker is already in the thread or whose label is already live.

Why this is worth fixing upstream

Each defect alone is survivable. Together they form a failure mode with no signal:

data silently absent → unbounded matcher guesses → guess forced onto the Lead → wrong label re-applied faster than a human can remove it → and no warning at any step.

The routing tables in that repository had been dead since creation and nothing said so. The [] was the root cause; the matcher and the fallback are what turned it into six wrong assignments that resisted manual correction.

What we did downstream (for reference, not a request)

We de-vendored the router into repository-owned code with a test seam, bounded the matching on token and path boundaries, deleted the role-keyword and lead-fallback layers so an unmatched issue gets no label, added a validation pass that warns on empty tables and unreachable members, gated the workflow, and added a marker-based idempotency guard on the comment. Happy to describe any of it in more detail if useful.

We are not opening a pull request: the fixes are shaped by our repository's constraints and some are opinionated (dropping the fallback entirely is a policy choice, not obviously the right default for everyone). §1 and §4 look to us like unambiguous bugs; §2 and §3 are the ones worth a maintainer's judgement.

Reproducing §1 and §2 without our repository

// §1 — silent empty parse
parseRoutingRules('## Routing Table\n\n| Work Type | Agent |\n|---|---|\n| Frontend | Lena |\n');
// => []  (no warning, no throw)

// §2 — unbounded matching
'the apia submission deadline'.includes('api');  // => true
'docs/scripts/build.md'.includes('scripts/');    // => true