feat(telegram): add topic session folder picker (successor of #3214, #3258) - #3271
Conversation
ca9dcaa to
331c71c
Compare
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head hostile review — APPROVE (MERGE_READY)
Reviewed head (exact): 331c71ccb5f660e42e61304f0961750c9b7d49cc
Base at review: origin/dev = 2d8770169c15cd13efd54aba43ea2cddaab56a93
Hosted Dev CI: 30246599838 (attempt 2, after fork-workflow approval)
Mergeability: MERGEABLE / CLEAN
Author: @gimso2x (cross-repo)
Scope: 11 files / +2964 / −17 — adoption intent store, daemon picker lifecycle, topic-registry user_created + isTopicIdAvailable, generation pin/manifest, lifecycle path admission, tests, CHANGELOG. No workflow mutations. No contributor-branch mutation from this review.
Successor of closed #3214 / #3258. This review only judges exact head 331c71cc; intermediate SHA claims in the PR body are not treated as clearance.
Predecessor / successor blockers
| Source | Blocker | Status at 331c71cc |
Evidence |
|---|---|---|---|
| #3214 B1 | no-spawn terminal_uncertain retained intent |
FIXED (carried) | Pre-send / control-unavailable → spawn_failed + remove intent; readiness_timeout / true terminal_uncertain retain. Covered by adoption suite. |
| #3258 CI-1 | generation guard: protected processTelegramUpdate/run without bump |
FIXED | DAEMON_GENERATION 30→31; digests refreshed. Telegram daemon generation guard = success (job). Local --validate-current-tree exit 0. |
| #3258 CI-2 | Windows readiness_timeout retention red |
FIXED | (a) win32-only isSafePath backslash admission in lifecycle-commands.ts; (b) adoption sidecar open("r")→open("r+") for fsync. Windows Telegram daemon safety = success (job). |
| #3258 CI-3 | Linux pin still toBe(30) after gen bump |
FIXED | Pin/prose updated to generation 31. Focused Linux notifications-telegram-daemon.test.ts = success on exact head. |
No open predecessor PRs remain for this scope (#3214 / #3258 closed, not merged). No other open successor competing on the same adoption feature.
Exact-head CI (required)
| Gate | Result |
|---|---|
Dev CI overall (331c71cc, attempt 2) |
success |
| Telegram daemon generation guard | success |
| Windows Telegram daemon safety | success |
notifications-telegram-daemon.test.ts |
success |
notifications-telegram-adoption-intent.test.ts |
success |
notifications-topic-registry.test.ts |
success |
| Affected path validation / evidence producer | success |
| Public site sync | success |
| Failed/pending required checks | 0 |
Initial attempt 1 was action_required with zero jobs (fork first-run approval). That is not green. After approval, attempt 2 is the terminal green evidence used here. Local corroboration: adoption-intent suite 39 pass / 0 fail.
Contributor gate / scope
- Feature-sized, coherent product slice; no
.github/workflowsedits. - Generation contract observed (bump + manifest + suite pin).
- CHANGELOG under
## [Unreleased]present (duplicate## [Unreleased]section is pre-existing hygiene noise / soft merge with tipdevCHANGELOG; GitHub reports CLEAN merge). - Head is 5 commits ahead / 6 behind tip
dev; non-overlapping product paths with tip drift; MERGEABLE/CLEAN.
Security / contracts (hostile pass)
- Paired-chat / configured-user / non-bot / private-chat gates on
forum_topic_created, picker callbacks, and path entry. - Adoption sidecars persist only
{intendedSessionId, topicId, chatId, target, createdAt, expiresAt}— no bot/control tokens or endpoint digests. - Durable intent written before lifecycle submit; definite no-spawn (
spawn_failed) removes intent (retry via pending topic +/session_create path/ re-entry paths);readiness_timeoutretains for late spawn (Windows CI proves retain). - User topic never deleted; registry adoption goes through
getOrCreateTopic+isTopicIdAvailable+ in-memorytryClaim. - Path admission still rejects shell metacharacters; backslash allowed only on
win32(POSIX unchanged). Full-trust paired chat remains the policy boundary.
No contract softens live-owner / incarnation / endpoint fencing observed in this diff beyond the declared generation-31 adoption surface.
Net assessment
Prior #3258 hard blockers (generation guard, Windows retention, gen pin) are cleared on this exact head with terminal green hosted evidence. Scope, security, and mergeability are acceptable.
Verdict: APPROVE — MERGE_READY on 331c71ccb5f660e42e61304f0961750c9b7d49cc.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Yeachan-Heo
left a comment
There was a problem hiding this comment.
stale exact-head review refresh: re-affirming APPROVED at current head (no diff since last review).
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head re-review — NOT MERGE_READY (DIRTY vs current dev)
Reviewed head (exact): 331c71ccb5f660e42e61304f0961750c9b7d49cc (unchanged)
Prior approval: still valid as code quality at that head vs base 2d877016 with Dev CI 30246599838 success.
Current mergeability: mergeable=false / DIRTY against tip dev.
Author: @gimso2x
Scope: 11 files / +2964 / −17 — topic folder picker / adoption intent (successor of #3214/#3258).
Why prior MERGE_READY is no longer actionable
Tip dev has advanced substantially past the approved base:
| Axis | At prior review (2d877016) |
Current tip (post-#3453) |
|---|---|---|
DAEMON_GENERATION |
31 on this head | 33 on tip |
| Telegram lifecycle | pre-#3304/#3325 | multi-select (#3304) + multiline (#3325) landed |
| Merge state | CLEAN | DIRTY |
Conflict files (changed on both sides):
packages/coding-agent/CHANGELOG.mdpackages/coding-agent/src/sdk/bus/telegram-daemon-contract.tspackages/coding-agent/src/sdk/bus/telegram-daemon.tspackages/coding-agent/test/notifications-telegram-daemon.test.tsscripts/telegram-daemon-generation-manifest.json
Exact-head CI on 331c71cc remains historically green, but that green is against the old base and cannot clear a dirty tip merge.
Hostile product re-check (code still at 331c71c)
Prior adversarial findings remain: adoption intent durability, paired-chat gates, no user-topic deletion, Windows path admission scoped to win32, generation-31 contract observed for that head. No new product-file drift on the PR branch itself (head SHA unchanged).
This re-review does not re-issue MERGE_READY.
Required next step (single mutation owner)
Do not merge 331c71cc as-is.
Unique mutation owner for this Telegram lifecycle lane must:
- Rebase onto current
dev(including post-#3453 tip) - Resolve daemon conflicts with multi-select/multiline + bump
DAEMON_GENERATIONbeyond tip (33 → next) with pin + generation-guard manifest - Re-run exact-head Dev CI + Windows Telegram safety + generation guard
- Fresh signed MERGE_READY only on the new exact head when CLEAN + terminal green
No duplicate mutation owner. This lane owns the sequential Telegram merge queue; #3453 is already merged (1b5e2ad5). #3271 is blocked on dirty rebase, not on product disagreement with the prior feature design.
Verdict: REQUEST_CHANGES — rebase required; NOT MERGE_READY on 331c71ccb5f660e42e61304f0961750c9b7d49cc.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
331c71c to
1d8b344
Compare
Rebase of Yeachan-Heo#3271 onto post-Yeachan-Heo#3453 dev advances DAEMON_GENERATION 33→34 for the protected topic-adoption surface and refreshes the generation-guard manifest digests.
CI on current dev surfaced two failures for the adoption scope: 1. Generation guard (PR mode) requires a strictly higher DAEMON_GENERATION for protected changes to processTelegramUpdate/run. Bump 30 -> 31 and refresh the tracked declaration digest in the generation manifest. 2. Windows readiness_timeout adoption test failed: isSafePath rejected the backslash, which is a shell escape on POSIX but the native path separator on Windows. normalizeLifecyclePath then returned undefined, so the adoption submit treated the folder as unavailable and removed the intent. Allow backslash only on win32; POSIX behavior is unchanged and shell metacharacters / wildcards stay rejected on both platforms.
The focused Linux telegram-daemon suite pinned DAEMON_GENERATION to 30
('keeps wire protocol 3 while generation 30...'). After the 30->31 bump
for the protected adoption lifecycle change, that assertion went red
(Expected: 30 / Received: 31). Update the pin and prose to generation 31
and note generation 31's adoption scope in the contract doc comment.
- open("r") → open("r+") in telegram-adoption-intent.ts:
Windows requires write access on the file handle for fsync;
read-only fd throws EPERM. POSIX unchanged.
- Two daemon tests gain explicit platform: "linux" so the
cooperative-signal code path is exercised deterministically
regardless of the host process.platform:
· reload rollback fence (sendSignal + sleep callback)
· stale dead-owner lock auto-reconciliation (endpoint cleanup)
Verified exact head (8400d99 + this patch):
Windows: notifications-telegram-daemon.test.ts 508 pass / 0 fail
Generation guard: green
Windows CI preflight filter: 24/24 green
Rebase topic-adoption Yeachan-Heo#3271 onto post-Yeachan-Heo#3454 tip. Protected daemon surface requires generation 35 (after WebP photo conversion at 34) with refreshed generation-guard digests.
1d8b344 to
13d4081
Compare
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Exact-head adversarial re-review — APPROVE (MERGE_READY)
Reviewed head (exact): 13d4081974a7adc1a4e4d20f397094ff85c38051
Prior heads: 331c71cc (DIRTY vs tip) → 1d8b344f (rebased post-#3453, gen 34) → this head rebased post-#3454
Base tip at review: 6e4a87400ded072c11998188651fe8db5cbbb7a9 (#3454 WebP photo conversion)
Hosted Dev CI: 30415833065 — success (0 fail / 0 pending)
Mergeability: MERGEABLE / CLEAN
Author: @gimso2x (maintainer_can_modify; unique Telegram mutation owner rebased)
Scope: topic session folder picker / adoption intent (successor of #3214/#3258)
Mutation path (single owner)
- Prior exact head
331c71ccwas product-APPROVED but DIRTY after tip advanced past multi-select (#3304), multiline (#3325), WebP delivery (#3453), and WebP photo conversion (#3454). - Unique Telegram lane rebased onto post-#3454 tip:
- preserved adoption feature + Windows path/fsync fixes
- advanced
DAEMON_GENERATION34 → 35 (tip already held 34 for #3454) - official
telegram-daemon-generation-guard.ts --write-manifestdigests refreshed - PR-mode guard:
required generation bump verifiedvs tip
- Force-with-lease push to contributor branch
feat/telegram-topic-folder-picker-v2.
No product redesign of the adoption picker; only tip rebase + generation/digest closure.
Exact-head CI
| Gate | Result |
|---|---|
Dev CI overall (13d40819) |
success |
| Telegram daemon generation guard | success |
| Windows Telegram daemon safety | success |
notifications-telegram-adoption-intent.test.ts |
success |
notifications-telegram-daemon.test.ts |
success |
notifications-topic-registry.test.ts |
success |
| native-build (PR matrix) | success |
| Public site sync | success |
| Failed/pending required checks | 0 |
Dev tip note (non-blocker for this PR)
Post-merge dev run 30415761408 on 6e4a8740 had an earlier external native-build / bun install node-gyp header ConnectTimeoutError. That is infrastructure, not this PR. Failed-job rerun is in progress on the same SHA; this PR's own exact-head native-build is green. No product mutation for that timeout.
Net assessment
Adoption feature still matches prior hostile review (paired-chat gates, durable intent, no user-topic deletion, win32 path admission, gen fence). Exact head is terminal green and CLEAN after tip rebase.
Verdict: APPROVE — MERGE_READY on 13d4081974a7adc1a4e4d20f397094ff85c38051.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
What
Adds user-created Telegram forum-topic adoption for new GJC sessions (successor of closed #3214 / #3258). The topic presents three folder sources: home (
~), verified recent work folders, and explicit path entry. Adoption intent is durable and the original user-created topic is retained.Rebased onto current
dev, all CI blockers from #3258 resolved with exact-head green evidence on Linux.Why
Starting a session from Telegram should not require manually typing a full lifecycle command when a known workspace or home directory is appropriate.
Successor of #3214 / #3258
This is the third iteration. #3214 was closed after draft limbo (no terminal CI). #3258 was closed after CI surfaced three failures (generation guard, Windows retention, Linux pin). This PR carries the accepted adoption scope and resolves all three.
Blocker-resolution map
terminal_uncertainretained intent (topic stuck until TTL)spawn_failed+ remove intent; post-send (readiness_timeout/trueterminal_uncertain) → retain. Carried from #3214f7cae5d7.processTelegramUpdate/runchanged withoutDAEMON_GENERATIONbumpDAEMON_GENERATION30→31 intelegram-daemon-contract.ts; refreshed tracked declaration digest in generation manifest. Guard verified green on exact head.readiness_timeoutretention red (two root causes)isSafePathrejected backslash, which is the Windows path separator →normalizeLifecyclePathreturnedundefined→ submit treated folder as unavailable → removed intent. Fixed: allow backslash only underprocess.platform === "win32", POSIX unchanged. (b)TelegramAdoptionIntentStoreusedfs.open(tmp, "r")which triggers EPERM on Windows for read-only handles on temporary files. Fixed:"r"→"r+".expect(DAEMON_GENERATION).toBe(30)toBe(31)with prose noting generation 31 adds topic-adoption folder picker. Verified 508/0 on exact-head Linux.Testing
Exact head
ca9dcaa0— Linux (verified locally):bun scripts/telegram-daemon-generation-guard.ts --validate-current-tree→ exit 0bun test test/notifications-telegram-adoption-intent.test.ts→ 39 pass / 0 failbun test test/notifications-telegram-daemon.test.ts(full suite, no filter) → 508 pass / 0 failkeeps wire protocol 3 while generation 31 adds topic-adoption folder picker→ PASS (pin fixed)readiness_timeout retains the durable intent (late spawn within TTL)→ PASS (Linux)control-unavailable create removes the intent so the same topic can retry→ PASSWindows CI — requested on exact head:
readiness_timeout retains the durable intent— expected green with bothisSafePathbackslash allowance +r→r+EPERM fix.Windows Telegram daemon safety— expected terminal green.Scope (files)
Adoption feature (from #3214):
packages/coding-agent/src/sdk/bus/telegram-adoption-intent.ts(new — intent store, +EPERM fix)packages/coding-agent/src/sdk/bus/telegram-daemon.ts(picker lifecycle, adoption frame)packages/coding-agent/src/sdk/bus/topic-registry.ts(user_createdorigin,isTopicIdAvailable)packages/coding-agent/test/notifications-telegram-adoption-intent.test.ts(new)packages/coding-agent/test/notifications-telegram-daemon.test.ts(adoption suite + pin fix + platform determinism)packages/coding-agent/test/notifications-topic-registry.test.tspackages/coding-agent/test/manifests/telegram-baseline-v1.jsonpackages/coding-agent/CHANGELOG.mdCI-green fixes:
packages/coding-agent/src/sdk/bus/telegram-daemon-contract.ts(DAEMON_GENERATION30→31 + doc)packages/coding-agent/src/sdk/bus/lifecycle-commands.ts(isSafePathplatform-aware backslash)scripts/telegram-daemon-generation-manifest.json(refreshed digest)dev--validate-current-treeexit 0,DAEMON_GENERATIONbumped)ca9dcaa0