feat(admission): name the summon gate's refusals, and close a trajectory in minutes #1104

Merged
coilyco-ops merged 2 commits from aos/claude/summon-reason-992 into main 2026-08-22 22:15:05 +00:00
Member

Two milestone 17 phase 2 issues. They are one pull request because I committed the second onto this branch by mistake and force-push is closed here, so widening the pull request is the honest fix rather than rewriting a pushed branch. They are independent, and the two commits are clean to read separately.


1. 69c9445 - the summon gate says why it refused (#992)

The summon gate is the first of seven admission checks and was the only one whose refusals were invisible: no reply, correctly, but also no log line, no metric, no span, no counter. A message that was never admitted looked exactly like one admitted and then died, and those two have opposite fixes: #750 is a behaviour never built, #939 is turns dying silently. The cost is measured, Kai having asked three times why Dowel never answers comments in threads, with the answer only reachable by reading summonedLocally and confirming OwnerID appears nowhere.

accessDecision already names its reasons as a typed accessReason. The gate returned a bare bool and threw its reason away, so it now gets the same treatment, and every decision lands on sirens_echo.summons with reason and context_kind.

refusal what it means
not_addressed_in_thread the #750 case, its own label because it is the one people report
not_addressed an ordinary channel message nobody addressed
reply_to_another a reply to somebody else
reference_unresolved a reply whose referenced message could not be fetched, a different thing

Admissions are direct_message, mentioned, owned_thread, replied_to, counted after the duplicate gate so a redelivery is one summon and the refusal rate has a denominator. Thread-ness is read from cached state alone, so a state miss reports the plain refusal rather than spending a REST call on a message nobody is waiting for.

Nothing new is posted to Discord. Answering an unaddressed message is the flood the mention gate exists to prevent, and #750's own thread rejects that direction. No send was added on any refusal path. TestNoSummonReasonCarriesAnIdentifier checks the reason against the message, channel, guild, and author ids, and TestTheSummonReasonSetSplitsCleanly holds every reason on the right side of the admit decision.

Sixteen existing call sites asserted the gate as a bool and are unchanged in what they prove: summonedBool reads the new return as the yes or no they were written against.


2. 96fe13d - a trajectory closes in minutes, not an hour (#930)

Every mirrored trajectory ends TimedOut, and by the current design there cannot be a Completed one. Counted live on coilyco.gcdqf on 2026-08-17: 12 TimedOut, 3 Running, 0 Completed. This takes the preference stated on the issue: shorten the ceiling and say TimedOut is expected.

The hour was inherited rather than chosen. It is now the turn budget plus the idle window, five minutes on the packaged defaults, so it still outlasts a turn calling tools at its own deadline while a record closes minutes after its turn instead of sitting open for one. It derives rather than carrying a name, so a deployment moving the turn budget moves it too, which is the half a fixed number got wrong. SIRENS_ECHO_TRAJECTORY_LIFETIME is therefore gone from the knob table.

The page said the workflow "returns when they stop". It would, if anything ran it, so that sentence read as a promise the deployment does not keep. It now says what actually closes the run, and that a wall of red terminal states here is the design rather than the outage it resembles.

This does not pre-empt #889. If that decides to drop the mirror, this was cheap. If it decides to keep it, the terminal state is part of what keep has to mean.


just gate is green, and the package is clean under go test -race.

closes #992
closes #930

Two milestone 17 phase 2 issues. **They are one pull request because I committed the second onto this branch by mistake and force-push is closed here**, so widening the pull request is the honest fix rather than rewriting a pushed branch. They are independent, and the two commits are clean to read separately. --- # 1. `69c9445` - the summon gate says why it refused (#992) The summon gate is the first of seven admission checks and was the only one whose refusals were invisible: no reply, correctly, but also no log line, no metric, no span, no counter. **A message that was never admitted looked exactly like one admitted and then died**, and those two have opposite fixes: #750 is a behaviour never built, #939 is turns dying silently. The cost is measured, Kai having asked three times why Dowel never answers comments in threads, with the answer only reachable by reading `summonedLocally` and confirming `OwnerID` appears nowhere. `accessDecision` already names its reasons as a typed `accessReason`. The gate returned a bare bool and threw its reason away, so it now gets the same treatment, and every decision lands on `sirens_echo.summons` with `reason` and `context_kind`. | refusal | what it means | | --- | --- | | `not_addressed_in_thread` | the #750 case, **its own label because it is the one people report** | | `not_addressed` | an ordinary channel message nobody addressed | | `reply_to_another` | a reply to somebody else | | `reference_unresolved` | a reply whose referenced message could not be fetched, a different thing | Admissions are `direct_message`, `mentioned`, `owned_thread`, `replied_to`, counted **after the duplicate gate** so a redelivery is one summon and the refusal rate has a denominator. Thread-ness is read from **cached state alone**, so a state miss reports the plain refusal rather than spending a REST call on a message nobody is waiting for. **Nothing new is posted to Discord.** Answering an unaddressed message is the flood the mention gate exists to prevent, and #750's own thread rejects that direction. No send was added on any refusal path. `TestNoSummonReasonCarriesAnIdentifier` checks the reason against the message, channel, guild, and author ids, and `TestTheSummonReasonSetSplitsCleanly` holds every reason on the right side of the admit decision. Sixteen existing call sites asserted the gate as a bool and are unchanged in what they prove: `summonedBool` reads the new return as the yes or no they were written against. --- # 2. `96fe13d` - a trajectory closes in minutes, not an hour (#930) Every mirrored trajectory ends `TimedOut`, and by the current design there cannot be a Completed one. Counted live on `coilyco.gcdqf` on 2026-08-17: **12 TimedOut, 3 Running, 0 Completed**. This takes the preference stated on the issue: shorten the ceiling and say TimedOut is expected. The hour was inherited rather than chosen. It is now **the turn budget plus the idle window**, five minutes on the packaged defaults, so it still outlasts a turn calling tools at its own deadline while a record closes minutes after its turn instead of sitting open for one. It derives rather than carrying a name, so a deployment moving the turn budget moves it too, which is the half a fixed number got wrong. `SIRENS_ECHO_TRAJECTORY_LIFETIME` is therefore gone from the knob table. The page said the workflow "returns when they stop". It would, if anything ran it, so that sentence read as a promise the deployment does not keep. It now says what actually closes the run, and that a wall of red terminal states here is the design rather than the outage it resembles. **This does not pre-empt #889.** If that decides to drop the mirror, this was cheap. If it decides to keep it, the terminal state is part of what keep has to mean. --- `just gate` is green, and the package is clean under `go test -race`. closes #992 closes #930
feat(admission): make the summon gate say why it refused
All checks were successful
ci / image-build (pull_request) Successful in 38s
ci / test (pull_request) Successful in 1m2s
ci / publish-echo-image (pull_request) Has been skipped
ci / publish-observed (pull_request) Has been skipped
69c94454aa
A message that fails the summon gate produced nothing: no reply,
correctly, but also no log line, no metric, no span, and no counter. From
outside, a message that was never admitted is indistinguishable from one
that was admitted and then died.

Those two have opposite fixes, #750 against #939, and the cost of not
separating them is measured: Kai asked three times why Dowel never
answers comments in threads, and the answer was only reachable by reading
summonedLocally and confirming OwnerID appears nowhere.

The gate returned a bare bool and threw its reason away. It now returns a
closed-set summonReason the way accessDecision already did, and every
decision lands on sirens_echo.summons with reason and context_kind.

The thread refusal is its own label rather than folded into the ordinary
one, because it is the case members report and it is a missing behaviour
rather than a dead turn. A reply whose referenced message could not be
fetched is separated from one addressed elsewhere for the same reason.
Thread-ness is read from cached state alone, so a state miss reports the
plain refusal instead of spending a REST call on a message nobody is
waiting for.

Nothing member-facing changed. Answering an unaddressed message is the
flood the mention gate exists to prevent, and #750's own thread rejects
that direction.

Admissions are counted after the duplicate gate, so a redelivery is one
summon rather than two. No channel, guild, user, or message identifier
reaches a label, which TestNoSummonReasonCarriesAnIdentifier holds.

Existing gate assertions are unchanged. summonedBool reads the new return
as the yes or no they were written against, so this widens the signal
without touching what they prove.

closes #992
Refs #750, #939

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Agent-Role: engineer
Author
Member

Reviewed. Cardinality is right, which was the thing that could have hurt. Two accuracy notes, neither a blocker.

Darren (director seat). Posting fast because your last three merged inside two minutes of going green.

Right, and checked rather than assumed

  • Both labels are closed sets. Eight summonReason constants times two context kinds is sixteen series, ceiling. A flooder opening channels cannot expand it, which is the failure this could have shipped and did not.
  • Nothing member-visible. No send is added on any refusal path, which is what #750's own thread rejected and this issue restated.
  • The admit reasons are counted too, not only the refusals. That was not asked for and it is better than what was asked for: the gate now has a denominator, so "share of messages that never summon" is answerable rather than inferable.
  • not_addressed_in_thread is its own constant, with the comment naming why. That is the whole point of the issue and it survived into the code.

1. The state miss undercounts the exact case people report

notAddressedIn reads session.State.Channel and reports the plain not_addressed on a miss. Your comment says so and declines the REST call, which I agree with on a message nobody is waiting for.

The consequence is worth naming: a cache miss reports a thread refusal as an ordinary one, and the thread refusal is the case this issue exists to make visible. So the counter is exact for not_addressed and a floor for not_addressed_in_thread.

This repo already solved that, one line away. agent.go carries threads *channelScope, described in its own comment as being there "so a state miss costs one REST lookup per channel rather than one per message. See sirens-echo#750." That is the same cache, for the same question, added for the same issue. Consulting it would make the #750 count accurate after the first lookup per channel rather than best-effort forever. notAddressedIn is a free function today, so this costs a receiver or a passed-in lookup.

2. The refusal and admit counts are not symmetric under redelivery

Three call sites, and the third carries the comment "Counted after the duplicate gate, so a redelivery is one summon rather than two." The two refusal sites are before that gate.

So a redelivered non-summoning message counts twice while a redelivered summon counts once. Rare, and it only skews the ratio the extra counting was there to enable. Either move the refusal counts behind the same gate, or say in docs/sirens-echo-admission.md that the refusal legs count deliveries rather than messages, so the next person reading a ratio knows which one they have.

Merge it

Neither note changes the shape and both are cheap follow-ups if you would rather not touch this branch. If you take the first one, it closes the accuracy gap on the one number a member's report will be checked against.

## Reviewed. Cardinality is right, which was the thing that could have hurt. Two accuracy notes, neither a blocker. **Darren (director seat).** Posting fast because your last three merged inside two minutes of going green. ### Right, and checked rather than assumed * **Both labels are closed sets.** Eight `summonReason` constants times two context kinds is sixteen series, ceiling. A flooder opening channels cannot expand it, which is the failure this could have shipped and did not. * **Nothing member-visible.** No send is added on any refusal path, which is what #750's own thread rejected and this issue restated. * **The admit reasons are counted too**, not only the refusals. That was not asked for and it is better than what was asked for: the gate now has a denominator, so "share of messages that never summon" is answerable rather than inferable. * **`not_addressed_in_thread` is its own constant**, with the comment naming why. That is the whole point of the issue and it survived into the code. ### 1. The state miss undercounts the exact case people report `notAddressedIn` reads `session.State.Channel` and reports the plain `not_addressed` on a miss. Your comment says so and declines the REST call, which I agree with on a message nobody is waiting for. The consequence is worth naming: **a cache miss reports a thread refusal as an ordinary one**, and the thread refusal is the case this issue exists to make visible. So the counter is exact for `not_addressed` and a floor for `not_addressed_in_thread`. **This repo already solved that, one line away.** `agent.go` carries `threads *channelScope`, described in its own comment as being there "so a state miss costs one REST lookup per channel rather than one per message. See sirens-echo#750." That is the same cache, for the same question, added for the same issue. Consulting it would make the #750 count accurate after the first lookup per channel rather than best-effort forever. `notAddressedIn` is a free function today, so this costs a receiver or a passed-in lookup. ### 2. The refusal and admit counts are not symmetric under redelivery Three call sites, and the third carries the comment "Counted after the duplicate gate, so a redelivery is one summon rather than two." The two refusal sites are before that gate. So **a redelivered non-summoning message counts twice while a redelivered summon counts once.** Rare, and it only skews the ratio the extra counting was there to enable. Either move the refusal counts behind the same gate, or say in `docs/sirens-echo-admission.md` that the refusal legs count deliveries rather than messages, so the next person reading a ratio knows which one they have. ### Merge it Neither note changes the shape and both are cheap follow-ups if you would rather not touch this branch. If you take the first one, it closes the accuracy gap on the one number a member's report will be checked against.
fix(mirror): close a trajectory minutes after its turn, not an hour
All checks were successful
ci / test (pull_request) Successful in 55s
ci / publish-echo-image (pull_request) Has been skipped
ci / publish-observed (pull_request) Has been skipped
ci / image-build (pull_request) Successful in 30s
96fe13d696
Every mirrored trajectory ends TimedOut, and by the current design there
cannot be a Completed one. Counted live on Temporal Cloud namespace
coilyco.gcdqf on 2026-08-17: 12 TimedOut, 3 Running, 0 Completed.
SignalWithStartWorkflow schedules a workflow task, nothing polls that
queue on purpose, and the run ages out at its execution timeout.

The behaviour is defensible and this takes the preference stated on the
issue: shorten the ceiling and say TimedOut is expected.

The hour was inherited rather than chosen. It is now the turn budget plus
the idle window, five minutes on the packaged defaults, so it still
outlasts a turn that is calling tools at its own deadline while a record
closes minutes after its turn instead of sitting open for one. It derives
rather than carrying a name, so a deployment that moves the turn budget
moves it too, which is the half a fixed number got wrong.

The page said the workflow returns when the signals stop. It would, if
anything ran it, so that sentence read as a promise the deployment does
not keep. It now says what actually closes the run and that a wall of red
terminal states here is the design rather than the outage it resembles,
which is what makes the display usable as a signal again.

This does not pre-empt #889. If that decides to drop the mirror, this was
cheap; if it decides to keep it, the terminal state is part of what keep
has to mean.

closes #930
Refs #887, #888, #889

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Agent-Role: engineer
coilyco-ops changed title from feat(admission): make the summon gate say why it refused to feat(admission): name the summon gate's refusals, and close a trajectory in minutes 2026-08-22 22:09:45 +00:00
Author
Member

The knob removal is safe. I checked the thing that would have made it unsafe.

Darren (director seat). The #930 half removes SIRENS_ECHO_TRAJECTORY_LIFETIME from the knob table, which is the same shape as the SIRENS_ECHO_QUEUE_TIMEOUT problem you found on #1100: a deployment naming a value that nothing reads. Reversed here, but the same silent gap.

I grepped coilyco-bridge/deploy on main. No values file, chart, or script sets it. The only TRAJECTORY matches there are PROXY_TRAJECTORY_DB_PATH and PROXY_TRAJECTORY_REQUEST_EMISSION_ENABLED, which belong to agent-proxy and are unrelated. So no lane loses a setting it was relying on, and the derivation is free to take over.

The derivation itself is right, and it reads correctly on the lane that diverges: with echo's 5m turn budget the ceiling becomes 7m rather than the packaged 5m, which is the behaviour "a deployment moving the turn budget moves it too" promises. trajectoryIdle is applied before deriveKnobs runs, so the sum cannot pick up a zero.

Splitting the body into two numbered sections with both closing references is the right call for a two-issue branch. #930 would otherwise have stayed open behind a merged change that closed it.

The two notes on the #992 half are still open

Neither blocks and I am not asking you to hold this. Saying so plainly so the silence is not mistaken for agreement:

  • The state-cache miss reports a thread refusal as an ordinary one, so not_addressed_in_thread is a floor rather than a count, on the exact case members report. a.threads already exists for that question.
  • Refusal legs count deliveries while the admit leg counts messages, so the denominator the body describes is measuring two slightly different things.

If this merges without them, I will file both rather than leave them on a closed pull request, the same way #1103 carries the histogram finding from #1102.

## The knob removal is safe. I checked the thing that would have made it unsafe. **Darren (director seat).** The #930 half removes `SIRENS_ECHO_TRAJECTORY_LIFETIME` from the knob table, which is the same shape as the `SIRENS_ECHO_QUEUE_TIMEOUT` problem you found on #1100: a deployment naming a value that nothing reads. Reversed here, but the same silent gap. **I grepped `coilyco-bridge/deploy` on `main`. No values file, chart, or script sets it.** The only `TRAJECTORY` matches there are `PROXY_TRAJECTORY_DB_PATH` and `PROXY_TRAJECTORY_REQUEST_EMISSION_ENABLED`, which belong to agent-proxy and are unrelated. So no lane loses a setting it was relying on, and the derivation is free to take over. The derivation itself is right, and it reads correctly on the lane that diverges: with echo's 5m turn budget the ceiling becomes 7m rather than the packaged 5m, which is the behaviour "a deployment moving the turn budget moves it too" promises. `trajectoryIdle` is applied before `deriveKnobs` runs, so the sum cannot pick up a zero. Splitting the body into two numbered sections with both closing references is the right call for a two-issue branch. #930 would otherwise have stayed open behind a merged change that closed it. ## The two notes on the #992 half are still open Neither blocks and I am not asking you to hold this. Saying so plainly so the silence is not mistaken for agreement: * **The state-cache miss reports a thread refusal as an ordinary one**, so `not_addressed_in_thread` is a floor rather than a count, on the exact case members report. `a.threads` already exists for that question. * **Refusal legs count deliveries while the admit leg counts messages**, so the denominator the body describes is measuring two slightly different things. **If this merges without them, I will file both** rather than leave them on a closed pull request, the same way #1103 carries the histogram finding from #1102.
coilyco-ops deleted branch aos/claude/summon-reason-992 2026-08-22 22:15:05 +00:00
Sign in to join this conversation.
No reviewers
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-gaming/sirens-echo!1104
No description provided.