Repository navigation
fix(sdk): retire a running Slack/Discord chat daemon once its notifications are disabled - #6312
Conversation
probepark
left a comment
There was a problem hiding this comment.
Review (head 61bdaf7, gajae-reviewer on behalf of probepark)
CI: PR 원인 1개 — Affected path validation / plan이 Exact-head CI requires this PR head to contain base 0e761bcc27b39eaeb21e4fcecd48b7cc8e764a6c; rebase onto current dev.로 실패했습니다 (https://github.com/Yeachan-Heo/gajae-code/actions/runs/37184961902/job/111384864871). head의 merge-base는 a808f53이고 dev는 이미 0e761bc입니다(#6298, #6293 머지). plan에서 멈췄기 때문에 coding-agent 샤드, Telegram daemon generation guard, Windows Telegram daemon safety는 모두 skipping입니다. 이 head에서는 실행되지 않았으니 통과로 보지 않습니다.
범위: +229 / -7, 5 files — packages/coding-agent/src/sdk/bus (chat-daemon-cli.ts, chat-daemon-control.ts), test (daemon-control.test.ts), changelog.d fragment, scripts/telegram-daemon-generation-manifest.json
규약: changelog.d fragment 있음 (chat-daemon-retire-when-disabled.md), released section 편집 없음, 생성 파일 없음, 라벨 없음, generation bump (discord 83→84, slack 90→91)와 manifest digest 동시 갱신
주목할 곳:
chat-daemon-cli.tsconfigDisablesProvider:loadConfig가undefined를 돌려주는 경우는 셋입니다.notifications.enabledfalse, providerdesiredEnabledfalse, 그리고loaded.status === "not-found"(config.yml 삭제)입니다. 그래서 config 파일을 지우는 경우도 두 번 연속 읽으면 retire로 이어집니다. 의도한 동작이라면 changelog와 주석에 "파일 없음"도 retire 조건이라고 한 줄 추가하세요. read/validation error(throw)를 계속 serve로 처리하는 경로는catch { return false; }로 맞게 닫혀 있습니다.retireIfDisabled:stopping = true; await stopRuntime();이후 serve loop의while (!stopping)이 빠져나오고finally의releaseChatDaemonOwnership이 stopped로 기록합니다. operator stop과 같은 경로이고stopRuntime은??=로 한 번만 호출되므로, interval tick이 겹쳐도 transport stop이 두 번 일어나지 않습니다. 테스트expect(harness.stops()).toBe(1)이 이 동작을 확인합니다.- (note) 새 테스트의
setIntervalstub이as unknown as typeof setInterval과1 as unknown as ReturnType<...>이중 캐스트를 씁니다.deps.setInterval타입을 좁히거나 typed helper로 빼면 캐스트를 없앨 수 있습니다. 테스트 전용이라 blocking은 아닙니다.
blocking: 1개. stale base라 이 head에서 exact-head CI(plan과 하위 샤드 전체, generation guard 포함)가 평가되지 않았습니다. 3단계 코드 리뷰에서 코드 결함으로 볼 blocking 후보는 없었습니다. git rebase origin/dev(dev의 새 커밋은 TUI/session 파일만 건드려 이 PR 파일과 겹치지 않습니다)로 rebase한 뒤 새 head에서 CI가 green이면 다시 판정합니다. rebase 후 generation guard가 새 base 기준으로도 required generation bump verified를 내는지 확인하세요.
PR body verdict line count=0, not updated. Suggested verdict line: gajae.pr-review-verdict.v1 needs-human sha256:e384b6602792377f891a0a51cd97e9d23cecb14c40ee1a97aaf8f60965faf9ca reviewer:critic reviewer-id:gajae-reviewer evidence:stale-base-plan-failed;ci-unevaluated;code-read-no-defect
Verdict: gajae.pr-review-verdict.v1 needs-human sha256:e384b6602792377f891a0a51cd97e9d23cecb14c40ee1a97aaf8f60965faf9ca reviewer:critic reviewer-id:gajae-reviewer evidence:stale-base-plan-failed;ci-unevaluated;code-read-no-defect
…ations are disabled
The chat daemon owner read its notification config once at startup and never
again, and ensure() returns "disabled" without touching a live owner, so a
daemon kept posting lifecycle messages after the user turned Slack/Discord
off until a manual `gjc daemon stop`. The owner now re-evaluates the config
on its existing 5 s heartbeat tick and, after two consecutive definite
disabled reads, stops its transport and exits through the same orderly path
as an operator stop, releasing ownership as stopped.
Lore-id: 7c3e9a51
Constraint: read/validation errors (half-written, incomplete, quarantined config) must keep the owner serving
Constraint: no new file watchers; piggyback the existing heartbeat interval
Rejected: retire on the first disabled read | a truncated-but-valid YAML rewrite defaults notifications.enabled to false
Rejected: retire on identity change | ensure() already replaces mismatched owners; out of scope
Rejected: change operate("reload") allowDisabledNoop message | owner now self-retires within ~10 s, making it eventually accurate
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: keep config errors non-retiring; only a definite loadConfig() === undefined counts
Tested: daemon-control retirement suite (slack/discord x provider/global disabled, successor reacquire, transient errors); sdk-daemon-cli-e2e; sdk-chat-daemon-control-frames; notifications-compiled-daemon-smoke; coding-agent check
Not-tested: real Slack/Discord transport shutdown on a live install
Co-authored-by: Claude <noreply@anthropic.com>
The chat daemon owner loop changed its lifecycle behavior, which the generation guard protects; bump discord 83->84 and slack 90->91 so an already-running pre-fix owner is replaced instead of attached to. Lore-id: 4b8d2f60 Constraint: generated via telegram-daemon-generation-guard --fix-generations against merge-base a808f53 Confidence: high Scope-risk: narrow Reversibility: clean Tested: generation guard in pull_request mode Co-authored-by: Claude <noreply@anthropic.com>
61bdaf7 to
fe53f17
Compare
…t daemon configDisablesProvider treats every loadConfig() undefined result as a definite disable, which includes a missing config file. Review asked for that condition to be stated alongside the explicit enabled:false cases; it matches the conditions under which a new owner would not start. Lore-id: 2f6a91c4 Confidence: high Scope-risk: narrow Reversibility: clean Tested: telegram-daemon-generation-guard (pull_request mode) and daemon-control suite unchanged Co-authored-by: Codex <noreply@openai.com>
|
@probepark blocking: 옛 base 때문에 exact-head CI가 평가되지 않은 문제 → 해결
설정 파일 삭제도 종료 조건이라는 점을 명시 → 반영 (
테스트
exact head
|
probepark
left a comment
There was a problem hiding this comment.
Review (head d5e3f1e, gajae-reviewer on behalf of probepark)
CI: green — 이전 판정(61bdaf7)의 blocking이던 stale base가 해소됐습니다. 이 head는 base 0e761bc 위에 있고, Affected path validation(plan, check:@gajae-code/coding-agent, test:@gajae-code/coding-agent:shard-1-of-8, test:packages/coding-agent/test/daemon-control.test.ts 포함), Telegram daemon generation guard, Windows Telegram daemon safety, Virtual integration validation, gjc-state-gates가 모두 pass입니다 (https://github.com/Yeachan-Heo/gajae-code/actions/runs/37186545338). 나머지는 경로 무관 skipping입니다.
범위: +231 / -7, 5 files — packages/coding-agent/src/sdk/bus (chat-daemon-cli.ts, chat-daemon-control.ts), test (daemon-control.test.ts), changelog.d fragment, scripts/telegram-daemon-generation-manifest.json
규약: changelog.d fragment 있음, released section 편집 없음, 생성 파일 손편집 없음, 라벨 없음. generation bump(discord 83→84, slack 90→91)와 manifest digest가 함께 갱신됐고 generation guard가 이 head에서 pass입니다.
주목할 곳:
range-diff: rebase로311dd74/fe53f17의 내용은68e675d/61bdaf7과 같습니다(=). 새 커밋은d5e3f1e하나이고,configDisablesProvider주석과 changelog fragment에 "config 파일 없음도 retire 조건"을 추가했습니다. 이전 리뷰의 첫 지적이 반영됐습니다. 코드 변경은 없습니다.chat-daemon-cli.tsretireIfDisabled:renewHeartbeat()가 성공한 tick에서만 호출되고,++disabledReads < 2 || stopping이면 반환합니다. 그다음stopping = true; await stopRuntime()가 operator stop과 같은 경로로 serve loop를 빠져나가finally에서 ownership을 해제합니다.stopRuntime의??=때문에 tick이 겹쳐도 transport stop은 한 번만 일어납니다.configDisablesProvider의catch { return false; }는 read/validation error를 serve로 보내면서 카운트를 0으로 되돌립니다.- (note) interval 콜백이
void (async…)에서return (async…)로 바뀌었습니다. NodesetInterval은 반환값을 무시하므로 동작은 같습니다. 내부try/catch가 모든 경로를 받으므로 unhandled rejection도 생기지 않습니다.
blocking: 없음. approve gate는 ALLOW(pending 0, failed 0, need_local 0, covered check:@gajae-code/coding-agent)입니다.
PR body verdict line count=0, not updated. Suggested verdict line: gajae.pr-review-verdict.v1 merge-approved sha256:2fd4c198f63c5f2fcfddd5442ab495e73127da3782c99740cf1eeaeea2ab64d2 reviewer:human reviewer-id:probepark evidence:ci-green;rebased-onto-dev;range-diff-docs-only;generation-guard-pass;retire-path-read
Verdict: gajae.pr-review-verdict.v1 merge-approved sha256:2fd4c198f63c5f2fcfddd5442ab495e73127da3782c99740cf1eeaeea2ab64d2 reviewer:human reviewer-id:probepark evidence:ci-green;rebased-onto-dev;range-diff-docs-only;generation-guard-pass;retire-path-read
|
Merged into dev as e0b164c.
Thanks @nahyeongjin1. |
What
알림을 꺼도 이미 실행 중인 Slack/Discord 채팅 데몬이 계속 살아서 Slack에 게시하던 문제를 고칩니다.
loadConfig()로 설정을 다시 읽습니다(packages/coding-agent/src/sdk/bus/chat-daemon-cli.ts).notifications.enabled: false또는 provider의enabled: false)가 두 번 연속 나오면 스스로 종료합니다. 종료 경로는 운영자gjc daemon stop과 같습니다: runtime 정지 → serve loop 종료 →finally에서stoppedAt과 함께 소유권 해제.ensure()가 교체하므로 종료 조건에 넣지 않았습니다.CHAT_DAEMON_GENERATIONS를 discord 83→84, slack 90→91로 올리고 semantic manifest를 재생성했습니다(fe53f17a0).Why
실제 설치 환경에서 관찰한 현상입니다.
notifications.slack.enabled: false,notifications.enabled: false로 바꾼 뒤에도 기존 Slack 데몬(gjc daemon slack-internal)이health: running과 갱신되는 heartbeat로 계속 실행됐습니다.GJC session ready./Session closed.20건을 채널에 게시했고, 수동gjc daemon stop slack후에야 멈췄습니다.원인
runChatDaemonInternal은 소유권을 얻기 전에 설정을 한 번만 읽고 다시 읽지 않습니다. 5초 주기는 heartbeat와 소유권만 확인합니다.ChatDaemonController.ensure()는 provider가 꺼져 있으면"disabled"를 바로 반환하고 실행 중인 owner는 건드리지 않습니다.두 번 연속 읽기를 요구하는 이유
notifications.enabled의 기본값이false입니다. 그래서 원자적이지 않은 재작성 중에 잘린 파일이 유효한 YAML로 읽히면 꺼진 것처럼 보일 수 있습니다.Telegram 데몬은 이미 owner watchdog(
telegram-daemon-cli.ts~L533–554)에서 설정을 다시 읽고 꺼짐·불완전·자격 증명 변경 시 종료하므로 이 PR에서 바꾸지 않았습니다.Testing
RED → GREEN —
packages/coding-agent/test/daemon-control.test.ts의runChatDaemonInternal retires when notifications are disabled(5건)stoppedAt이 기록되는지, 상태가stopped인지 검증관련 테스트 묶음과 체크
daemon-control,sdk-daemon-cli-e2e,sdk-chat-daemon-control-frames,notifications-compiled-daemon-smoke→ 286 pass / 0 faildaemon-control.test.ts를 별도로 다시 실행 → 194 pass / 0 failcd packages/coding-agent && bun run check→ exit 0 (경고는 이 PR이 건드리지 않는test/sdk-diagnostics-*의 기존 항목)bunx biome check→ exit 0telegram-daemon-generation-guardPR 모드(base0e761bcc2, rebase 후 재실행) →v52 required generation bump verified잔여 위험
operate("reload")의allowDisabledNoop경로는 owner가 아직 실행 중이어도leaving daemon stopped라고 응답합니다. 이제 owner가 약 10초 안에 스스로 종료하므로 이 PR에서는 그대로 두었습니다.runtime.stop()을 거치며, 실제 연결로는 테스트하지 않았습니다.Approval
Merges to
devrequire one approving GitHub review from a write-access maintainer on the current head. Agent reviews (architect/critic) are advisory comments.Risk classification: medium / narrow — 채팅 데몬 lifecycle의 주기 작업에 종료 조건을 추가하고 generation을 올립니다. Telegram 데몬과
ensure()는 바뀌지 않습니다.devbun checkpasses (cd packages/coding-agent && bun run check)packages/<pkg>/changelog.d/(if user-facing)