fix(notifications): exclude retained cleanup leak artifacts from endpoint health - #3729
fix(notifications): exclude retained cleanup leak artifacts from endpoint health#3729Yeachan-Heo wants to merge 1 commit into
Conversation
…oint health `classifyNotificationEndpoint` routed every unparseable file in the endpoint directory to `unreadable` unless its name matched a canonical lifecycle artifact. Retained native exact-unlink leak artifacts are zero-byte by construction, so each one was counted as an unreadable endpoint. On a real machine that inflated the scan from 3 endpoints to 13 and reported "1 dead / 10 unreadable of 13 endpoint file(s); run recovery" — a WARN that `notify recovery` could never clear, because the reaper deliberately retains those artifacts (telegram-daemon.ts reapNotificationLeakArtifact) and the native protocol never constructs ExchangePlaceholderRemoval::Removed. Reuse the existing `isNotificationLeakArtifactName` predicate so all four `NOTIFICATION_LEAK_ARTIFACT_PREFIXES` classify as `non-endpoint`, which the health scanner already skips before counting. Genuinely corrupt endpoint files still classify as `unreadable`. This changes classification only. The retention protocol in crates/pi-natives/src/path_identity.rs is deliberate fail-closed behaviour from #3596 and is left untouched. Verified on the real notifications directory: before: [warn] endpoints: 1 dead / 10 unreadable of 13 endpoint file(s) after: [warn] endpoints: 1 dead / 0 unreadable of 3 endpoint file(s) after `notify recovery`: [ok] endpoints: 2 live, 0 unverified — and it now converges across repeated runs instead of oscillating dead<->unreadable. Tested: notifications-service, telegram-daemon-self-heal, and staging-temp-leak suites (75/75); coding-agent tsc --noEmit; biome check. The three added tests fail without the src change.
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Terminal maintainer red-team review at exact head abe22990543f72eb2bb9c88f343e88c79c150408 (base dev c1bf3be50e2cf0dfdf51b57eb43f7a99521c94b8; base is an ancestor of head).
Scope: notification endpoint/leak-artifact classification, security/data exposure, redaction/health classification, and user-facing contract. No source mutation.
Evidence: exact-head GitHub checks 16 success / 6 skipped / 0 failure; local exact-head bun test packages/coding-agent/test/notifications-service.test.ts 57 pass; notification #2956 regression/red-team tests 6 pass. Retained cleanup prefixes are excluded from endpoint candidates while genuinely corrupt non-artifact files remain unreadable; no new secret exposure observed.
Decision: no blocking findings; terminal review is favorable. GitHub APPROVE state cannot be emitted because the authenticated terminal account Yeachan-Heo is the PR author. Signed via terminal GitHub CLI.
|
The reviewed head is now behind current — |
|
The current head has passed exact-head re-review, but it is now conflicting with current — |
REQUEST_CHANGES — notification census exact-head reviewReviewed exact head The contract is not superseded by merged history: current The PR is nevertheless not merge-ready because GitHub reports it conflicting with current No source mutation or merge was performed. Current repair ownership remains with this PR. Signed: Yeachan-Heo via GitHub CLI |
|
GJC adversarial verdict — REQUEST_CHANGES for exact head Signed: GJC / Yeachan-Heo |
|
Signed GJC scope/owner handoff for the DIRTY exact head Current dev is
Owner: Signed: GJC / Yeachan-Heo |
|
REQUEST_CHANGES Signed exact-state owner disposition for PR #3729 on 2026-08-05. Exact head / base / CI
That CI is valid only for the stale exact head/base. It cannot be transferred to a conflict-resolved current- GPT-heavy hostile gateTwo independent
The implementation direction remains relevant and bounded, but this head is not mergeable and misses a retained-readable quarantine class. Contributor ledger / owner path
Required path: update this existing PR only, rebase onto exact current No source mutation, push, merge, CI control, or release action was performed by this review. Signed: GJC owner batch / Yeachan-Heo / 2026-08-05 |
What
notify healthcounted retained native cleanup leak artifacts as endpoint files, pinning it to a WARN thatnotify recoverycould never clear.classifyNotificationEndpointroutes every unparseable file in the endpoint directory tounreadableunless the name matches a canonical lifecycle artifact. Retained exact-unlink leak artifacts are zero-byte by construction, so each one was counted as an unreadable endpoint.Found while dogfooding the current
devbuild for release prep. On a real machine:The directory actually held 3 endpoints plus 10
.gjc-exact-unlink-placeholder-*files. Running the suggested recovery only flipped the wording and never converged:Why it could never clear
The retention is deliberate, so recovery is not at fault:
crates/pi-natives/src/path_identity.rs:2956returnsRetainedFailure(name, "cleanup_pending")even when the placeholder identity matches — there is nounlinkaton that path.ExchangePlaceholderRemoval::Removedis matched in 5 places and constructed in none (#[allow(dead_code, reason = "retained cleanup outcomes are platform-conditional")]).telegram-daemon.ts:1569reapNotificationLeakArtifactexplicitly returns"retained"for this prefix.So the artifacts are meant to persist. The bug is that the health scanner misreads them as endpoints.
Change
Reuse the existing
isNotificationLeakArtifactNamepredicate inunreadableEndpointResultso all fourNOTIFICATION_LEAK_ARTIFACT_PREFIXESclassify asnon-endpoint— a kind the scanner already skips before counting (notification-service.ts:801). Genuinely corrupt endpoint files still classify asunreadable.Classification only. The retention protocol in
path_identity.rsis untouched — it is intentional fail-closed behaviour from #3596 and not something to change in a release-prep window. The 271 placeholders sitting in a real session store are a separate concern and are left for their own issue.Verified on the real notifications directory
[warn] 1 dead / 10 unreadable of 13 endpoint file(s); run recovery[warn] 1 dead / 0 unreadable of 3 endpoint file(s); run recoverynotify recovery[ok] 2 live, 0 unverifiednotify recovery[ok] 2 live, 0 unverified(converges)Total is corrected from a phantom 13 to the real 3. The remaining
1 deadwas a genuinely dead endpoint, which recovery then cleared properly.Tested
bun test packages/coding-agent/test/notifications-service.test.ts packages/coding-agent/test/notifications-telegram-daemon-self-heal.test.ts packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts— 75/75srcchange (verified by stashing it: 2 fail)bun run check:typesinpackages/coding-agent— cleanbun x biome checkon both touched files — cleanChangelog entry added under
### Fixed.