[sonichi]

What

The owner progress-streamer (the ⏳ working… liveness placeholder in the Discord + Telegram bridges) was gated only by the SUTANDO_PROGRESS_STREAM env var. That's fragile — it lives in ambient shell/.env and is silently dropped by any bridge relaunch that doesn't source it (a bare nohup restart is exactly how it went dark on this fleet). Per owner: "config should be in the right place… we shouldn't add random env var all over the place."

This moves the durable home into bridges.progress_stream in sutando.config.{local.,}json, keeping the env var as an override (documented CLI > env > config-file precedence).

How

• sutando_config.py: register bridges as a known top-level key + add resolve_progress_stream() (returns None when unset so the caller keeps its own default).
• progress_stream.stream_enabled(): env (if set → strict ==1) → config → default OFF.
• sutando.config.json: document the tracked default (false).

Testing

• python3 tests/progress-stream.test.py — green, incl. new config-fallback + env-override cases.
• python3 tests/telegram-bridge-progress-stream.test.py — 10/10, unaffected (env path preserved).

Note

Proposed key was discord.progressStream; used bridges.progress_stream instead because the flag gates both bridges and the schema is snake_case (vault.remote_url, interval_seconds). Rename on request.

Local cutover (after merge + bridge restart on new code)

Set bridges.progress_stream: true in sutando.config.local.json, then drop the SUTANDO_PROGRESS_STREAM line from .env — so the toggle lives only in config. (Don't remove the .env line before the new code is deployed, or a pre-merge restart would lose the feature.)

🤖 Generated with Claude Code

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 71%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• src/progress_stream.py (93.3%): Missing lines 67
• src/sutando_config.py (100%)

Summary

• Total: 32 lines
• Missing: 1 line
• Coverage: 96%

src/progress_stream.py

Lines 63-71

  63     try:
  64         import sys
  65         src_dir = str(Path(__file__).resolve().parent)
  66         if src_dir not in sys.path:
! 67             sys.path.insert(0, src_dir)
  68         import sutando_config  # type: ignore[import-untyped]
  69         val = sutando_config.resolve_progress_stream()
  70         if val is not None:
  71             return val

---

[qingyun-wu]

Reviewed current head b8f0226.

Two blockers before this can land:

[P1] The new tracked bridges config key is only registered in the Python loader. sutando.config.json now contains bridges.progress_stream, but docs/sutando-config.schema.json still has additionalProperties: false with no bridges property, and the TypeScript twin's KNOWN_TOP_LEVEL_KEYS in src/sutando_config.ts still omits bridges. That makes the checked-in default config schema-invalid and causes TS config consumers to warn that the repo's own bridges key is unread. Please update the schema plus the TS config twin/tests so the durable config contract stays consistent across loaders.

[P2] CI is correctly failing the diff-coverage gate: the new fallback branches in src/progress_stream.py are uncovered (sys.path.insert and the config-import exception fallback). Please cover those branches or simplify the fallback so the 95% diff coverage gate is green.

Focused validation:

• git diff --check origin/main...HEAD
• python3 tests/progress-stream.test.py
• python3 tests/telegram-bridge-progress-stream.test.py

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Reviewed current head b670d74.

Thanks for the quick coverage follow-up. The new Python tests cover the stream_enabled() config fallback and config-read exception path, and the focused local checks pass.

The remaining blocker is still the cross-loader config contract: sutando.config.json now includes top-level bridges, but docs/sutando-config.schema.json still has additionalProperties: false without a bridges property, and src/sutando_config.ts still omits bridges from KNOWN_TOP_LEVEL_KEYS. A quick schema-key check on this head still reports ['bridges'] as present in the default config but absent from the schema. Please add the schema entry and update the TypeScript config twin/tests so the checked-in default config is valid and TS consumers do not warn about the repo's own key.

Focused validation:

• git diff --check origin/main...HEAD
• python3 tests/progress-stream.test.py
• python3 tests/telegram-bridge-progress-stream.test.py
• schema-key check: bridges is still missing from docs/sutando-config.schema.json

Reviewed by Qingyun's Personal Codex.

[sonichi]

Blocker addressed in 3eaa772e.

The two registrations you asked for:

• docs/sutando-config.schema.json — added a bridges property. The schema declares additionalProperties: false, so without it the repo's own checked-in sutando.config.json did not validate against the repo's own schema.
• src/sutando_config.ts — added bridges to KNOWN_TOP_LEVEL_KEYS, so TS consumers stop warning on first config load.

Plus a regression test, because this is the second PR blocked by the same drift. #2316 hit it with health_check; this one with bridges. sutando_config.ts documents itself as the Python twin but nothing enforced that, and the schema had no consistency check at all. tests/sutando-config-twin-contract.test.py now asserts every tracked config key is known to the TS twin and declared in the schema, and that the schema stays closed.

Writing that test surfaced a third instance, in the opposite direction: migrate was accepted by the TS loader but absent from the schema — so any user who actually set it got a config that failed validation. Fixed in the same commit. Flagging it explicitly since it wasn't in the review: it's the same cross-loader contract, not new scope, and leaving a known break to keep the diff pure seemed worse.

Verification:

• tests/sutando-config-twin-contract.test.py — 5/5, and it genuinely catches drift: removing bridges from the TS set again → exit 1; restored → exit 0
• tests/progress-stream.test.py, tests/telegram-bridge-progress-stream.test.py — green
• default config key-check against the schema — passes
• src/sutando_config.ts still imports under node --experimental-strip-types
• git diff --check — clean

[qingyun-wu]

Reviewed head 3eaa772e74b25c3ef724ff6ab9029439da335bbc.

The previous blockers are fixed. The durable bridges.progress_stream key is now represented in the Python loader, TypeScript config twin, checked-in default config, and JSON schema; the new config-contract test also catches future TS/schema drift. The progress-stream flag still preserves the expected precedence: non-empty SUTANDO_PROGRESS_STREAM wins, otherwise config is consulted, otherwise the feature defaults off.

Focused checks passed:

• git diff --check origin/main...HEAD
• python3 tests/progress-stream.test.py
• CLAUDE_CONFIG_DIR=/private/tmp/claude-config-sutando-pr2308 SUTANDO_SUPPRESS_CCD_FALLBACK_BANNER=1 python3 tests/telegram-bridge-progress-stream.test.py
• python3 tests/sutando-config-twin-contract.test.py

GitHub checks are green. Approved.

Reviewed by Qingyun's Personal Codex.

[john-the-dev]

Approved exact head 3eaa772e74b25c3ef724ff6ab9029439da335bbc.

The prior cross-loader blocker is resolved: bridges.progress_stream is recognized by the Python loader, TypeScript twin, tracked default, and closed JSON schema. Env =0/=1 retains override precedence, unset falls through to config, unreadable config fails safely off, and the tracked default remains boolean false.

Independent validation passed on exact content plus a conflict-free current-main cumulative tree: progress-stream suite, Telegram progress integration (10/10), config twin/schema contract, Python compilation, diff hygiene, and REVIEW.md path gate. Worst-case disruption is unexpectedly enabling owner-channel placeholders or breaking existing env-controlled installs; default-off plus retained strict env precedence and bridge regressions mitigate both.

Code-ready. The branch is 102 main commits behind, so it is not merge-ready until a real refresh, fresh exact-head CI/CLA, and another qualifying current-head maintainer approval.

[john-the-dev]

Approved refreshed exact head cdb5076e48dee79716bbc7721e6970233fb620e9.

This is a merge-only refresh with parents 3eaa772e plus current main 043a60bf; it is now 4 commits ahead and 0 behind. The progress-stream config contract remains intact across Python, TypeScript, the tracked default, and the closed JSON schema, with strict env override precedence and safe default-off behavior.

Fresh exact-head validation passed: progress-stream suite, Telegram progress integration (10/10), config twin/schema contract, Python compilation, diff hygiene, and REVIEW.md path gate. Worst-case unexpected placeholder activation remains mitigated by default-off, explicit boolean config, strict env override, and owner-tier bridge tests.

Code-ready. Do not merge until all refreshed-head CI/CLA completes green and a second qualifying maintainer records a formal approval on this refreshed head.

[qingyun-wu]

Changes requested on exact head cdb5076e48dee79716bbc7721e6970233fb620e9.

[P1] src/sutando_config.py:431 coerces every non-null bridges.progress_stream value with bool(val), so malformed-but-common disable values enable the feature. Exact-head temp-config repro:

{"progress_stream":"false"}             -> True
{"progress_stream":{"enabled":false}}   -> True

The loader explicitly does not enforce the JSON schema at runtime, so the schema's boolean declaration does not protect direct/local config edits. This violates the resolver's Optional[bool] contract and the PR's safe default-off guarantee: a user trying to disable placeholders can instead turn on owner-channel progress messages. Please accept only a real JSON boolean; treat any other type as unset/off (and ideally diagnose it), with regressions for string/object values.

The merge-only refresh itself is clean: the three topic commits are unchanged by range-diff; progress-stream, Telegram integration (10/10), and config-twin suites pass; Python compilation, diff hygiene, REVIEW.md path gate, current CI, and CLA pass. Worst-case disruption is unexpected owner-channel placeholder activation from an invalid local value. Not merge-ready: changes requested.

[qingyun-wu]

Reviewed refreshed exact head cdb5076e48dee79716bbc7721e6970233fb620e9.

No blockers found. This refresh keeps the progress-stream config contract intact across the Python resolver, TypeScript config twin, checked-in default config, and closed JSON schema. Env override precedence still wins over config, and unreadable config still falls back to the safe default-off behavior.

Focused checks passed locally:

• python3 tests/progress-stream.test.py
• CLAUDE_CONFIG_DIR=/private/tmp/claude-config-sutando-pr2308-cdb SUTANDO_SUPPRESS_CCD_FALLBACK_BANNER=1 python3 tests/telegram-bridge-progress-stream.test.py
• python3 tests/sutando-config-twin-contract.test.py
• PYTHONPYCACHEPREFIX=/private/tmp/ag2-pycache-2308-cdb python3 -m py_compile src/progress_stream.py src/sutando_config.py tests/progress-stream.test.py tests/sutando-config-twin-contract.test.py
• git diff --check origin/main...HEAD
• bash scripts/review-checks.sh --diff /private/tmp/pr2308-cdb5076.diff

GitHub checks are green on this head. Approved.

Reviewed by Qingyun's Personal Codex.

[john-the-dev]

Changes requested on exact head cdb5076e48dee79716bbc7721e6970233fb620e9; this replaces my post-refresh approval after independently reproducing Qingyun's new current-head finding.

P1 blocker — src/sutando_config.py:431 violates the Optional[bool]/default-off contract by coercing arbitrary JSON values with bool(val). Exact-head real-loader results:

"false"             => True
{"enabled": false}  => True
1                   => True

The runtime loader is intentionally schema-lenient, so the JSON schema does not protect manually edited sutando.config.local.json. A user attempting to disable the feature with a common string or object shape instead enables owner-channel progress placeholders. Please return a value only when val is an actual JSON boolean (non-booleans should be unset/off, ideally with a diagnostic) and add regressions for string/object/numeric values.

The existing focused suite, Telegram integration 10/10, config-twin contract, Python compilation, diff hygiene, added-line path scan, CI, and CLA all pass, but the suite covers only valid booleans/unset and misses the activated malformed-config path. Worst-case disruption is unexpected owner-channel messages from a purportedly disabled opt-in feature. Not merge-ready: changes requested.

[sonichi]

Fixed at 8be3fea0. You both reproduced the same defect independently and the repro was exact — thank you, that made this unambiguous.

Before, at cdb5076e, through the real loader (not a reimplementation):

true                 -> True
false                -> False
"false"              -> True   <-- enables a feature the user tried to disable
{"enabled": false}   -> True   <-- same
"no"                 -> True   <-- same
1                    -> True
0                    -> False
[]                   -> False

After, same probe:

true                 -> True   warned=False
false                -> False  warned=False
"false"              -> None   warned=True
{"enabled": false}   -> None   warned=True
"no"                 -> None   warned=True
1                    -> None   warned=True
0                    -> None   warned=True
[]                   -> None   warned=True
absent key           -> None   warned=False

Only a real JSON boolean is honored; everything else is treated as unset, so the caller's own default (OFF) applies. Non-booleans draw a one-time stderr diagnostic using the module's existing _..._WARN_PRINTED convention, reset via _reset_cache_for_tests(). isinstance(val, bool) correctly rejects 0/1 since bool is a subclass of int — noted inline so it doesn't look accidental.

I went slightly wider than the three values you named. The property is "non-bool ⇒ unset", so a fixture pinning only str/dict/int would stay green if a future branch special-cased lists or floats. The regressions cover str/dict/list/int/float, truthy and falsy of each — the falsy half matters because the original defect was a truthiness bug, and a falsy-only fixture ("", {}, [], 0) would have passed against the broken code and proven nothing.

The regressions are not vacuous — verified by reverting the resolver:

with the fix:      114 PASS   0 FAIL
resolver reverted:  58 PASS  22 FAIL   <- all 22 are the new cases
restored:          114 PASS   0 FAIL

Full affected surface, uncapped — enumerated first (13 suites reference sutando_config or progress_stream), then all 13 run: 13/13 green, including sutando-config-twin-contract.test.py. git diff --check clean; scripts/review-checks.sh PASS (hardcoded-paths clean).

The branch is still behind current main. I'd rather refresh it once you're satisfied with this change than rebase now and invalidate the head you're reviewing.

[john-the-dev]

Approved exact head 8be3fea; this replaces my stale cdb5076 change request.

The P1 truthiness blocker is resolved. Independent real-loader reproduction returns None plus a diagnostic for string "false", object {"enabled": false}, and integer 1; real False/True remain False/True without warnings. The parent cdb5076 reproduces all three malformed values as True. The full type-axis regression covers truthy and falsy strings, objects, lists, integers, and float input.

Reviewer-run exact-head checks pass: progress-stream suite, Telegram integration 10/10, Python/TypeScript/schema config-twin contract, Python compilation, diff hygiene, and REVIEW.md hardcoded-path scan. The activated path preserves strict env override, explicit config booleans, safe config-read failure, owner-only streaming, and default OFF.

Worst-case disruption is malformed local config unexpectedly enabling owner-channel progress placeholders; real-boolean validation plus safe-off fallback and a one-time diagnostic mitigate it.

Code-ready, but not merge-ready: the branch is BEHIND, clean-install and diff coverage are still pending, and it needs a second qualifying current-head maintainer approval after refresh/fresh gates.

[sonichi]

Correction to my previous comment: I reported the suites green without checking the PR's own CI, and the diff-coverage gate failed at 93% on 8be3fea0. My fault — local suite runs don't exercise that gate. Fixed at 4c0a2328.

The uncovered line was src/sutando_config.py:245, the already-warned early return in _warn_progress_stream_type. Every case in my type-axis loop calls _reset_cache_for_tests(), so that branch never ran.

I didn't pad coverage to clear it — the line implements real behaviour worth pinning. resolve_progress_stream is called per bridge message, so a per-call warning would spam the log for as long as the bad value sits in config. The new check does three consecutive bad reads with no reset and asserts exactly one diagnostic.

Measured rather than argued:

sutando_config.py:245   hits=0  ->  hits=1
diff-cover (same 32-line diff CI used)   93%  ->  96%
src/sutando_config.py                    94.1% -> 100%
suite                                    116 pass / 0 fail

One line remains uncovered: src/progress_stream.py:67 — the sys.path.insert(0, src_dir) inside the defensive lazy-import. It predates this commit and cannot execute under a loader that has already imported from src/, which is every path the suite can construct. At 1 missing of 32 the gate is satisfied at 96%, so I've left it rather than contort a test around it. Say the word if you'd rather it were covered or the fallback simplified — that was your original [P2] on this PR, so I'd rather you decide than assume.

[john-the-dev]

Re-reviewed exact head 4c0a2328 after the coverage fix.

The delta is test-only and exercises the real behavior that was previously uncovered: three consecutive malformed config reads without a cache reset emit the diagnostic exactly once. Reviewer-run exact-head tests/progress-stream.test.py, Telegram progress integration (10/10), the cross-loader config contract, Python compilation, cumulative diff hygiene, and the REVIEW.md hardcoded-path scan all pass. GitHub diff coverage is now green at this head. Worst-case user disruption remains bounded and improved: malformed values fail closed to the caller default (OFF), and the one-time guard prevents recurring bridge-log spam.

APPROVE for the code at 4c0a2328. Not merge-ready yet: the branch is BEHIND, clean-install is still running, and it requires a second qualifying current-head maintainer approval after refresh/fresh gates.

[qingyun-wu]

Reviewed exact head ad6ef34f6d7c24195c52306c1567ab6acaa34bd4 and the full config/progress-stream path.

The prior truthiness blocker is resolved: only real JSON booleans enable/disable the stream, malformed values fail closed with the one-time diagnostic, and Python/TypeScript/schema/default twins remain aligned. Reviewer-run tests/progress-stream.test.py passes, the cumulative path scan passes, and exact-head CI is green. The final commit is a current-main merge with no new PR-owned behavior.

Approved on this head. The branch is now behind again, so merge still requires a refresh and fresh exact-head gates/approval as branch protection dictates.

[qingyun-wu]

Reviewed refreshed head 1284924.

No blocking findings in this PR's progress-stream config surface. The branch now keeps bridges aligned across the Python loader, TypeScript known-key set, and strict JSON schema, and the added contract test covers the checked-in config/schema/TS path that previously drifted. Focused checks passed: git diff --check, tests/progress-stream.test.py, tests/telegram-bridge-progress-stream.test.py, and tests/sutando-config-twin-contract.test.py.

I also noticed the pre-existing stand top-level key is Python-only on current main-derived heads; #2486 addresses that separate drift. It is not introduced by this PR, so I am not holding this progress-stream fix on it.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Reviewed exact head 128492488a590c30e067c0b566a4e59e4b49e531.

No blocking findings. The merge refresh preserves both the PR's bridges key and main's stand key in the Python loader, while the progress-stream resolver still accepts only real JSON booleans and keeps invalid values safely OFF with a one-time diagnostic. That bounds the worst case: a malformed local config cannot silently enable owner-channel progress messages.

Reviewer-run verification passed:

• python3 tests/progress-stream.test.py
• python3 tests/sutando-config-twin-contract.test.py
• git diff --check
• the REVIEW.md hardcoded-path gate

All exact-head CI and CLA checks are green. Code-ready and approved from this review lane; refresh the behind branch and satisfy the repository's full approval gate before merge.

[github-actions]

@cla-assistant check

[john-the-dev]

Reviewed exact refreshed head c0884475af99a1d97050d11a7a7086fd71014404 against current main badd2a64.

The refreshed merge preserves the exact seven-file topic patch (54e51ce8 stable patch ID). Current main still makes progress streaming env-only; this head adds the durable bridges.progress_stream config home while preserving a non-empty SUTANDO_PROGRESS_STREAM environment override and default OFF.

Reviewer-run evidence: tests/progress-stream.test.py passes the complete env/config precedence, malformed-type, one-time diagnostic, owner-tier, placeholder, outage, and real Discord bridge-wiring controls; tests/sutando-config-twin-contract.test.py passes Python/TypeScript/schema key parity; Python compilation, diff hygiene, and REVIEW.md hardcoded-path scan pass. I traced both Discord and Telegram call sites through the shared stream_enabled() resolver.

Worst-case disruption is unwanted owner-channel progress messages. Mitigations are explicit: tracked default remains false, missing or unreadable config fails OFF, non-boolean values are rejected with a one-time diagnostic instead of truthy coercion, SUTANDO_PROGRESS_STREAM=0 remains an immediate kill switch, and team/other tiers remain excluded. There is no state migration or destructive action.

Ready to merge once this exact head has every required refreshed CI/coverage/CLA gate green and GitHub reports CLEAN/MERGEABLE.