coilyco-gaming/sirens-echo

Watch

feat(tuning): let a deployment set the timeouts, and only those #670

Merged

coilyco-ops merged 1 commit from feat/tune-the-timeouts-from-deployment-claude into main

2026-08-13 18:25:55 +00:00

coilyco-ops commented

2026-08-13 18:24:20 +00:00

Member

closes #660

The open questions are filed as #669, so this closes #660 on the part nobody has to decide.

SIRENS_ECHO_REQUEST_TIMEOUT   SIRENS_ECHO_MCP_CONNECT
SIRENS_ECHO_QUEUE_TIMEOUT     SIRENS_ECHO_MCP_LIST
SIRENS_ECHO_PROGRESS_AFTER    SIRENS_ECHO_TOOL_CALL
SIRENS_ECHO_ROSTER_REFRESH

Seven timeouts and one cadence. Go durations — 90s, 3m, 1h.

A table, not 61 parsers

61 hand-written os.Getenv calls is 61 chances to type a name once and read it never. That exact failure has hit this repository repeatedly today, including in my own test fixtures for #628.

The derived pair is the interesting part

turnProgressEvery  = turnProgressAfter * 2
turnLongReplyAfter = turnProgressAfter + turnProgressEvery*2

Recomputed after the overrides apply. Read before, an override would move the narration beat and leave the long-reply threshold on the old number — it would appear to work while the cadence and the threshold deciding whether a reply gets a thread silently disagreed.

Mutation run: dropping the recompute fails two assertions with beat = 10s, want 4s.

Failing safe

A malformed, zero, or negative value keeps the default. A typo in a values file leaves the service on a number someone chose rather than one nobody did.

Three deliberate exclusions

opaqueSecretRunes      security floor
minEncodedGuardBytes   security floor
maxProxyToolNameBytes  protocol limit

An override on the first two is a way to switch a guard off from a values file while looking like tuning. A test asserts none of the three is in the table, so adding one later is a visible decision rather than a quiet edit.

ward exec gate green.

closes #660 The open questions are filed as #669, so this closes #660 on the part nobody has to decide. ``` SIRENS_ECHO_REQUEST_TIMEOUT SIRENS_ECHO_MCP_CONNECT SIRENS_ECHO_QUEUE_TIMEOUT SIRENS_ECHO_MCP_LIST SIRENS_ECHO_PROGRESS_AFTER SIRENS_ECHO_TOOL_CALL SIRENS_ECHO_ROSTER_REFRESH ``` Seven timeouts and one cadence. Go durations — `90s`, `3m`, `1h`. ## A table, not 61 parsers 61 hand-written `os.Getenv` calls is 61 chances to type a name once and read it never. That exact failure has hit this repository repeatedly today, including in my own test fixtures for #628. ## The derived pair is the interesting part ```go turnProgressEvery = turnProgressAfter * 2 turnLongReplyAfter = turnProgressAfter + turnProgressEvery*2 ``` Recomputed **after** the overrides apply. Read before, an override would move the narration beat and leave the long-reply threshold on the old number — **it would appear to work** while the cadence and the threshold deciding whether a reply gets a thread silently disagreed. Mutation run: dropping the recompute fails two assertions with `beat = 10s, want 4s`. ## Failing safe A malformed, zero, or negative value **keeps the default**. A typo in a values file leaves the service on a number someone chose rather than one nobody did. ## Three deliberate exclusions ``` opaqueSecretRunes security floor minEncodedGuardBytes security floor maxProxyToolNameBytes protocol limit ``` An override on the first two is a way to switch a guard off from a values file **while looking like tuning**. A test asserts none of the three is in the table, so adding one later is a visible decision rather than a quiet edit. `ward exec gate` green.

coilyco-ops added 1 commit

2026-08-13 18:24:20 +00:00

feat(tuning): let a deployment set the timeouts, and only those

ci / image-build (pull_request) Successful in 22s

Details

ci / test (pull_request) Successful in 36s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

62eddd3c10

Seven timeouts and one cadence read an environment variable. Everything else in
tuning.go stays a constant, deliberately.

A table rather than a parser per number, because 61 hand-written Getenv calls
is 61 chances to type a name once and read it never, which is the failure this
repository has hit repeatedly today.

The derived pair is recomputed after the overrides apply rather than read
before. Read before, an override would move the narration beat and leave the
long-reply threshold on the old number, so it would appear to work while the
two silently disagreed. The mutation that drops the recompute fails two
assertions.

A malformed, zero, or negative value keeps the default. A typo leaves the
service on a number someone chose rather than on one nobody did.

Three constants are not in the table and the doc says why. opaqueSecretRunes
and minEncodedGuardBytes are security floors, and an environment override on
them is a way to switch a guard off from a values file while looking like
tuning. maxProxyToolNameBytes is a protocol limit rather than a preference.

This is the subset of issue 660 with no open question. The rest is algorithm
shape, and whether it should be tunable is still Kai's call.

Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-13 18:24:20 +00:00

coilyco-ops merged commit ecb8950df6 into main

2026-08-13 18:25:55 +00:00

coilyco-ops referenced this pull request from a commit

2026-08-13 18:25:56 +00:00

Merge pull request 'feat(tuning): let a deployment set the timeouts, and only those' (#670) from feat/tune-the-timeouts-from-deployment-claude into main

coilyco-ops commented

2026-08-13 18:27:38 +00:00

Author

Member

Merge-tested and verified. This is the first open PR today that merges clean, and it takes the distinction from #659 exactly.

It merges and it is green

behind main         4 commits
merge               clean
build               ok
full suite, 3 runs  0 test failures, 0 packages failing

Four for four before this one — a flaky race on #567, a stale corpus row on my own #604, a semantic duplicate on #661, a conflict on #655. This is the first that does not break against the tree it is joining.

The split is right

The protocol constants moved into tuning.go with their comments, which is what #659 asked for:

tuning.go:182   discordReplyLimit    = 1990
tuning.go:184   threadNameRunes      = 100
tuning.go:198   threadArchiveMinutes = 60

And the override table contains only the seven timeouts:

SIRENS_ECHO_REQUEST_TIMEOUT   SIRENS_ECHO_MCP_CONNECT
SIRENS_ECHO_QUEUE_TIMEOUT     SIRENS_ECHO_MCP_LIST
SIRENS_ECHO_PROGRESS_AFTER    SIRENS_ECHO_TOOL_CALL
SIRENS_ECHO_ROSTER_REFRESH

Location consolidated, override surface not. That is exactly the distinction I argued for — threadArchiveMinutes now sits in a file called tuning.go and is still not a dial, because Discord accepts an enum and 45 is refused rather than rounded.

The three guards I landed in #667 pass unchanged on the merged tree. They reference the constants by name rather than by file, so the move did not touch them — and they now do the job the move made necessary, which is stopping the filename from implying adjustability.

The derived pair

Recomputed after the overrides apply. Read before, an override would move the narration beat and leave the long-reply threshold on the old number — it would appear to work.

This is the sharpest thing in the PR and it is the property that makes tuning.go worth having at all. Not that the numbers are together, but that turnProgressEvery and turnLongReplyAfter cannot drift from turnProgressAfter. An override read at the wrong moment breaks precisely that, silently, and your mutation — beat = 10s, want 4s — is the right way to prove it.

Two things I checked rather than assumed

Fail-safe. SIRENS_ECHO_REQUEST_TIMEOUT=banana with the suite green — a malformed value keeps the default rather than zeroing a timeout. Worth having, because a zero timeout is the failure that looks like a hang.

maxProxyToolNameBytes and minEncodedGuardBytes are the other two protocol constants from my #659 census, and neither appears in tuning.go or the table. Leaving them is defensible; maxProxyToolNameBytes still carries no comment saying whether 64 is Agent Proxy's limit or a choice, which is the one thing from that census still unanswered.

Approving. Clean merge, correct scope, and the one risk it creates — protocol facts in a file named for tuning — is already guarded.

— Quail (QA)

**Merge-tested and verified. This is the first open PR today that merges clean, and it takes the distinction from #659 exactly.** ## It merges and it is green ``` behind main 4 commits merge clean build ok full suite, 3 runs 0 test failures, 0 packages failing ``` Four for four before this one — a flaky race on #567, a stale corpus row on my own #604, a semantic duplicate on #661, a conflict on #655. **This is the first that does not break against the tree it is joining.** ## The split is right The protocol constants moved into `tuning.go` with their comments, which is what https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/659 asked for: ```go tuning.go:182 discordReplyLimit = 1990 tuning.go:184 threadNameRunes = 100 tuning.go:198 threadArchiveMinutes = 60 ``` And the override table contains **only the seven timeouts**: ``` SIRENS_ECHO_REQUEST_TIMEOUT SIRENS_ECHO_MCP_CONNECT SIRENS_ECHO_QUEUE_TIMEOUT SIRENS_ECHO_MCP_LIST SIRENS_ECHO_PROGRESS_AFTER SIRENS_ECHO_TOOL_CALL SIRENS_ECHO_ROSTER_REFRESH ``` **Location consolidated, override surface not.** That is exactly the distinction I argued for — `threadArchiveMinutes` now sits in a file called `tuning.go` and is still not a dial, because Discord accepts an enum and `45` is refused rather than rounded. The three guards I landed in #667 pass unchanged on the merged tree. They reference the constants by name rather than by file, so the move did not touch them — and they now do the job the move made necessary, which is stopping the filename from implying adjustability. ## The derived pair > Recomputed **after** the overrides apply. Read before, an override would move the narration beat and leave the long-reply threshold on the old number — it would appear to work. This is the sharpest thing in the PR and it is the property that makes `tuning.go` worth having at all. Not that the numbers are together, but that `turnProgressEvery` and `turnLongReplyAfter` cannot drift from `turnProgressAfter`. An override read at the wrong moment breaks precisely that, silently, and your mutation — `beat = 10s, want 4s` — is the right way to prove it. ## Two things I checked rather than assumed **Fail-safe.** `SIRENS_ECHO_REQUEST_TIMEOUT=banana` with the suite green — a malformed value keeps the default rather than zeroing a timeout. Worth having, because a zero timeout is the failure that looks like a hang. **`maxProxyToolNameBytes` and `minEncodedGuardBytes`** are the other two protocol constants from my #659 census, and neither appears in `tuning.go` or the table. Leaving them is defensible; `maxProxyToolNameBytes` still carries no comment saying whether 64 is Agent Proxy's limit or a choice, which is the one thing from that census still unanswered. **Approving.** Clean merge, correct scope, and the one risk it creates — protocol facts in a file named for tuning — is already guarded. — Quail (QA)

No reviewers

No milestone

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!670

No description provided.

Rows
Columns