Skip to content

fix(sync): do not blank a ResourceTarget id when the lookup misses - #1581

Open
cplieger wants to merge 2 commits into
moghtech:mainfrom
cplieger:fix/resource-target-id-blanked-on-lookup-miss
Open

cplieger wants to merge 2 commits into
moghtech:mainfrom
cplieger:fix/resource-target-id-blanked-on-lookup-miss

Conversation

@cplieger

@cplieger cplieger commented Aug 13, 2026 •

Copy link
Copy Markdown
Contributor

Managed-mode writeback can emit TOML that Komodo itself cannot parse, which leaves the sync broken until someone edits the file by hand. Hit this on a live deployment; the deploy pipeline stopped until I removed the offending line.

What happens

A Commit Sync wrote this into resources.toml:

[[alerter]]
name = "Discord-stack-state"
[alerter.config]
alert_types = ["StackStateChange"]
resources = [{ type = "Stack" }]

Every read after that failed:

ERROR: failed to read resources from ".../resources.toml"
  1: failed to parse resource file contents
  2: TOML parse error at line 1548, column 14
     resources = [{ type = "Stack" }]
                  ^^^^^^^^^^^^^^^^^^
     missing field `id`

Why

  1. replace_resource_target_ids! rewrote each id to the resource name with .map(|r| r.name.clone()).unwrap_or_default(), so a missed lookup yields an empty String.
  2. ResourceTarget is adjacently tagged, #[serde(tag = "type", content = "id")], so id is required to deserialize.
  3. TOML_PRETTY_OPTIONS sets skip_empty_string: true, so the empty id is omitted from the emitted table.

In my case the lookup missed because the file supplied the target by name, which is how every other synced reference is written, while the cache is keyed by id. Whether names should be accepted in that position is a separate question; this PR only stops a miss from corrupting the file.

The change

Leave the id untouched when the lookup misses. On a hit nothing changes; on a miss the previous outcome was a file that cannot be read back.

DeploymentImage::Build has the same shape (build_id blanked, adjacently tagged) and is covered in #1584. The plain String fields nearby (server_id, swarm_id, linked_repo, builder_id) still parse when empty, so they are unaffected.

Verification

cargo check -p komodo_core clean with no new warnings, cargo fmt --check clean. No test: the macro reads the global all_resources_cache() and bin/core has no test scaffolding for it.

Managed-mode writeback can emit TOML that Komodo itself cannot parse, which
leaves the sync permanently broken until a human edits the file.

replace_resource_target_ids! rewrote each id to the resource name and fell
back to unwrap_or_default() when the lookup missed, so a miss produced an
empty String. ResourceTarget is adjacently tagged (tag = "type", content =
"id"), and TOML_PRETTY_OPTIONS sets skip_empty_string, so the empty id is
dropped from the emitted table. The result is

  resources = [{ type = "Stack" }]

which cannot be deserialized back into a ResourceTarget, so every later read
of that file fails with 'missing field id'.

Leaving the id untouched on a miss keeps the value round-trippable. Nothing
else changes: the only behavioural difference is what happens on a miss, and
the previous outcome on a miss was an unparseable file.

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.

1 participant