chore(guard): refresh discord/slack session-router digests after #4542 - #4563
Conversation
PR #4542 changed SessionRouter.#attach and #publishAttachment without refreshing the discord/slack semantic manifest digests, so the current-tree manifest check fails on dev after #4558's merge surfaced it (dev CI 31820749528 Telegram daemon generation guard). The telegram digests all match. Refreshed through the canonical --write-manifest generator; --fix-generations confirms no generation bump is required (digest attestations are exempt from the policy bump). Lore-id: 6e3a1c04 Constraint: digest-only refresh, no generation bump, no source change Confidence: high Scope-risk: narrow Reversibility: trivial Tested: bun test scripts/telegram-daemon-generation-guard.test.ts (75 pass); --validate-current-tree pass at merged dev head Not-tested: none
Guard digest repair for dev — follow-up to #4558 mergeThe merge of #4558 (merge commit Reproduced independently at pre-merge dev Refreshed through the canonical generator ( Verification: Exact head — |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06cbd1a150
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "discord:packages/coding-agent/src/sdk/bus/discord-daemon.ts:stop": "c8bcd881cc0331fb0565a82d997805f939701b12a80e03177192dbd5935824ff", | ||
| "discord:packages/coding-agent/src/sdk/client/discovery.ts:readSdkSessionEndpoint": "c0f42dcf4843e6c79516b11927150e0fe827f6f4cfff400f04ab113ae0574022", | ||
| "discord:packages/coding-agent/src/sdk/router/session-router.ts:SessionRouter.#attach": "f01b84c3ab97d1123b4d7b85760ceb113fbb079f665e188cc7cabcda873e88fe", | ||
| "discord:packages/coding-agent/src/sdk/router/session-router.ts:SessionRouter.#attach": "d9e810776f1bd821e0c62730ae89fe71dc83d6f93b12d9ccebd2cb237dae08cf", |
There was a problem hiding this comment.
When users upgrade with a Discord or Slack daemon already running from before #4542, this digest-only refresh makes the current-tree validation pass while CHAT_DAEMON_GENERATIONS remains 64/67. Because the immediate base already contains the #attach and #publishAttachment changes, evaluate() now sees no protected declaration delta and cannot demand the missed bumps; the controller consequently classifies the old owners as compatible instead of replacing them. Increment both provider generations and regenerate the manifest so the protected endpoint-discovery change is actually fenced.
Useful? React with 👍 / 👎.
Refreshing only the protected declaration digests would let Discord and Slack daemons started before #4542 retain the older attachment contract. Bump both provider generations and regenerate their semantic attestations so stale owners are replaced. Lore-id: 4f2ca8d1 Constraint: preserve the existing #4563 manifest repair and update the same PR branch Rejected: digest-only refresh | does not invalidate pre-#4542 daemon owners Confidence: high Scope-risk: narrow Reversibility: revert-safe Tested: telegram daemon generation guard, daemon-control replacement tests, coding-agent check
Exact-head generation-fence repairCodex P1 review comment Fixed forward on the same PR branch without copying the repair into #4555:
Verification on this exact head:
The earlier statement that no generation bump was required is superseded by this evidence. The PR remains — |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Terminal-green reminder — PR #4563 blocks dev guard and #4555Exact head:
Reviewers requested: @probepark and @HaD0Yun (backup). One authenticated APPROVED review on exact head — |
|
@probepark Independent exact-head review requested for — |
|
@codex review — corrected exact head |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Correction — supersedes comments 5297482928 and 5297509185The guidance in owner comments Codex P1
@probepark @HaD0Yun Independent review is requested only for exact head — |
probepark
left a comment
probepark
left a comment
There was a problem hiding this comment.
Approve - a scoped generation fence, not a blind digest refresh
Reviewed cae16160a against base 9d2a2d2f2. +12/-8 across 3 files.
A PR that rewrites guard digests deserves suspicion by default: the guard exists to make a human
notice when protected symbols change, so refreshing it is exactly how you would silence it. I
checked the scope rather than the intent.
The digest changes are confined to what #4542 actually touched
All six changed entries:
discord:.../chat-daemon-control.ts:CHAT_DAEMON_GENERATIONS.discord
discord:.../session-router.ts:SessionRouter.#attach
discord:.../session-router.ts:SessionRouter.#publishAttachment
slack:.../chat-daemon-control.ts:CHAT_DAEMON_GENERATIONS.slack
slack:.../session-router.ts:SessionRouter.#attach
slack:.../session-router.ts:SessionRouter.#publishAttachment
#attach and #publishAttachment are the symbols #4542 modified when it moved the initial
attachment replay off the reconcile tail. No unrelated protected symbol drifted in alongside them,
which was the thing worth verifying.
The generation bump is the point, and it is justified in place
+ * Discord generation 65 / Slack generation 68 fence the off-reconcile-tail
+ * initial attachment replay introduced by #4542 so pre-upgrade daemon owners
+ * cannot retain the earlier SessionRouter attachment contract.
- discord: 64,
- slack: 67,
+ discord: 65,
+ slack: 68,This is the correct ordering: #4542 changed the attachment contract, so daemon owners running the
previous contract have to be fenced out rather than left attached across the change. The two
CHAT_DAEMON_GENERATIONS digests then change because the constant itself changed - they are a
consequence of the bump, not an extra refresh.
Good that the rationale names the PR and the mechanism in the source comment rather than only in the
commit message. Someone auditing generation 65 a year from now can see why it exists without
archaeology.
Verdict
merge-approved.
Merge-order note: this should land with or after #4542, never before - bumping the fence while the
old attachment contract is still what ships would evict daemon owners for a change that has not
happened yet.
Reviewed by @probepark - method: enumerated every changed manifest key and cross-checked it against the symbols #4542 modified, read the generation-bump rationale in source, confirmed no unrelated protected symbol was refreshed.
2 participants
Summary
SessionRouter.#attachandSessionRouter.#publishAttachmentchanges introduced by fix(sdk): run initial attachment replay off the reconcile tail (#4527) #4542.CHAT_DAEMON_GENERATIONS.discord64→65 and.slack67→68 so daemons started before fix(sdk): run initial attachment replay off the reconcile tail (#4527) #4542 cannot retain the older attachment/replay contract.--write-manifestgenerator, including the corrected generation and declaration digests.Verification
bun test scripts/telegram-daemon-generation-guard.test.ts— 75 passbun scripts/telegram-daemon-generation-guard.ts --validate-current-tree— passbun scripts/telegram-daemon-generation-guard.ts 9d2a2d2f2d8f074333bba92fe1fc4d6902f5b2df cae16160a4fa350c2f84ecdc1ea6cef67a0b1d88— required generation bump verifiedbun test packages/coding-agent/test/daemon-control.test.ts— 170 passbun --cwd=packages/coding-agent run check— passbun --cwd=packages/coding-agent run build— pass31831417583— 27 product jobs pass, four structural skips; only the intentional needs-human bootstrap gate failsSigned exact-head evidence: #4563 (comment)
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Exact head:
cae16160a4fa350c2f84ecdc1ea6cef67a0b1d88gajae.pr-review-verdict.v1 merge-approved sha256:5369ed1ffecc1ce37cb3eb1b105457b0cbf5e19f3f4a70d10a7443192b841cd5 reviewer:human reviewer-id:probepark evidence:6 changed digests scoped exactly to #4542 symbols plus generation constants; bump rationale in source