Skip to content

fix: keep the check level when a merged layer does not set one - #2872

Open
tonyandrewmeyer wants to merge 3 commits into
canonical:mainfrom
tonyandrewmeyer:fix/check-level-merge
Open

tonyandrewmeyer wants to merge 3 commits into
canonical:mainfrom
tonyandrewmeyer:fix/check-level-merge

Conversation

@tonyandrewmeyer

@tonyandrewmeyer tonyandrewmeyer commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes two places where combining layers in Harness and Scenario didn't match what Pebble does:

  • Combining check layers cleared the level when the overriding layer didn't set one. Pebble keeps the existing level, as for every other field (Check.Merge in internals/plan/plan.go only copies Level when the other layer's is non-empty).
  • Combining services, checks, or log targets copied the overriding layer's override into the combined plan. Pebble's Merge methods never touch Override, so the combined plan keeps the override of the layer that first added the entry.

Neither affects real Pebble, only the plan that Harness and Scenario build from the layers. Several existing tests expected the copied override, so they now expect the base layer's one instead (or no override at all, where the base layer didn't set one).

Fixes #2894

Combining check layers cleared the level when the overriding layer did not set one. Pebble only copies the level when the other layer's is non-empty, as for every other field, so this affected Harness and Scenario plan rendering rather than real Pebble.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tonyandrewmeyer and others added 2 commits November 11, 2026 15:43
Service, Check and LogTarget _merge copied the new layer's override onto
the existing entry. Pebble's Merge methods never touch Override, so a
combined plan keeps the override of the layer that introduced the entry
(replace, usually), where ops showed merge. Harness and Scenario plans
now match. The tests that expected the copied override now expect
Pebble's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CheckLevel.UNSET and CheckStartup.UNSET look the same in the merge but
mean different things, which the comment this branch replaced used to
say.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harness plan doesn't match Pebble's when merging layers: repeated list entries and a lost check level

2 participants