fix(cron): preserve runtime snapshot for isolated delivery#86604
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed May 25, 2026, 2:49 PM ET / 18:49 UTC. Summary PR surface: Source +16, Tests +50. Total +66 across 2 files. Reproducibility: yes. by source inspection: current main rebuilds cfgWithAgentDefaults from input.cfg, which can make Discord token resolution fall back to an unresolved SecretRef instead of the active runtime snapshot. I did not execute the regression test in this read-only review. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge Security Review detailsBest possible solution: Land the narrow runtime-snapshot cron fix after normal maintainer review and required CI, keeping the regression test with the behavior change. Do we have a high-confidence way to reproduce the issue? Yes, by source inspection: current main rebuilds cfgWithAgentDefaults from input.cfg, which can make Discord token resolution fall back to an unresolved SecretRef instead of the active runtime snapshot. I did not execute the regression test in this read-only review. Is this the best way to solve the issue? Yes, the PR uses the existing runtime snapshot selector before agent-default derivation rather than adding a new config path or Discord-specific workaround. The regression coverage targets the reported isolated Discord delivery failure. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against e844d1d6e56b. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +16, Tests +50. Total +66 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
39befee to
230cf7f
Compare
|
ClawSweeper PR egg ✨ Hatched: 🌱 uncommon Neon Proofling Hatch commandComment Hatchability rules:
Rarity: 🌱 uncommon. What is this egg doing here?
|
230cf7f to
5b5d378
Compare
|
Merge proof for current head 5b5d378: Local proof:
CI proof:
Known proof gap:
|
Summary
Verification
Real behavior proof
Behavior addressed: cron isolated Discord delivery keeps using resolved runtime snapshot credentials after agent defaults are rebuilt.
Real environment tested: local source checkout with focused isolated-cron delivery, Discord token, and runtime snapshot tests.
Exact steps or command run after this patch: node scripts/run-vitest.mjs src/cron/isolated-agent.direct-delivery-core-channels.test.ts extensions/discord/src/token.test.ts src/config/runtime-snapshot.test.ts
Evidence after fix: 3 test files passed; 32 tests passed after rebasing on current main. Earlier broad focused proof also passed 7 files and 90 tests.
Observed result after fix: isolated Discord announce delivery receives the runtime snapshot channels config with the resolved token.
What was not tested: live Discord cron delivery was not rerun in this local PR pass.
Fixes #86545