Problem/Motivation

The natural upgrade sequence breaks the site.

Steps to reproduce

On a site running 1.13.x:

composer require drupal/mcp_sentinel:^2.0
drush updatedb

mcp_sentinel.audit_logger has a hard dependency on audit_chain.logger, so the moment the 2.0.0 code lands and audit_chain is not yet enabled, the container cannot compile:

The service "mcp_sentinel.audit_logger" has a dependency on a
non-existent service "audit_chain.logger".

The front end returns 500 and — this is the trap — you cannot run drush en audit_chain to recover, because drush needs the same container. Reproduced on a real site.

The obvious rollback makes it worse: composer require drupal/mcp_sentinel:^1.13 also removes audit_chain, since at that point it is only a transitive dependency.

The only working order installs and enables the dependency before the code that needs it:

composer require drupal/audit_chain:^1.0.1
drush en audit_chain -y
composer require drupal/mcp_sentinel:^2.0
drush updatedb -y

Proposed resolution

Documenting the ordering is not enough. A required-dependency addition that bricks a site on the wrong ordering is a landmine, the error names a service rather than the module to enable, and anyone running a routine composer update followed by drush updatedb in a deploy takes production down with no obvious recovery path under pressure.

Make the container able to build without audit_chain enabled, and fail closed at runtime instead:

  • Inject @?audit_chain.logger so the reference is optional at compile time.
  • Have McpAuditLogger refuse to operate when it is absent — throw from log() with a message naming the module to enable, and surface it on the status report via hook_requirements(). Explicitly not a silent no-op: an audit logger that quietly stops recording is the exact failure this module exists to prevent.
  • Consider installing the module from an update hook so drush updatedb self-heals, once the container can build far enough to run it.

Then document the ordering anyway, as belt and braces.

Remaining tasks

The 2.0.0 release notes now carry the ordered sequence, but the code should not depend on operators reading it.

Implementation notes: mcp_sentinel issue 71.

Comments

jmcerda created an issue. See original summary.

jmcerda’s picture

Status: Active » Fixed

Fixed on the 1.x branch (GitHub PR #81).

Four changes, because documenting the ordering was not a fix:

- the service reference is optional (`@?audit_chain.logger`), so the container builds without audit_chain enabled;
- `McpAuditLogger` fails **closed** — a governed write with no chain throws naming the module, rather than succeeding unaudited;
- `hook_requirements()` reports the missing module at ERROR on the status report;
- `drush updatedb` self-heals. Update 10016 installs audit_chain before migrating, and 10017 covers sites with no legacy audit table for 10016 to migrate.

That last point was a real defect in the first attempt: update hooks run in ascending order, so 10017 — the hook written to install audit_chain — ran *after* 10016, which threw when the chain table was missing. On the one upgrade path 10017 existed for, it never ran at all. A test now pins the ordering by asserting the install precedes the throw within 10016.

Verified by reproducing the original failure: reverting the optional injection produces the reported error verbatim — `The service "mcp_sentinel.audit_logger" has a dependency on a non-existent service "audit_chain.logger"`. A kernel test installs the module without audit_chain and asserts the container compiles and every path fails honestly.

`docs/UPGRADE.md` documents the ordered sequence as well, for anyone landing 2.0.0 or 2.0.1 specifically.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

jmcerda’s picture

Version: 2.0.0 » 2.1.0

Fixed in 2.1.0 (GitHub #71 / PR #81). Optional @?audit_chain.logger, fail-closed logger, requirements ERROR, updates 10016/10017 self-heal.

jmcerda’s picture

Status: Fixed » Closed (fixed)

Shipped in 2.1.0. Closing after release.