Do not require os in recipe manifest Platform blocks (#974)

[allanli4]

Supersedes #1158 (closed). This is the same change, on a dev/ branch on
origin per docs/CONTRIBUTING.md, which — confirmed empirically — is also the
only route on which uat.yml runs at all rather than being skipped.

Read the scope section before reviewing. This change is necessary but
not sufficient to resolve #974: it does not fix the reporter's cloud-side
ResolveComponentCandidates failure. It fixes the on-device half, which is the
half that lives in this repository. Whether a partial fix is acceptable is a
maintainer decision, and this PR is deliberately not making it for you.

Issue

#974 —
"ggdeploymentd: Do not require os and architecture fields in component recipes"
Reported by: @aws-kevinrickard

Problem

The AWS component recipe reference documents os and architecture as optional
attributes of a manifest Platform block, but nucleus lite behaves as if os is
required. A manifest such as:

Manifests:
  - Platform:
      runtime: "*"

is silently treated as incompatible with the current platform. Expected
behaviour, per the reporter, is to match the non-lite nucleus and impose no
requirement when the fields are absent.

Scope — this fix is NECESSARY BUT NOT SUFFICIENT for the reported symptom

This change does not fix the failure the reporter observed. The error they
quoted —
doesn't claim platform {runtime=aws_nucleus_lite, os=linux, architecture=amd64} compatibility
— is produced by cloud-side ResolveComponentCandidates in the AWS IoT
Greengrass service. That matching logic is not in this repository, is not
reachable from this code, and is not reproducible on a developer host. Nothing in
this patch can change it. A reporter who applies this change and retries a cloud
deployment of a recipe with only runtime: "*" will see the same error.

What this change does fix is the on-device manifestation of the same
documented-contract violation: nucleus lite's own local platform matching also
treated os as required, so a recipe omitting it failed artifact resolution,
systemd unit-file generation, and lifecycle execution locally. That is a real,
independently observable defect against the same public documentation, and it is
the part that lives in this repository. Fixing it is a prerequisite for the
end-to-end behaviour the issue asks for — hence necessary — but the cloud-side
resolver must be addressed separately for the reporter's scenario to work.

The platform attributes nucleus lite sends to the cloud
(modules/ggdeploymentd/src/deployment_handler.c:1152-1157) are unchanged; they
already populate all three fields, so they are not the cause either.

Recommendation: do not close #974 on this change alone. Either keep it open
pending the service-side fix, or split the cloud-side half into its own tracking
item.

Acceptance criteria

All criteria are DERIVED — the issue states none explicitly.

#	Criterion	Met	How
1	Platform with a lite-compatible runtime but no os is compatible on Linux	Yes	Fix at recipe.c:399-411; test select_manifest_platform_without_os
2	Platform with no architecture imposes no architecture requirement (no regression)	Yes	architecture.len == 0 branch preserved byte-identical; test select_manifest_platform_without_architecture
3	Platform: { runtime: "*" } (both omitted — the issue's exact input) is selected on Linux	Yes	Test select_manifest_platform_without_os_or_architecture — asserts GG_ERR_OK and deep map equality of the selected manifest
4	Existing negatives preserved: no runtime; non-lite runtime; explicit non-matching os; explicit non-matching architecture — all still skipped	Yes	Four tests: without_runtime, with_other_runtime, with_other_os, with_other_architecture
5	Non-string os / architecture still yields GG_ERR_INVALID	Yes	Tests with_invalid_os, with_invalid_architecture. See "Behaviour changes" — for architecture this is a strictness increase
6	Covered by an automated test in the existing framework	Yes	Inline GG_TEST_DEFINE in the module's own #ifdef GG_SDK_TESTING block, run by ctest as ggl-recipe-inline-test. No new framework or target

Root cause

manifest_selection() in modules/ggl-recipe/src/recipe.c carries a two-channel
contract: the return value answers "did I see malformed input?", while the
out-parameter answers "did this manifest match?". GG_ERR_OK is therefore
overloaded — it is returned both on a match and on a deliberate skip — so any
path that exits without writing the out-parameter is read by callers as "skip
this manifest". Skip is reached by omission rather than by decision.

At the base revision, if (gg_map_get(platform, GG_STR("os"), &os_obj))
(recipe.c:400) used the presence of the os key as the gate around the entire
remaining compatibility-and-selection body (recipe.c:401-484) — which contains
every statement that can assign the out-parameter. With os absent, the whole
body was skipped, the function returned GG_ERR_OK having selected nothing, the
caller's error guard did not fire, and the post-loop NULL check produced
No Manifest was found for linux (recipe.c:609) → GG_ERR_FAILURE.

The comment at recipe.c:398 — // If OS is not provided then do nothing —
described the code accurately and the requirement inaccurately: "impose no OS
requirement" was implemented as "do nothing", and in this function doing nothing
means being skipped. Tellingly, architecture one level down was already
implemented correctly (absent → zero-length buffer → accepted at recipe.c:434),
but only reachable when os was present.

The prediction this hypothesis made, and it held: making an absent os yield
a wildcard instead of skipping the body, with everything downstream semantically
unchanged, makes the failure disappear. It did, on the first attempt.

Set aside, one line each:

• Extract a platform_is_compatible() predicate (the alternative fix shape
  proposed during localization). Rejected: it bundles a refactor with a bug fix,
  which docs/CONTRIBUTING.md explicitly asks contributors not to do, and
  manifest_selection() already carries // TODO: Refactor it, so maintainers
  have scoped that separately. Its one correctness argument — the unreachable
  architecture type check — is delivered by this fix anyway. Offered as a
  follow-up below.
• manifest_selection() never reads architecture.detail, though nucleus lite
  advertises it. A real defect, but a different one — not folded in.
• Cloud-side resolver behaviour — not in this repository. See "Scope".

The fix

One file: modules/ggl-recipe/src/recipe.c.

The production change is four edits inside manifest_selection():

1. GgObject *os_obj; → GgObject *os_obj = NULL; so absence is representable.
2. The gg_map_get(... "os" ...) block now contains only the non-string type
   check; it no longer gates anything.
3. An absent os defaults to GG_STR("*"), i.e. "no OS requirement", applied
   with the same optional-fetch shape the adjacent architecture block already
   uses. recipe.c:332 already does GgBuffer arch_detail = GG_STR("");, so this
   is established local style.
4. The former gated body is de-indented one level so it always runs.

Why this is minimal: the wildcard default means the OS predicate itself needs
no edit at all. The runtime gate, the OS predicate, the architecture
fetch/validate/default, the architecture predicate, the Lifecycle pick, the
Selections pick, and the all default are all byte-identical apart from
indentation — git diff -w reduces the production change to exactly the four
items above. The remainder of the diff is the unavoidable re-indent plus tests.
No public signature changes; manifest_selection is static, and
modules/ggl-recipe/include/ggl/recipe.h is untouched.

Blast radius of the bug being fixed — one root cause, four entry points:

Entry point	Path	Effect of the bug
modules/ggdeploymentd/src/deployment_handler.c:722	ggl_get_recipe_artifacts_for_platform	artifact download fails
modules/recipe2unit/src/unit_file_generator.c:499	select_linux_lifecycle	systemd unit generation fails
modules/recipe-runner/src/runner.c:385	select_linux_lifecycle	lifecycle scripts never run
modules/ggl-docker-client/src/docker_artifact_cleanup.c:116, :260	ggl_get_recipe_artifacts_for_platform	Docker image cleanup blocked

Diff accounting, hunk by hunk

git diff --shortstat main..HEAD reports +188/-69 on one file, which is a lot
for a validation relaxation. CONTRIBUTING requires narrow diffs, so here is
every line accounted for. Ignoring whitespace the same diff is +137/-18, so
roughly 51 insertions and 51 deletions are whitespace-only.

Group	Size	Justification
A. Behaviour change	5 lines	GgObject *os_obj = NULL;, the wildcard default GgBuffer os = GG_STR("*");, the if (os_obj != NULL) { os = ... } guard, and moving the closing brace so the type check no longer gates the body. This is the entire semantic change.
B. Comment correction	2 lines	// If OS is not provided then do nothing → ...it imposes no requirement. The old comment documented the bug; leaving it would mislead the next reader.
C. Forced re-indent	53 removed / 53 added, whitespace-only	Removing the if (gg_map_get(... "os" ...)) wrapper eliminates one nesting level, so its 84-line body shifts left by 4 columns. Unavoidable: the body must become unconditional, and in C that means removing a brace level. Statement order and content are byte-identical — git diff -w shows nothing here.
D. clang-format re-joins	6 hunks, ~16 lines	Lines that were wrapped at the deeper indent now fit within ColumnLimit: 80, and .clang-format requires joining them. Verified: clang-format --dry-run -Werror passes on the changed region, and would fail if these were left wrapped. Not discretionary reformatting — the repository's own formatter mandates it.
E. Tests	~118 lines	Ten GG_TEST_DEFINE cases plus three fixture macros. This is the oracle and the criterion coverage; see below.

On the alternative that would have avoided group C entirely. The re-indent can
be dodged by keeping the wrapper if and making its condition always true —
synthesise a default GgObject for the absent case, then test os_obj != NULL
instead of the map lookup. That yields a ~7-line total diff. It was rejected:
it leaves a permanently-true conditional in the code, which is worse to read and
maintain than a correct nesting level, and readability-* clang-tidy checks are
active. Trading permanent code quality for a one-time smaller diff is the wrong
trade; group C is whitespace-only and reviewable with git diff -w.

On the test volume. Groups A-D total ~23 non-whitespace lines; the tests are
the bulk of the diff. They are not optional — the oracle-first design makes the
failing-test-turned-green the entire basis of the claim, and each of the six
acceptance criteria needs a test that fails if the behaviour regresses. Three
fixture macros already cut roughly 90 lines of duplication versus writing the
recipe literals out longhand (the idiom used by
modules/ggdeploymentd/src/component_config.c). If a maintainer wants the set
narrowed, the two most trimmable are select_manifest_platform_without_os
(partially subsumed by the oracle) and
select_lifecycle_platform_without_os_or_architecture (covers the second entry
point, whose assertion is the weakest) — but that trades regression coverage for
line count, so it is offered rather than assumed.

Note on "zero test files". This diff touches no file named *test*, and that
is correct for this repository: module unit tests live inside the module's own
.c file under #ifdef GG_SDK_TESTING, and root CMakeLists.txt:377 registers
with add_test only the ${name}-inline-test targets built from those blocks.
Ten other modules follow this idiom. The test_modules/* directories build
standalone binaries that ctest never runs, so a test placed there would not
execute in CI and could not serve as an oracle. A filename-based "does this change
include tests?" check will report a false negative here.

Device verification on real hardware

Beyond the unit suite and CI, the change was proved end to end on a real device,
in both directions.

Host: EC2 i-0f9a24a9c5535efcc, c6i.xlarge, Ubuntu 24.04.4 (glibc 2.39, gcc 13),
us-west-2. Reached over SSM send-command only — zero inbound security-group
rules, no SSH key. Both .debs were built on the device, and each build's
HEAD was asserted equal to the expected commit before the artifact was trusted.

Component deployed, whose manifest Platform omits both os and
architecture — the issue's exact reproduction:

Manifests:
  - Platform:
      runtime: "*"
    Artifacts: []
    Lifecycle:
      run: |
        echo "NoOsPlatform running"

Driven with
ggl-cli deploy -r <recipes> -a <artifacts> -c com.example.NoOsPlatform=1.0.0,
identically in both directions, with full device state reset in between
(purge + rm -rf /var/lib/greengrass, verified residual_units=0).

	Patched a068edfa	Unpatched base b33c3682
Runtime provenance	Nucleus version: 2.6.0-a068edf	Nucleus version: 2.6.0-b33c368
Source probe on device	fixed comment present, 19 GG_TEST_DEFINE	buggy comment present, 9 GG_TEST_DEFINE
Generated unit in /var/lib/greengrass/	ggl.com.example.NoOsPlatform.service after 5 s	none
Verdict	GREEN	RED

Verbatim rejection from the unpatched base build:

D[recipe2unit] parser.c:116: Attempting to find bootstrap phase from recipe
E[ggl-recipe] recipe.c:532: No lifecycle was found for linux
W[ggdeploymentd] deployment_handler.c:2847: Completed deployment processing and reporting job as FAILED.

Health was gated per-unit on ggl.core.ggconfigd.service and
ggl.core.ggdeploymentd.service, both active with NRestarts=0 in both
directions. Worth flagging for anyone testing this runtime:
systemctl is-active greengrass-lite.target reported active while six units
were failed, so the aggregate target is not a usable health signal. The six
failures are all credential-dependent units (iotcored, tesd,
gg-fleet-statusd and their sockets), expected because the device was
deliberately brought up with no cloud identity.

One finding here is worth a reviewer's attention. On a real deployment the
failure surfaces at recipe.c:532 — select_linux_lifecycle, reached via
recipe2unit — not at recipe.c:609 (select_linux_manifest) where the unit-test
oracle fails. Both entry points share the single root cause, and this independently
justifies the select_lifecycle_platform_without_os_or_architecture test in this
patch: select_linux_lifecycle is the path real deployments take and it had no
regression test at all before this change.

All off-tree resources were torn down and the teardown verified by re-query:
instance terminated, no tagged instances, no orphan volumes, security group
InvalidGroup.NotFound, IAM role and instance profile NoSuchEntity.

Rebase onto current main

Rebased from the original base b33c3682 onto d3c2fa3f, picking up two
merged sibling PRs (#1161, #1162). No conflicts — those changes touch
modules/ggipcd/, modules/ggl-config-interpolation/, and
modules/recipe2unit/; this patch touches only modules/ggl-recipe/src/recipe.c.
Verified rather than assumed: the merged file list and this patch's file list are
disjoint. The diff is byte-identical in shape after the rebase (+188/-69, one
file).

Everything below was re-verified against the new baseline, not carried over:

Check	Result on 4677c296
Build	0 errors
Full suite	100% tests passed, 0 tests failed out of 11 (11 not 10 — a sibling added a test target)
clang-format 19.1.7	no violations
Compiler warnings	4 — all pre-existing at d3c2fa3f

The 4 warnings were attributed by building d3c2fa3f alone in a separate
worktree: it produces the identical 4 at the identical lines —
modules/iotcored/src/tls.c:183 and :184 (-Wconversion), and
_deps/unity-src/src/unity.c:2158 and :2174 ('noreturn' function does return,
vendored third-party). Zero warnings arise in the changed file. They only became
visible now because the rebase forced a fuller rebuild than the previous
incremental one.

The load-bearing test, and why it was strengthened

Device verification changed which test matters. On a real deployment the failure
surfaces at recipe.c:532 — select_linux_lifecycle, reached via recipe2unit —
not at recipe.c:609 (select_linux_manifest) where the unit oracle fails.
select_linux_lifecycle is the path real deployments take, and it had no
regression test at all before this change. That makes
select_lifecycle_platform_without_os_or_architecture the load-bearing test, not
the incidental one.

It originally asserted only selected.len > 0, which is too loose for a test
carrying that weight — it would accept a lifecycle rather than the right one.
It now asserts deep map equality:

GG_TEST_ASSERT_OK(select_linux_lifecycle(recipe_map, &selected));
GG_TEST_ASSERT_MAP_EQUAL(TEST_LIFECYCLE_MAP, selected);

TEST_LIFECYCLE_MAP was factored out so the fixture and the expectation cannot
drift apart; the property asserted is "the returned lifecycle is the manifest's
lifecycle", which is exactly right and survives a future change to the fixture.

Mutation-tested, so it is not tautological. Changing only the expectation (not
the shared fixture macro) to a different literal map makes the test fail with
FAIL: Values were not equal; restoring it — file checksum verified identical —
returns it to PASS. GG_TEST_ASSERT_MAP_EQUAL is a deep comparison, not a
length check.

Verified against the non-lite nucleus

The issue asks for behaviour that matches the classic nucleus, so that claim was
checked against the reference implementation rather than inferred from the
documentation. The authoritative matcher is
PlatformHelper.isRequirementSatisfied / isAttributeSatisfied in
aws-greengrass-component-common,
reached from the nucleus via PlatformResolver.findBestMatch →
RecipeLoader.loadFromFile.

Recipe Platform input	Classic nucleus	Lite after this patch
os key omitted	no requirement — the matcher streams over platformRequirement.entrySet(), i.e. only keys the recipe declares, so an omitted key is never tested	no requirement ✅
architecture key omitted	no requirement, same reason	no requirement ✅
os: "*"	wildcard	wildcard ✅
os: "all"	wildcard (legacy special case)	wildcard ✅
os: "" (present, empty)	rejected — fails the ^[a-zA-Z0-9] "simple label" guard	rejected ✅
os: "windows" on Linux	rejected	rejected ✅
non-matching architecture	rejected	rejected ✅

So the change this PR makes — an omitted os imposes no requirement, while an
explicitly empty one does not match — is what the classic nucleus does. The
wildcard-for-absent choice was not merely a plausible reading of the docs.

Divergences that remain, all pre-existing and out of scope here:

• Absent or empty Platform object. Classic returns true outright —
  // no platform is considered a wild-card. Lite still returns GG_ERR_INVALID
  ("Platform not provided"). Tracked separately; this also settles the open design
  question there, in favour of "matches everything" rather than "skip as
  nucleus-only".
• architecture: "". Lite treats a present-but-empty value as a wildcard via
  its architecture.len == 0 branch; classic rejects it by the same simple-label
  guard that rejects os: "". This is the one place lite is more permissive than
  classic, and it is untouched by this patch.
• any is a legacy wildcard in classic for os/architecture; lite accepts
  * and all but not any.
• /regex/ templates are supported by classic and not by lite.
• runtime has no special meaning in classic — it is matched like any other
  attribute. Lite's rule that a manifest omitting runtime is nucleus-only and
  must be skipped is a deliberate lite-specific divergence, and this patch leaves
  it exactly as it was.

Behaviour changes worth accepting knowingly

Both follow from the fix being correct, and both are user-visible:

1. Manifest precedence. In
   Manifests: [{runtime: "*"}, {runtime: "*", os: linux, architecture: amd64}]
   the first entry used to be skipped and the second selected; the first is now
   selected. That is correct first-match semantics and matches classic nucleus,
   but it changes which Lifecycle / Artifacts block a real deployment picks.
2. One previously-tolerated malformed recipe now hard-fails.
   Platform: {runtime: "*", architecture: 42} with no os used to be silently
   skipped, letting a later valid manifest win. Because the architecture type
   check was also nested inside the os gate, de-nesting makes it reachable, so
   this now returns GG_ERR_INVALID and fails the recipe. Consistent with how a
   malformed os already behaved, and it closes criterion 5, but it is a
   strictness increase beyond the reported defect.

Evidence

Oracle — the failing test that now passes:

• Test: modules/ggl-recipe/src/recipe.c ::
  select_manifest_platform_without_os_or_architecture

• Command: ctest --test-dir <build> -R ggl-recipe-inline-test --output-on-failure

• Before the patch, at base revision b33c36822f2f5f9b8ea873954142bfa98dff7aaa:

  E[ggl-recipe] modules/ggl-recipe/src/recipe.c:609: No Manifest was found for linux
  modules/ggl-recipe/src/recipe.c:758:test_gg_select_manifest_platform_without_os_or_architecture:FAIL: Return value was not GG_ERR_OK
  E[gg-test] .../unity/gg_test/process_wait.c:27: Process 8 exited with status 1.
  
  -----------------------
  10 Tests 1 Failures 0 Ignored
  FAIL
  

• After the patch: passes. That symptom no longer occurs.
  ggl-recipe-inline-test reports 19 Tests 0 Failures 0 Ignored / OK.

Reproduction was deterministic (1/1); the defect is a pure control-flow branch on
map-key presence, with no timing or I/O dependence.

Regression run — build and test:

Check	Command	Result
Build	cmake --build <build> -j$(nproc)	clean — 0 errors, 0 warnings
Test suite	ctest --test-dir <build> --output-on-failure	100% tests passed, 0 tests failed out of 10
Lint	nix flake check -L	not run — see below

• Patch-induced failures: none.
• Pre-existing failures at base revision, deliberately left alone: the test
  suite was clean at base, so there are none in the recorded command set. Two
  environment-specific ones were found by extra checks and left alone; see below.

CI results on this branch — the checks have now actually run, so the earlier
"could not be verified locally" caveats are superseded. Run: CI 30412935626 on
a068edfa, conclusion success.

Check	Result
formatting	pass (failed on the first push; fixed — see below)
clang-tidy	pass
unit-tests	pass — the 10 new inline tests ran in CI for the first time
build-clang	pass
build-musl-pi	pass
iwyu	pass
cmake-lint	pass
spelling	pass
editorconfig	pass
shellcheck	pass
packages-default	pass
nix flake check --no-build	pass
CodeQL, Analyze (c-cpp / python / actions)	pass
uat-test (47-job matrix)	pass — 47/47

All 64 checks on this PR pass. Zero failures.

Two things this settles that were previously only reasoned about:

• clang-tidy passes. The concern was readability-function-cognitive-complexity
  (active in .clang-tidy) against the NOLINTNEXTLINE that manifest_selection()
  still carries. The patch removes a nesting level, so the suppression remains
  valid. No suppression was added, loosened, or moved to obtain this.
• spelling passes, so the new identifiers are acceptable to cspell.

The one formatting failure, and the fix. First push (4740f7b9) failed with:

check-formatting> File ./modules/ggl-recipe/src/recipe.c not formatted

Patch-induced, not pre-existing: CI on main at the base revision
b33c36822f2f is green for formatting. Cause: CI formats with clang-format
19.1.7 (nix clang-19.1.7), and after the de-indent that version prefers

GG_LOGE("Platform architecture is invalid. It must be a string"
);

over the wrapped form. Fixed by applying exactly that, and nothing else — one
whitespace-only hunk, inside the region this patch already touches. Verified by
re-running clang-format 19.1.7 over the whole file (clean) and re-running the
build and full test suite before re-pushing.

Worth recording for future contributors: the version matters. An earlier local
check used clang-format 22.1.8, which disagreed with 19.1.7 in both
directions — it demanded a change 19.1.7 does not want in
is_recipe_variable_valid_three_part (an untouched, already-committed line, so
"pre-existing violation" was an artifact of the wrong tool version, not a real
finding), and it accepted the GG_LOGE wrapping that 19.1.7 rejects.

uat-test — 47/47 pass (run 30412935701, conclusion success, on
a068edfa). Worth noting how it got there, since it is a repository-wide
constraint any contributor will meet: uat.yml serialises UAT across the whole
repository:

concurrency:
  group: uat-component-tests
  cancel-in-progress: false

One global slot, newer runs queue. Three dev/* branches pushed within ten
minutes cancelled each other's UAT runs, and this commit's run was initially
cancelled with 0 jobs executed. It was re-dispatched once, after confirming
the global group was empty, and then passed cleanly. It was not re-triggered in a
loop — that would cancel a sibling branch's pending run and add load to the
shared, rate-limited AWS account the concurrency group exists to protect.

No UAT job failed. For context, a sibling branch in the same batch observed an
environmental UAT failure mode unrelated to any patch (misc/run_nucleus runs
systemd-tmpfiles --create under set -e and exits 73 where /etc and
/etc/default ownership mismatch on the runner); it did not occur in this run.

Also confirmed empirically: UAT is skipped entirely for fork PRs — the
list-tests job is gated on
github.event.pull_request.head.repo.full_name == 'aws-greengrass/aws-greengrass-lite',
and the two earlier fork-based runs show conclusion skipped. Had this work stayed
on a fork, all 47 UAT jobs would have been silently skipped and the PR would have
looked green while never being exercised. That is why it is on an origin dev/
branch.

Local verification (retained, since it is what drove the patch): build clean
with 0 errors and 0 warnings; ctest 100% tests passed, 0 tests failed out of 10; ggl-recipe-inline-test 19 Tests 0 Failures. A clean local build with
-D ENABLE_WERROR=1 additionally surfaces 2 errors — 'noreturn' function does return in _deps/unity-src/src/unity.c, vendored third-party Unity source,
not repository code. Proven pre-existing by re-running the identical build with
the patch stashed at base revision: same 2 errors, same 2 lines. It is
gcc-specific and invisible upstream because CI's -Werror checks use clang/musl
while unit-tests sets BUILD_TESTING without ENABLE_WERROR, so that
combination never occurs in CI. No repository source file emitted a warning under
-Werror, including the patched file.

Reviewer verdict

An independent critic reviewed the diff, the acceptance criteria, the rejected
alternative hypothesis, and the test results, without access to the patch
author's self-assessment.

Field	Value
Verdict	concur
Confidence	Medium
Outcome applied	Patch preserved

Advisory notes, surfaced rather than acted on. They did not block delivery by
design — the reviewer is advisory, and it is the test run that verified the patch.

1. An explicitly empty os: is still skipped. The reviewer reported that the
   YAML decoder (modules/ggl-yaml/src/yaml_decode.c) renders every scalar as a
   buffer, so os: with no value becomes a zero-length buffer, fails the OS
   predicate, and still produces the exact error from this issue — while
   architecture: left empty is honoured as "no requirement" — and suggested
   accepting os.len == 0 for consistency.

   This note has since been resolved against it, and the current behaviour is
   correct. The classic nucleus also rejects an explicitly empty os: "" (see
   "Verified against the non-lite nucleus" above — it fails the ^[a-zA-Z0-9]
   simple-label guard). Treating absent as a wildcard while treating empty as a
   non-match is exactly the classic semantics, so no change was made here. The
   reviewer's underlying observation was still valuable, but it points the other
   way: it is lite's architecture: "" → wildcard branch that diverges from
   classic, not its handling of os. That branch is pre-existing and deliberately
   untouched by this patch.

2. Add a two-manifest test to pin skip-versus-abort. The four negative tests
   use single-manifest fixtures and GG_TEST_ASSERT_BAD (which only asserts
   != GG_ERR_OK), so nothing asserts that a non-matching manifest is skipped
   while the loop continues rather than aborting the whole recipe. A
   [non-matching, matching] fixture would close that and lock in first-match
   precedence.

3. Silent skips have no diagnostics. The runtime gate logs why it skipped; the
   os mismatch and the architecture mismatch say nothing, and the architecture
   mismatch exits by falling through ~50 lines to the terminal return GG_ERR_OK.
   A GG_LOGD on each skip would have made this issue self-diagnosing.

4. The fall-through shape that caused this bug is still present. Converting
   the implicit fall-through into an explicit else { return GG_ERR_OK; } would
   remove it at near-zero diff cost — without adopting the full predicate
   extraction.

5. Nits: assertion style is mixed across the new tests
   (GG_TEST_ASSERT_BAD for platform-mismatch negatives, exact
   TEST_ASSERT_EQUAL_INT(GG_ERR_INVALID, ...) for type negatives — the exact form
   is better); and encoding "no os requirement" as the sentinel "*" makes the
   fix depend on "*" remaining in the predicate, where a bool os_specified
   would state the intent directly (a deliberate trade for a smaller diff).
   The reviewer's first nit — that
   select_lifecycle_platform_without_os_or_architecture asserted only
   selected.len > 0 — has been addressed, see below.

6. The reviewer was barred from modifying the tree, so it could not mutation-test
   the new assertions; its discrimination claims rest on control-flow reasoning
   plus per-test log paths, not on observed red-on-mutation.

For the human reviewer

Focus here:

• The empty-os: question is now settled, not open. An earlier draft of this
  PR left it for a maintainer to decide. Checking the classic nucleus resolved it:
  classic rejects os: "" too, so lite's current behaviour is right and no change
  was made. See "Verified against the non-lite nucleus".
• The two behaviour changes above — manifest precedence, and the newly
  hard-failing malformed recipe — are correct consequences of the fix, but they
  affect real deployments. Please accept them knowingly.
• Whether the strictness increase in criterion 5 is wanted now or should be
  split into its own change.
• All 64 checks pass, including clang-tidy, unit-tests, and the full 47-job
  uat-test matrix. No suppression was added, no test weakened, and no check
  narrowed to obtain green. CONTRIBUTING step 3's "confirm the CI UAT run on your
  dev/ branch is green" is satisfied.
• The change is additionally proved on real hardware in both directions — see
  the device verification section. The unpatched base build rejects the same recipe
  that the patched build accepts, so the device green is evidence rather than a
  scenario that would have passed anyway.
• Convention compliance: docs/CONTRIBUTING.md's AWS-internal path is
  followed — a dev/-prefixed branch (dev/recipe-platform-os-optional) on
  origin, per "Create a branch off main prefixed with dev/".

Branch: dev/recipe-platform-os-optional on origin — commit 4677c296,
rebased onto main at d3c2fa3f. Originally developed against b33c3682, which
remains the base revision cited in the device-verification and oracle sections
below because that is the revision those comparisons were run against.

Relationship to #1158

#1158 carries this identical commit from a fork branch. It is superseded by this
PR and is being left open for the operator to close; nothing in this run opened,
closed, or modified it. The fork branch
allanli4:wt/LiAllanPersonalAICapabilities-t6 was deliberately left in place,
because deleting it would auto-close #1158.

Why this PR rather than that one: docs/CONTRIBUTING.md puts AWS-internal work on
a dev/-prefixed branch on origin, and .github/workflows/uat.yml triggers UAT
on dev/* branches — so the origin + dev/ route is what gets this change
actually exercised in CI. A fork-based PR does not.