From 75999e3c5fb13df2f142fbcb7a87422c1485bdbf Mon Sep 17 00:00:00 2001 From: icciAaron Date: Thu, 16 Apr 2026 10:44:42 -0400 Subject: [PATCH 1/2] fix(job): scope lock to --run, fix exit code, remove dead code Three issues in the fwconsole job command: 1. Lock acquired too early: the LockableTrait lock was acquired at the top of execute(), blocking --list, --enable, and --disable when a --run was already in progress. These non-destructive operations do not need mutual exclusion. 2. Lock contention returned exit code 0 (success). Cron and scripts could not detect that the job run was skipped. Return 1 and wrap the message in tags for console visibility. 3. Dead code: duplicate if($input->getOption('disable')) block at line 61 (copy-paste of the block at line 56) was unreachable and contained unrelated runJobs() logic. Move the lock to immediately before the --run dispatch so that --list, --enable, and --disable execute without contention. Reported-by: Aaron Salsitz Co-Authored-By: Claude Code --- .../admin/libraries/Console/Job.class.php | 40 +++++++++---------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/amp_conf/htdocs/admin/libraries/Console/Job.class.php b/amp_conf/htdocs/admin/libraries/Console/Job.class.php index 6f1e8fa2f7..ffd87705c6 100644 --- a/amp_conf/htdocs/admin/libraries/Console/Job.class.php +++ b/amp_conf/htdocs/admin/libraries/Console/Job.class.php @@ -39,11 +39,6 @@ protected function execute(InputInterface $input, OutputInterface $output){ $this->output = $output; $this->input = $input; - if (!$this->lock()) { - $output->writeln('The command is already running in another process.'); - return 0; - } - if($input->getOption('force')) { $this->force = true; } @@ -58,22 +53,6 @@ protected function execute(InputInterface $input, OutputInterface $output){ return 0; } - if($input->getOption('disable')) { - $this->runJobs($this->registerTasks($this->findAllJobs([$input->getOption('run')]))); - return 0; - } - - if($input->hasParameterOption('--run') && is_null($input->getOption('run'))) { - //run all - $this->runJobs($this->registerTasks($this->findAllJobs())); - return 0; - } - - if($input->getOption('run')) { - $this->runJobs($this->registerTasks($this->findAllJobs([$input->getOption('run')]))); - return 0; - } - if($input->getOption('list')) { $table = new Table($output); $table->setHeaders(array(_('ID'),_('Module'),_('Job'),_('Cron'),_('Next Run'),_('Action'),_("Enabled"))); @@ -94,6 +73,25 @@ protected function execute(InputInterface $input, OutputInterface $output){ return 0; } + // Acquire lock only for --run to prevent concurrent job execution. + // Non-destructive operations (--list, --enable, --disable) do not + // need mutual exclusion and should not be blocked by a running job. + if (!$this->lock()) { + $output->writeln('The command is already running in another process.'); + return 1; + } + + if($input->hasParameterOption('--run') && is_null($input->getOption('run'))) { + //run all + $this->runJobs($this->registerTasks($this->findAllJobs())); + return 0; + } + + if($input->getOption('run')) { + $this->runJobs($this->registerTasks($this->findAllJobs([$input->getOption('run')]))); + return 0; + } + $this->outputHelp($input,$output); return 0; } From 1cecfde71da19a6cf77db70a3c34faa283053297 Mon Sep 17 00:00:00 2001 From: icciAaron Date: Thu, 16 Apr 2026 10:50:38 -0400 Subject: [PATCH 2/2] fix(job): gate lock behind --run parameter check The lock must only be acquired when --run is present. Without this gate, running `fwconsole job` with no arguments (help display) would unnecessarily contend for the lock and show an error instead of help when another --run is in progress. Consolidate the two --run branches (run-all vs run-specific) into a single --run block for clarity. Co-Authored-By: Claude Code --- .../admin/libraries/Console/Job.class.php | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/amp_conf/htdocs/admin/libraries/Console/Job.class.php b/amp_conf/htdocs/admin/libraries/Console/Job.class.php index ffd87705c6..77cbd621dc 100644 --- a/amp_conf/htdocs/admin/libraries/Console/Job.class.php +++ b/amp_conf/htdocs/admin/libraries/Console/Job.class.php @@ -74,21 +74,20 @@ protected function execute(InputInterface $input, OutputInterface $output){ } // Acquire lock only for --run to prevent concurrent job execution. - // Non-destructive operations (--list, --enable, --disable) do not - // need mutual exclusion and should not be blocked by a running job. - if (!$this->lock()) { - $output->writeln('The command is already running in another process.'); - return 1; - } - - if($input->hasParameterOption('--run') && is_null($input->getOption('run'))) { - //run all - $this->runJobs($this->registerTasks($this->findAllJobs())); - return 0; - } + // Non-destructive operations (--list, --enable, --disable, help) + // do not need mutual exclusion and should not be blocked. + if($input->hasParameterOption('--run')) { + if (!$this->lock()) { + $output->writeln('The command is already running in another process.'); + return 1; + } - if($input->getOption('run')) { - $this->runJobs($this->registerTasks($this->findAllJobs([$input->getOption('run')]))); + if(is_null($input->getOption('run'))) { + //run all + $this->runJobs($this->registerTasks($this->findAllJobs())); + } else { + $this->runJobs($this->registerTasks($this->findAllJobs([$input->getOption('run')]))); + } return 0; }