Repository navigation
refactor(discord): share prepared binding expiry selection - #140643
Merged
Merged
Conversation
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
steipete
marked this pull request as ready for review
September 7, 2026 02:27
Contributor
|
Codex review: needs maintainer review before merge. Reviewed September 6, 2026, 10:57 PM ET / September 7, 2026, 02:57 UTC. ClawSweeper reviewWhat this changesDiscord shares expiration selection between session-binding reporting and automatic cleanup while preserving its timestamp rules, timeout settings, and idle-expiration tie precedence. Merge readiness✅ Ready for maintainer review Keep open: this is a focused, maintainer-authored refactor that remains distinct from current main. No actionable correctness or security defect was found. Priority: P3 Review scores
Verification
How this fits togetherDiscord thread bindings connect conversations to agent sessions. Their timestamps and timeout settings determine the expiration reported to session services and when the background sweeper removes a binding. flowchart TD
A[Discord binding records] --> C[Prepare validated deadlines]
B[Timeout defaults and overrides] --> C
C --> D[Select earliest deadline]
D --> E[Report session binding expiration]
D --> F[Background expiration check]
F --> G[Remove expired binding and send farewell]
Before mergeNone. Agent review detailsSecurityNone. PR surfaceSource -22, Tests +54, Docs +8. Total +40 across 7 files. View PR surface stats
Review metrics
Technical reviewBest possible solution: Keep one shared deadline selector while leaving timestamp validation and timeout defaults with each plugin. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR addresses duplicated implementation rather than a reported runtime failure; source inspection confirms the separate selection paths. Is this the best way to solve the issue? Yes: extracting only candidate selection avoids imposing the standard lifecycle's binding-time clamp on Discord and preserves the existing public APIs. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against acba0c658dc9. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (2 earlier review cycles)
|
Keep Discord timestamp validation and idle-expiry tie precedence while sharing candidate selection with the binding lifecycle owner. Proof: node scripts/run-vitest.mjs src/shared/thread-binding-lifecycle.test.ts extensions/discord/src/monitor/thread-bindings 57 focused tests passed; independent review has no actionable findings.
steipete
force-pushed
the
steipete/dedup-channels-bindings-0906
branch
from
September 7, 2026 02:51
6f3250a to
5e3dbbc
Compare
Contributor
Author
|
Merged via squash.
|
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Sep 7, 2026
…140643) Keep Discord timestamp validation and idle-expiry tie precedence while sharing candidate selection with the binding lifecycle owner. Proof: node scripts/run-vitest.mjs src/shared/thread-binding-lifecycle.test.ts extensions/discord/src/monitor/thread-bindings 57 focused tests passed; independent review has no actionable findings.
This was referenced Sep 7, 2026
This was referenced Oct 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Problem This Solves
Discord computed binding deadlines independently in its session adapter and expiry sweeper, duplicating the shared lifecycle's earliest-deadline selection. Keeping those paths separate makes lifecycle reporting and removal reasons easier to diverge.
Related: #140073, #129801, #138007. All three diffs were inspected before this change. Their configured-binding provenance, native Discord reply routing, and Feishu live-config refresh remain with their respective owners.
Why This Change Was Made
The shared binding lifecycle now selects prepared expiry candidates. Discord prepares its effective durations and guarded timestamps once for both adapter projection and sweeping. Idle expiration continues to win ties. Native timestamp validation and metadata projection remain with their channel owners: Discord's positive finite timestamp rules differ from the standard lifecycle's binding-time clamp.
User Impact
No user-visible behavior change. Binding identity, metadata precedence, persistence, expiration, and asynchronous sweeper fencing remain unchanged. Existing exports remain available.
Evidence
node scripts/run-vitest.mjs src/shared/thread-binding-lifecycle.test.ts extensions/discord/src/monitor/thread-bindings: 57 tests passed across five files, including async sweeper fencing, same-target metadata preservation, legacy timestamps, non-finite overrides, disabled limits, and expiry ties.git range-diff.node scripts/check-changed.mjsplus targeted continuation: all four typecheck lanes, core lint, SDK/doctor/dependency guards, export scans, and remaining architecture guards passed. Local extension lint encountered the documented nested-checkoutDeclaration input escapes checkoutlimitation; no ancestor install or guard was changed.pnpm build: passed, including all 152 public SDK subpaths and 90 external plugins. A native Node probe imported the builtopenclaw/plugin-sdk/thread-bindings-session-runtimehelper and verified idle-expiry tie behavior.5e3dbbc8835869c077c6a59be37f30443fd9e5b1: https://github.com/openclaw/openclaw/actions/runs/34077676325