Conversation
|
| Filename | Overview |
|---|---|
| api/routes.py | Threads show_webhook_sessions through session payload building and settings-save cache invalidation. |
| api/route_session_list_cache.py | Adds show_webhook_sessions to the session-list cache key. |
| api/models.py | Adds webhook source handling, Webhooks project assignment, default hiding, and rescue loading. |
| static/panels.js | Adds settings load, autosave, and explicit-save wiring for webhook session visibility. |
Reviews (6): Last reviewed commit: "feat: add Webhooks sidebar filter parity" | Re-trigger Greptile
470a06d to
ac12ee5
Compare
🔬 Gate certification — GREEN (logic) ✅ · BLOCKED on rebase
|
| Gate | Result |
|---|---|
| Codex (reproduce) | SHIP ONLY WITH FIXES — 6/7 findings are stale-base artifacts; 1 real-but-harmless (below) |
| Opus (full review) | APPROVE — "clean, well-tested 1:1 parity with cron; no regressions" |
Full pytest suite (-p no:xdist, clean tree) |
11063 passed, 0 failed |
| PR's own tests + edited cron tests (32) | 32 passed |
| In-process gating verification | server-side display gate confirmed (see below) |
| i18n parity | 2 new keys in all 13 locales ✓ |
⚠️ Merge prerequisite #1 — CONFLICTING on rebase (warm-up's job)
GitHub reports MERGEABLE (state BEHIND) because it merges via merge-commit, but a rebase onto current master conflicts in 2 files:
static/i18n.js(2 hunks): master added French translations for the cron/settings keys since this PR's base; the PR branch still has the older English strings → conflict. A merge-as-is would clobber master's Frenchsettings_label_cron_sessionsetc. with English — a subtle i18n regression.api/routes.py(1 hunk): master refactored the session-list-cache into a__getattr__dynamic-export module; the PR still has the old inline_session_list_cache_key→ conflict.
These are stale-base reconciliation conflicts (the i18n-translation-clobber class), squarely warm-up's rebase domain. The PR should be rebased onto current master before merge so it adds only the 2 new webhook keys (English fallback, per convention) on top of master's translations and reconciles with the cache refactor. I did not resolve them here (rebase/re-warm is the warm-up layer's role, not the gate's).
Codex findings triage
6 of 7 are stale-base artifacts (code from other shipped PRs the stale base appears to "revert"): session-media PDF/HTML auth (#5157), gateway-restart (#5181), update-order, macOS git resolution (#5175), settings-search blob, todo-renderer content??text. None are in this PR's actual webhook delta — they'll vanish on rebase.
1 finding is real but non-blocking (pre-existing pattern):
/api/settingsdirect POST can persistshow_webhook_sessions=truewhileshow_cli_sessions=false(the route savesbodybefore dependency normalization;save_settingsonly bool-coerces). I reproduced it — but it's behaviorally harmless: the session-list builder consumesshow_webhook_sessionsONLY insideif show_cli_sessions:(routes.py:1752), so webhook rows never render when CLI is off regardless of the stored bool (defense in depth, verified). Master's cron toggle has the identical property — this PR faithfully mirrors it rather than introducing a new defect. Optional hardening (not a blocker): normalize bothshow_cron_sessionsandshow_webhook_sessionsto false server-side whenshow_cli_sessionsis false, beforesave_settings(body)— would fix the cosmetically-inconsistent persisted bool for both toggles.
Findings (the feature is sound)
- ✅ Parity, not a new stacked filter (Nathan's concern): it's a Settings toggle "Show webhook sessions" mirroring "Show cron sessions" exactly (gated on CLI sessions, defaults off, same flood rationale) — NOT a new sidebar filter chip.
- ✅ Gating correct at render time: both frontend save paths gate
show_cli && show_webhook, AND the server short-circuits (webhook surfacing only insideif show_cli_sessions:). - ✅ Detection by source metadata, not session_id (avoids the cron id-pattern misclassification bug class) — test-covered.
- ✅ Default-hidden: webhook rows returned-but-hidden unless toggle on; no cron/CLI/messaging regression (32 tests pass).
- ✅ i18n parity (all 13 locales, English-fallback convention per the repo).
Recommendation to the next agent
Logic is GREEN — but do NOT merge as-is; it needs a rebase first. Send it back through warm-up to rebase onto current master (resolve the i18n-translation + session-cache-refactor conflicts so it adds only the 2 webhook keys on top of master), then it re-gates clean and merges. Optional: fold in the server-side toggle-normalization hardening (applies to cron too). It's a Settings-toggle parity feature (not a stacked sidebar filter), Opus-approved, faithful to the #4956 design. I applied gate-pass for the logic but the rebase is a hard merge prerequisite — flagging rather than self-resolving (warm-up's role). Cert valid only at sha:73d6368762cf; the rebase will move the head → re-gate.
Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy/close, and I do not rebase CONFLICTING PRs (that's warm-up). Cert valid only at sha:73d6368762cf.
🔬 Gate re-check — BLOCKED on rebase
|
e5e9777 to
39e3691
Compare
Co-authored-by: Kilian Tyler <kilian@kil.dev>
39e3691 to
0142e21
Compare
🔬 Gate certification — GREEN ✅ (re-cert: structural rebase resolved, cross-module re-integration verified)Certified head: What I ran (isolated worktree
|
| Gate | Result |
|---|---|
| Codex (reproduce, vs current master) | SAFE TO SHIP (no findings — cache-key re-integration correct, #5200 intact, no regression) |
Full pytest suite (-p no:xdist) |
11161 passed, 0 failed |
| PR's webhook/cron-session/cache tests | 42 passed |
node --check static/panels.js |
OK |
| Fresh-based | merge-base == current master (dd4d4b2e), behind 0 — no rebase needed |
Findings (clean — the re-integration I asked for)
- ✅ Cross-module re-integration done right —
show_webhook_sessionsis now threaded through the relocatedapi/route_session_list_cache.py_session_list_cache_key(×2: signature + returned tuple) +api/routes.py(×10 call-sites) + thestatic/panels.jssettingsShowWebhookSessionscheckbox (×3). Codex verified the cache-key position/order is correct (capped vs uncapped responses don't share cache entries). - ✅ perf(sessions): page archived sidebar rows #5200 archived-paging PRESERVED —
archived_limit×6 in the module (the contributor re-merged onto current masterdd4d4b2ewhich includes perf(sessions): page archived sidebar rows #5200, and did NOT drop it — unlike feat(composer): allow footer control reordering #5075's merge which reverted it). This is the correct handling of a hot-shared-file merge against fast-moving master. - ✅ Webhook sessions gated on CLI sessions (server short-circuits otherwise); the 42 webhook/cron-session/cache tests pass; node-check clean; no session-list-cache or cron-toggle regression (Codex confirmed).
- ✅ Fresh-based on current master (BLOCKED status is just the missing gate-pass label, not a conflict).
Concept: 4/5 — webhook-session sidebar filter parity (matches the existing CLI/cron/messaging session filters); a sensible, low-risk sidebar-filter addition.
Recommendation to the next agent
Ready to merge — cert fresh for sha:0142e21701a0. The previously blocked-on-rebase structural collision (master's cache-module relocation) is cleanly resolved: show_webhook_sessions threaded through the relocated module + all call-sites, #5200 archived-paging preserved, Codex SAFE, suite green, 42 targeted tests, fresh-based on current master. Prior GREEN logic + this clean re-integration = ship-ready. Concept 4/5. Cert valid only at sha:0142e21701a0.
Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy/close. Cert valid only at sha:0142e21701a0; a new push invalidates it → re-gate.
🎬 Cutter preview — PR #4957
|
|
This shipped in v0.51.767 (verified: the change is on |

Summary
session_source="webhook"with aWebhooklabelshow_webhook_sessionssettings/UI/i18n plumbing parallel toshow_cron_sessionsCloses #4956
Tests
/Users/maudebot/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_webhook_project_sessions.py tests/test_issue2841_show_cron_sessions_toggle.py tests/test_gateway_sync.py::test_agent_session_source_normalization_contract -q/Users/maudebot/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_webhook_project_sessions.py tests/test_1079_cron_session_project.py tests/test_issue3019_cron_project_sessions.py tests/test_issue4385_cron_archive_reappears.py tests/test_claude_code_session_import.py::test_get_cli_sessions_cache_invalidates_when_sqlite_wal_changes tests/test_issue3585_cron_session_overflow.py::test_cron_sessions_recovered_by_second_pass tests/test_issue4842_cron_projection_perf.py tests/test_issue4842_cli_sessions_streaming_freeze.py tests/test_issue4766_sidebar_source_pushdown.py tests/test_issue4775_sidebar_hidden_zero_message_pushdown.py -q