Fix critical deployment bugs from audit#88
Merged
jonathonbyrdziak merged 2 commits intomasterfrom Mar 22, 2026
Merged
Conversation
Bug #1: PID key mismatch — ProtocolStart, ProtocolStop, IncidentDetector read 'git.slave.pid' but GitSlave writes 'slave.pid'. Fixed all consumers. Bug #2: release-watcher writes release.current on failed deploys, making failures permanently un-retryable. Now only updates on successful promotion. Bug #3: BlueGreen::promote() stops old containers before starting new, guaranteeing downtime. Rewritten to start new first, then stop old, then restart old on shadow ports for standby rollback. Bug #7: JsonLock/Json singleton caches never cleared in long-running release-watcher daemon. Added clearInstances() calls each poll cycle. Bug #12: GitSlave::execute() calls JsonLock::delete() on every start, wiping ALL lock file state (release.current, blue-green state, etc). Changed to only clear the stale PID key. Bug #13: release-watcher had zero logging between startup and release detection. Added comprehensive poll cycle logging and moved credential refresh before API calls. Bug #14: Shell::run() appended '2>&1' to every command, swallowing stderr and mixing it with stdout. Removed — callers that need stderr redirect explicitly. Bug #15: Shell::isLockedPIDStillRunning() read from Json (protocol.json) instead of JsonLock (protocol.lock) for PID. Fixed import and call. Also: Config::clearInstances() added for daemon cache clearing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Resolve conflicts with PR #87's merge into master: - ProtocolStart, ProtocolStop, IncidentDetector: use DeploymentState API (supersedes the manual PID key fix since DeploymentState handles it) - release-watcher: keep master's wlog() version, re-apply bug fixes: - Bug #7: singleton cache clearing each poll cycle - Bug #13: credential refresh failure logging - Bug #2: release.current only written on successful promotion Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
Fixes 8 critical/high-severity bugs found during deployment system audit. These bugs caused:
Bugs Fixed
git.slave.pid, writer usesslave.pidrelease.currentwritten on failed deploys — failures become permanentpromote()stops old containers before starting new — guaranteed downtimeJsonLock::delete()on every GitSlave start wipes ALL lock file stateShell::run()appends2>&1to all commands, swallowing stderrisLockedPIDStillRunning()reads Json instead of JsonLock for PIDFiles Changed
bin/release-watcher.php— Bugs repo:slave should kill the server if it falls out of sync #2, protocol key:generate should save keyfile information into protocol.json #7, protocol start should first check system requirements #13src/Commands/GitSlave.php— Bug BUG #12src/Commands/ProtocolStart.php— Bug Docker image should be updated #1src/Commands/ProtocolStop.php— Bug Docker image should be updated #1src/Helpers/IncidentDetector.php— Bug Docker image should be updated #1src/Helpers/BlueGreen.php— Bug Welcome to Protocol Discussions! #3src/Helpers/Shell.php— Bugs Git objects are bloating #14, protocol composer install inside docker container #15src/Utils/Config.php—clearInstances()for daemon cacheTest plan
protocol restartstarts watcher without wiping lock file🤖 Generated with Claude Code