Repository navigation
Claude/bold clarke kx8upp - #32
Merged
Merged
Conversation
Reliability fixes in e2nb_core: - Seen-item caches evicted arbitrary entries once past their size limit because they were stored as unordered sets, so recently seen RSS items and POP3 messages could be forgotten and notified again. They are now insertion-ordered and evict oldest-first. - RSS item ages were computed by comparing feedparser's UTC timestamps against local time, skewing every age by the machine's UTC offset. On a UTC+10 host a five-minute-old item looked ten hours old and was dropped as stale. Dates are now converted to local time. - Log monitoring tracked file position by re-encoding decoded text, which drifts on undecodable bytes and CRLF endings and then re-reads or skips lines. Reading is now done in binary with exact byte offsets, and a partially written final line is held until it is complete. - Routing rules that matched the same notification overwrote each other, so only the last rule's channels received it. Matching routing rules are now unioned, with suppression applied last. - Monitor events (RSS, web, HTTP, database, log) were dispatched as source_type="email" at default severity, so no source- or severity-specific routing rule ever matched them. Added dispatch_event(), which preserves both. - One channel raising could abort the whole fan-out; each channel is now isolated, as are result callbacks, rule evaluation, and templating. - Template rendering substituted variables one at a time, so text pulled in by an earlier variable could itself be substituted. It is now single-pass. - config.ini is written atomically and restricted to owner-only permissions; a failed write can no longer truncate a working configuration holding credentials. - Missing config sections are filled in on load, so a hand-edited config no longer makes saving fail with a KeyError. - MonitorState tolerates corrupt, non-object, and legacy state files, and a failed state save no longer propagates into the monitor loop. GUI: - Added Database and Log Files pages; both sources were supported by the core and the daemon but unreachable from the GUI, and the GUI monitor loop ignored them entirely. - Each source now runs on its own check interval instead of the global one. - The sidebar scrolls with a visible scrollbar; ~240px of navigation was unreachable at the default window size. Scrollbars are now usable rather than one pixel wide, and appear only when content overflows. - Form help text no longer clips; protocol, TLS and SSL controls are themed instead of raw Tk widgets. - Unsaved changes are shown in the status bar and prompted for on exit. Testing: - New pytest suite (198 tests) covering config handling, validators, email parsing and filters, state persistence and eviction, templates, routing rules, dispatcher fan-out with mocked senders, log monitoring, and a GUI smoke suite that is skipped when no display is available. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UHBtwQGR4gt7w4zrsDvCe
- GitHub Actions workflow running ruff and the full test suite on Python 3.9-3.13, with Tk and Xvfb installed so the GUI tests run rather than skip. - pyproject.toml configuring pytest and ruff; requirements-dev.txt for the test and lint tooling. - Fixed the findings ruff reported and silenced the one intentional late import with an explanation. - .gitignore now covers state files, logs, temp config writes, and editor and virtualenv directories, none of which belonged in the repository. - README: documented database and log file monitoring, which were fully implemented but appeared nowhere in the configuration reference; added a Development section covering tests, linting and CI; described the new GUI pages, per-source intervals and unsaved-change handling; and made the plain-text credential storage explicit under Security. - requirements.txt lists the optional database drivers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UHBtwQGR4gt7w4zrsDvCe
- Credential fields (auth tokens, bot tokens, API keys, passwords) are now masked with a per-field Show/Hide control instead of sitting in plain view; several were not masked at all. - self.config is read by the monitor thread while the UI thread rewrites it on save. Reads and writes now take a lock, and the monitor loop snapshots the whole configuration once per cycle rather than reading it field by field across a cycle that can span minutes. - Email settings in the loop come from EmailConfig, so the GUI honours the same TLS, CA bundle and timeout handling as the daemon. - Removed the dead Toast fade scaffolding, which assigned over a widget method and computed an unused alpha on every step. - Added monitor-loop tests: log matches are delivered, a source-type routing rule reaches the dispatcher, one failing source does not stop the others, and a line already reported is not reported again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UHBtwQGR4gt7w4zrsDvCe
`e2nb-headless.py --test` failed any configuration without email or the SMTP receiver, even though the daemon's own start-up validation accepts RSS, web, HTTP, database, and log sources on their own. A log-only or endpoint-only install therefore looked broken in the exact step the deployment docs tell people to run first. It now reports which sources are configured and only fails when there is genuinely nothing to monitor. Also: - The daemon schedules on the monotonic clock instead of wall time, so an NTP step or a daylight-saving change cannot stall or double-trigger checks. - The setup script's --help listed defaults that no longer matched the values it uses, and carried a STATE_FILE variable assigned twice and never read. - New tests for the daemon's validation, the --test path, and event severity propagation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UHBtwQGR4gt7w4zrsDvCe
The GUI told users to "check email_monitor.log for details" after a failed connection, but never configured logging, so the file did not exist and the underlying reason — the TLS error, the DNS failure, the API's response body — was discarded. It now sets up file logging at startup like the daemon does. The log file also had no rotation, so a monitor left running for months grew one unbounded file. Both entry points now rotate at 5 MB keeping three older files, and a log path that cannot be opened falls back to console output with a warning instead of stopping startup. Also: themed the plain Tk scrollbars inside the JSON editors and the activity log, and corrected the routing-rule help, which still listed only email, rss, web and http as source types and described last-rule-wins precedence that no longer matches the behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UHBtwQGR4gt7w4zrsDvCe
config.ini held credentials once filled in, yet was tracked, so a working install had to either commit its secrets or carry a permanent dirty file. It is now gitignored, and config.ini.sample is the tracked template. The sample is a rewrite rather than a rename: - It documents all 21 sections. The old tracked file had drifted and was missing [Teams], [Pushover] and [Ntfy] entirely, along with several keys. - Every channel ships disabled with empty credentials. The old file enabled Twilio, WhatsApp, Slack, Telegram and Discord with placeholder tokens, so a copied config immediately failed against five fake endpoints. - Each setting carries a comment explaining what it does and, where the value is JSON, an example. A test asserts the sample and DEFAULT_CONFIG_SECTIONS describe the same settings, so they cannot drift apart again the way the old template did. Supporting changes: - The advanced email keys the GUI writes (tls_mode, verify_ssl, ca_bundle, connection_timeout, max_emails_per_check) are part of the schema, so a generated config and the sample match. - E2NB writes config.ini mode 0600, but a file copied from the sample by hand keeps the default 0644. Loading a config other users can read now logs a warning naming the chmod that fixes it. - The installer deploys config.ini.sample as both the starter config and a reference copy next to it. - README documents the copy step, the permissions, and what existing users should expect when pulling a commit that untracks their config.ini. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UHBtwQGR4gt7w4zrsDvCe
![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: f2a5478d7f
ℹ️ 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".
| except OSError: | ||
| return False | ||
|
|
||
| if not mode & 0o077: |
There was a problem hiding this comment.
When the provided installer is used, it creates /etc/e2nb2/config.ini as root:e2nb2 mode 0640 and runs the daemon as e2nb2; this condition treats that intentionally service-readable file as exposed and tells the operator to chmod 600 it. The warning appears on every load, and following it removes the daemon's read permission, so the next restart or reload cannot load its configuration. Recognize the configured service group or avoid owner-only permission advice for this deployment.
Useful? React with 👍 / 👎.
Comment on lines
+1818
to
+1820
| tracked_vars = self._service_vars() + self._source_vars() + [ | ||
| self.rules_var, self.smtp_recv_var, self.dbmon_first_check_var, | ||
| ] |
There was a problem hiding this comment.
Changing only the email protocol, TLS mode, or SSL-verification controls does not set unsaved_changes: their StringVar/BooleanVars are absent from this list, and the recursive key-release binding only catches entry/text edits. In that scenario, closing the GUI skips the save prompt and silently loses the configuration change. Register every configuration-bound variable with the unsaved-change tracker.
Useful? React with 👍 / 👎.
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
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.
No description provided.