fix: Change Docker publishing to trigger only on GitHub Releases by Yeraze · Pull Request #10 · Yeraze/meshmonitor

Yeraze · 2025-09-28T16:32:19Z

Summary

Update the Docker publish workflow to only build and publish images when a GitHub Release is published, rather than on every push to main.

Problem

The current workflow triggers Docker builds on:

Every push to main branch
Every tag push (v*)

This creates intermediate Docker images that aren't tied to official releases, cluttering the container registry.

Solution

Change the workflow trigger to:

release: types: [published] - Only publish images for official GitHub Releases
Keep PR builds for validation (without publishing)

Changes

✅ Workflow Trigger Update

Removed: push to main branch
Removed: push to tags (v*)
Added: release with type published
Kept: pull_request for validation builds

✅ Tag Generation Update

Removed: type=ref,event=branch (no longer needed)
Updated: latest tag now only applies when event_name == 'release'
Kept: All semver patterns ({{version}}, {{major}}.{{minor}}, {{major}})
Kept: PR reference tags for validation builds

Behavior After Merge

What Will Trigger Builds:

✅ Creating a GitHub Release (e.g., v1.2.0)

Builds and publishes with tags: 1.2.0, 1.2, 1, latest

✅ Opening/updating a PR

Builds for validation only (does not publish)

What Will NOT Trigger Builds:

❌ Pushing commits to main
❌ Pushing tags manually with git push --tags

Benefits

Controlled Release Process - Images only for official releases
Cleaner Registry - No intermediate or test images
Clear Versioning - Each image corresponds to a GitHub Release
Maintains PR Validation - Still tests builds on PRs

Test Plan

Workflow syntax validated
Merge to main
Verify no build triggered by merge commit
Create next release (e.g., v1.1.1)
Verify Docker images are published automatically
Confirm images appear at ghcr.io/yeraze/meshmonitor

Notes

This aligns with best practices where Docker images are only published for official releases, not on every commit to main. The workflow will automatically trigger when you create releases through the GitHub UI or gh release create.

🤖 Generated with Claude Code

Update the docker-publish workflow to only build and publish images when a GitHub Release is published, rather than on every push to main. Changes: - Replace push (branches/tags) trigger with release (published) trigger - Remove branch-based tag generation - Update latest tag to only apply on release events - Keep PR builds for validation (without publishing) Benefits: - More controlled release process - Images only published for official releases - Cleaner image registry without intermediate builds - Maintains PR validation builds 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

claude · 2025-09-28T16:32:33Z

Claude finished @Yeraze's task —— View job

PR Review in Progress

Reviewing the Docker workflow changes for release-only publishing.

Todo List

Read and analyze the workflow file changes
Review code quality and best practices
Check for potential bugs or issues
Evaluate performance considerations
Assess security concerns
Review test coverage implications
Provide comprehensive feedback

Analysis Complete - Providing Review

🔍 Code Review: Docker Workflow Changes

✅ Excellent Changes - Well Implemented

The modifications successfully achieve the stated goal of publishing Docker images only on GitHub Releases. Here's my detailed analysis:

Code Quality & Best Practices ⭐

Clean trigger configuration: The change from push + tags to release: types: [published] is exactly right
Proper conditional logic: The latest tag condition ${{ github.event_name == 'release' }} is correctly implemented
Maintained validation: Keeping pull_request builds for validation is a smart practice

Security Assessment 🔒

✅ No security concerns: The workflow maintains proper permission scoping (contents: read, packages: write)
✅ Secure authentication: Uses GITHUB_TOKEN correctly with conditional login
✅ No secrets exposure: No hardcoded credentials or sensitive data

Performance Considerations ⚡

✅ Improved efficiency: Eliminates unnecessary builds on every main branch push
✅ Maintained caching: GitHub Actions cache (type=gha) is preserved
✅ Reduced registry bloat: Prevents intermediate image creation

Specific Changes Analysis

Removed (Good decisions):

# Before
on:
  push:
    branches: [main]
    tags: ['v*']

# Metadata tags removed:
type=ref,event=branch
type=raw,value=latest,enable={{is_default_branch}}

Added (Proper implementation):

# After  
on:
  release:
    types: [published]

# Updated metadata:
type=raw,value=latest,enable=${{ github.event_name == 'release' }}

🎯 Implementation Correctness

Trigger Logic: Perfect - only GitHub Releases will trigger publishing
Tag Generation: Correct - semver patterns preserved, latest only on releases
PR Validation: Maintained - PRs still build for validation without publishing
Registry Auth: Unchanged and secure - only authenticates when not a PR

📋 Test Coverage Implications

No test changes needed: This is purely CI/CD configuration
Validation preserved: PR builds continue to validate Docker builds
Manual testing required: Will need to verify with actual release creation

🚀 Deployment Readiness

This change is ready for production with these expected behaviors:

✅ Merging to main → No Docker build triggered
✅ Creating GitHub Release → Docker images published with proper tags
✅ Opening PR → Validation build runs (no publish)

📝 Minor Suggestions

Documentation: Consider updating repository README to document the new release process
Release Notes: Ensure release creation process is documented for maintainers

⚠️ Testing Recommendation

After merge, create a test release (e.g., v1.1.1) to verify the workflow behaves as expected before the next major release.

Overall Assessment: ✅ APPROVED - This is a well-thought-out change that follows GitHub Actions best practices and achieves the stated goals without introducing any issues.

- Mark messages played from the delivery-success callback, not at enqueue: handleCommand returns { responses, playOnDelivery }; a dropped body DM now leaves the message pending instead of losing it. (Yeraze#1) - Wire purgeExpired into databaseMaintenanceService so expired rows are reclaimed daily (purgeExpired returns a count). (Yeraze#2) - Count the per-recipient cap across the recipient's identity forms via an injected node resolver, so it can't be bypassed by addressing one node by several name forms. (Yeraze#3) - Mailbox bypasses the per-node cooldown (interactive flow). (Yeraze#4) - inbox play <sender> filter matches !hex/node-num forms too. (Yeraze#5) - Non-DM commands return [] (no unsolicited DM). (Yeraze#6) - inbox delete returns the same response for not-yours vs non-existent ids (no id enumeration). (Yeraze#7) - Wrap the mailbox dispatch in try/catch like the script branch. (Yeraze#8) - Remove the command-prefix tolerance: canonical msg/inbox only. (Yeraze#9) - Use shared nodeIdHex (unsigned coerce) for the mailbox log target. (Yeraze#10) Docs + tests updated to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…gaps (#3578) Addresses Claude Code Review findings on PR #3578: - Sanitize device-controlled message before logging/forwarding: strip control chars (log-injection defense) and bound length to 500 (#9, #10). - Widen protected-cap regex to {1,8} hex digits so a short node id still reconciles (#4). - Exclude clientNotification from the unknown-FromRadio debug JSON.stringify dump (#3). - Drop the redundant `&& this.sourceId` guard (always set) and comment why the toast still fires when the DB revert fails (#2, #5). - Frontend: name the level magic numbers and note they mirror the backend NOTIFICATION_LEVEL (#1). - Tests: sanitizer (control chars/whitespace/truncation/empty), short-hex-id parse, and the DB-revert-failure path (#6). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011JEaCGwY9Wz8jeV4e22GW4

…8 favorite/ignore cap (#3548) (#3578) * feat(notifications): surface device ClientNotifications + handle firmware 2.8 favorite/ignore cap (#3548) MeshMonitor decoded FromRadio.ClientNotification (mesh.proto field 16) and dropped it. Surface these device-originated warnings/errors as toasts, and handle the firmware 2.8 protected-node-cap refusal for Set Favorite / Ignore. Backend: - clientNotificationPolicy.ts (pure/testable): suppression patterns (recurring power-save "sleeping for N interval" INFO + key-verification variants + empty messages), the 2.8 protected-node-cap refusal parser, level->severity mapping, and a per-source ToastThrottle that dedupes identical messages within a window. - meshtasticProtobufService.ts: add the clientNotification dispatch branch (was falling through to the generic catch-all). - dataEventEmitter.ts: client-notification event type + emitClientNotification(). - meshtasticManager.ts: handleClientNotification() reverts the optimistic favorite/ignore flag and re-broadcasts the node when the device refuses at the protected-node cap, then applies the suppression/throttle policy and emits the toast event. Source-scoped throughout. Frontend: - DeviceNotificationToaster.tsx: listens for client-notification inside ToastProvider, maps level->severity, shows the toast. Wired into App.tsx. - WS forwarding + per-source room filtering needed no changes. Scope: the 2.8 NodeDB warm-tier restructure and the on-disk snr_q4 field do NOT affect the over-the-air wire MeshMonitor reads. OTA NodeInfo.snr stays a float in dB; no protobuf/decode change. A regression test guards this. The cap-refusal warning is only emitted by firmware for the locally-connected node (from == 0). Tests (20 new, full suite green): policy unit tests, manager handler tests (reconciliation/suppression/throttle, per-source), and protobuf dispatch + SNR float guard. Docs: FAQ (node count + blocked-node 2.8 behavior; device-notification toasts), IgnoredNodesSection inline help, CHANGELOG, and the support plan dev-note. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011JEaCGwY9Wz8jeV4e22GW4 * address review: sanitize device notification text, robustness + test gaps (#3578) Addresses Claude Code Review findings on PR #3578: - Sanitize device-controlled message before logging/forwarding: strip control chars (log-injection defense) and bound length to 500 (#9, #10). - Widen protected-cap regex to {1,8} hex digits so a short node id still reconciles (#4). - Exclude clientNotification from the unknown-FromRadio debug JSON.stringify dump (#3). - Drop the redundant `&& this.sourceId` guard (always set) and comment why the toast still fires when the DB revert fails (#2, #5). - Frontend: name the level magic numbers and note they mirror the backend NOTIFICATION_LEVEL (#1). - Tests: sanitizer (control chars/whitespace/truncation/empty), short-hex-id parse, and the DB-revert-failure path (#6). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011JEaCGwY9Wz8jeV4e22GW4 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

- Mark messages played from the delivery-success callback, not at enqueue: handleCommand returns { responses, playOnDelivery }; a dropped body DM now leaves the message pending instead of losing it. (Yeraze#1) - Wire purgeExpired into databaseMaintenanceService so expired rows are reclaimed daily (purgeExpired returns a count). (Yeraze#2) - Count the per-recipient cap across the recipient's identity forms via an injected node resolver, so it can't be bypassed by addressing one node by several name forms. (Yeraze#3) - Mailbox bypasses the per-node cooldown (interactive flow). (Yeraze#4) - inbox play <sender> filter matches !hex/node-num forms too. (Yeraze#5) - Non-DM commands return [] (no unsolicited DM). (Yeraze#6) - inbox delete returns the same response for not-yours vs non-existent ids (no id enumeration). (Yeraze#7) - Wrap the mailbox dispatch in try/catch like the script branch. (Yeraze#8) - Remove the command-prefix tolerance: canonical msg/inbox only. (Yeraze#9) - Use shared nodeIdHex (unsigned coerce) for the mailbox log target. (Yeraze#10) Docs + tests updated to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…ore) (#3538) * feat: add Dead Drop / Mailbox auto-responder (async message store) A per-source 'mesh voicemail': a node DMs the radio `msg <name> <text>` and the message is held until the named recipient retrieves it via `inbox` / `inbox play`. Implemented as a new auto-responder responseType ('mailbox'), reusing the existing DM-gating, per-node cooldown, param extraction, and per-source scoping. - DB: dead_drop_messages table (SQLite/PostgreSQL/MySQL) + migration 092 - Repository (Drizzle-only) + DatabaseService.deadDrop accessor - DeadDropService: command brain (store/inbox/play[/sender]/delete/clear, 180-byte cap, per-recipient & per-sender caps, 7-day expiry, batch play) - meshtasticManager: 'mailbox' responseType dispatch branch - UI: 'Mailbox' response type option + guidance in the auto-responder editor - Tests: 34 (migration registry, repository perSource, service) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(dead-drop): add Mailbox option to the Add-Trigger form select The Mailbox response type was only added to the per-trigger edit view (TriggerItem); the separate Add-Trigger form in AutoResponderSection had its own type <select> (Text/HTTP/Script) that was missed, so new mailbox triggers couldn't be created from the UI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(dead-drop): don't require response text to add a Mailbox trigger The Add button's disabled gate required a non-empty response field for all types; Mailbox has no response, so the button stayed greyed out. Exempt mailbox from the response-required check (matches validateResponse). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(dead-drop): accept mailbox responseType in settings save validation The settings-save validator required a non-empty response for every trigger and only allowed text/http/script responseTypes, so saving a Mailbox trigger failed with a generic 400. Exempt mailbox from the response-required check and add it to the responseType allowlist. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(dead-drop): settings-save accepts mailbox triggers without response text Regression coverage for the mailbox responseType in the autoResponderTriggers validator: a mailbox trigger with empty response now saves (200), while non-mailbox empty responses and unknown responseTypes still 400. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(dead-drop): tolerate optional command keyword prefix The mailbox service parsed hardcoded msg/inbox, but a trigger configured with prefixed keywords (e.g. betamsg/betainbox, to coexist with another responder already using msg/inbox) would fire the handler yet fall through to help. Strip an optional non-space prefix from the leading verb so prefixed keywords parse correctly; no-op for plain msg/inbox. Caught by live over-the-air testing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(dead-drop): add Mailbox feature docs + live testing brief - automation.md: Mailbox response type + 'Mailbox (Dead Drop)' section (commands, recipient matching, delivery format, limits, command-prefix coexistence, configuration). - dev-notes/DEAD_DROP_TESTING.md: architecture, automated coverage, and the over-the-air validation results (ALTO MF / ALTO LF / ZN Office). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(dead-drop): register dead_drop_messages in migrate-db table lists The cross-database migrate-db CLI tracks every schema table in TABLE_ORDER / SKIP_TABLES; migrationTables.test.ts fails if a new schema table isn't listed. Add dead_drop_messages to TABLE_ORDER and SOURCE_SCOPED_TABLES (it carries a sourceId, like auto_favorite_targets). Caught by the full Vitest suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(dead-drop): address PR review feedback - Mark messages played from the delivery-success callback, not at enqueue: handleCommand returns { responses, playOnDelivery }; a dropped body DM now leaves the message pending instead of losing it. (#1) - Wire purgeExpired into databaseMaintenanceService so expired rows are reclaimed daily (purgeExpired returns a count). (#2) - Count the per-recipient cap across the recipient's identity forms via an injected node resolver, so it can't be bypassed by addressing one node by several name forms. (#3) - Mailbox bypasses the per-node cooldown (interactive flow). (#4) - inbox play <sender> filter matches !hex/node-num forms too. (#5) - Non-DM commands return [] (no unsolicited DM). (#6) - inbox delete returns the same response for not-yours vs non-existent ids (no id enumeration). (#7) - Wrap the mailbox dispatch in try/catch like the script branch. (#8) - Remove the command-prefix tolerance: canonical msg/inbox only. (#9) - Use shared nodeIdHex (unsigned coerce) for the mailbox log target. (#10) Docs + tests updated to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(dead-drop): recommend enabling Verify response on the mailbox trigger Played-state is committed from the delivery-success callback; with Verify response off (maxAttempts=1) a single unacked send could mark a voicemail played on transmit. Document enabling Verify response so undelivered bodies resurface. (PR #3538 review follow-up) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: chrisn <chrisn@DebDev1.corp.tlclocal.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Randall Hand <randall.hand@gmail.com>

Yeraze merged commit da2b728 into main Sep 28, 2025
2 checks passed

Yeraze deleted the fix/docker-publish-on-release-only branch September 28, 2025 16:34

Yeraze added a commit that referenced this pull request Oct 12, 2025

Merge pull request #10 from Yeraze/fix/docker-publish-on-release-only

23f37ef

claude Bot mentioned this pull request Nov 5, 2025

feat: Add configurable favorite telemetry storage and viewing periods (#411) #462

Merged

10 tasks

Yeraze mentioned this pull request Jun 18, 2026

feat: Dead Drop / Mailbox auto-responder (async per-source message store) #3538

Merged

Yeraze mentioned this pull request Jun 20, 2026

feat(notifications): surface device ClientNotifications + firmware 2.8 favorite/ignore cap (#3548) #3578

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Change Docker publishing to trigger only on GitHub Releases#10

fix: Change Docker publishing to trigger only on GitHub Releases#10
Yeraze merged 1 commit into
mainfrom
fix/docker-publish-on-release-only

Yeraze commented Sep 28, 2025

Uh oh!

claude Bot commented Sep 28, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Yeraze commented Sep 28, 2025

Summary

Problem

Solution

Changes

Behavior After Merge

What Will Trigger Builds:

What Will NOT Trigger Builds:

Benefits

Test Plan

Notes

Uh oh!

claude Bot commented Sep 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review in Progress

Todo List

Analysis Complete - Providing Review

🔍 Code Review: Docker Workflow Changes

✅ Excellent Changes - Well Implemented

Code Quality & Best Practices ⭐

Security Assessment 🔒

Performance Considerations ⚡

Specific Changes Analysis

🎯 Implementation Correctness

📋 Test Coverage Implications

🚀 Deployment Readiness

📝 Minor Suggestions

⚠️ Testing Recommendation

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

claude Bot commented Sep 28, 2025 •

edited

Loading