Skipping a commit-msg action causes `MessageAware::afterAction()` to access `$commitMsg` before initialization

[Bloemendaal]

Hi!

We have the following captain-hook.json:

{
    "config": {
        "ansi-colors": true,
        "fail-on-first-error": false,
        "plugins": [],
        "verbosity": "normal"
    },
    "commit-msg": {
        "enabled": true,
        "actions": [
            {
                "action": "\\Ramsey\\CaptainHook\\ValidateConventionalCommit",
                "options": {
                    "configFile": "./conventional-commits.json"
                },
                "conditions": [
                    {
                        "exec": "\\App\\Concerns\\HookConditions\\NotMerge"
                    },
                    {
                        "exec": "\\CaptainHook\\App\\Hook\\Condition\\Branch\\OnMatching",
                        "args": [
                            "/development|acceptance|master/"
                        ]
                    }
                ]
            }
        ]
    }
}

We only want to run ValidateConventionalCommit when we're on a CI/CD branch. The issue is that when OnMatching skips the ValidateConventionalCommit action, which at some point skips the MessageAware::beforeAction(), the property $commitMsg was never set, meaning the MessageAware::afterAction() fails. When I remove the OnMatching check, or switch to a CI/CD branch that matches our regex, it works fine again.

Claude helped me find this, but I personally don't like AI generated issues, so its response is hidden in a spoiler:

The two vendor files involved

1. vendor/captainhook/captainhook/src/Runner/Hook.php:312-353 — handleAction()

The bug: when doConditionsApply() returns false, the method returns at line 326, skipping beforeAction(). But the finally block at lines 350-353 still calls afterAction($action) unconditionally.

try {
    if (!$this->doConditionsApply($action->getConditions(), $io)) {
        $this->printer->actionSkipped($action);
        return;                          // ← skips beforeAction()
    }
    $this->beforeAction($action);
    ...
} finally {
    $this->hookLog->addActionLog(...);
    $this->afterAction($action);         // ← still runs
}

The fix here would be to track whether beforeAction() actually ran and only invoke afterAction() in that case (or move the afterAction() call out of finally and into the try body after execute()).

2. vendor/captainhook/captainhook/src/Runner/Hook/MessageAware.php:93-104 — afterAction()

The latent issue this exposes: $commitMsg is a typed property without a default, only initialized inside beforeAction() at line 81 (and only for non-cli actions). afterAction() reads it at line 99 with no isset() guard:

public function afterAction(Config\Action $action): void
{
    if (Util::getExecType($action->getAction()) !== 'cli') {
        if ($this->commitMsg->getRawContent() !== ...) {   // ← line 99, uninitialized read
            ...
        }
    }
    parent::afterAction($action);
}

The fix here would be if (isset($this->commitMsg) && $this->commitMsg->getRawContent() !== ...).

Which one is "the" bug?

Both are wrong, and either fix alone would mask the symptom. But the real defect is in Hook.php: the finally lifecycle hook (afterAction) is being called for an action whose beforeAction never ran. MessageAware is just the first subclass that notices, because it has state that's expected to be set up by beforeAction. Any future hook subclass that initializes typed state in beforeAction would hit the same crash.