carverauto/serviceradar

Fork 0

initial #2586

Merged

mfreeman451 merged 2 commits from refs/pull/2586/head into staging

2025-12-16 23:53:16 +00:00

mfreeman451 commented

2025-12-16 23:39:04 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2162
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2162
Original created: 2025-12-16T23:39:04Z
Original updated: 2025-12-16T23:53:23Z
Original head: carverauto/serviceradar:2151-poller-status-updates-overwrite-registration-metadata-with-defaults-upsert
Original base: staging
Original merged: 2025-12-16T23:53:16Z by @mfreeman451

User description

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

I have signed the DCO?
The build completes without errors?
All tests are passing when running make test?

PR Type

Bug fix

Description

Prevent poller status updates from overwriting registration metadata with defaults
Modify UPSERT conflict clause to only update operational fields
Preserve registration identity/provenance across heartbeat updates
Add regression test to prevent metadata clobbering

Diagram Walkthrough

flowchart LR
  A["Poller Status Update"] -->|"UPSERT with conflict"| B["CNPG Registry"]
  C["Service Registry Registration"] -->|"Update metadata"| B
  B -->|"Preserve: component_id, registration_source, spiffe_identity, metadata"| D["Registration Metadata"]
  B -->|"Update: last_seen, is_healthy, updated_at"| E["Operational State"]

File Walkthrough

Relevant files

Bug fix

cnpg_registry.go

Restrict poller status UPSERT to operational fields only

pkg/db/cnpg_registry.go

Remove registration metadata columns from UPSERT conflict update
clause
Keep only operational fields: first_seen (with COALESCE), last_seen,
is_healthy, updated_at
Prevent component_id, registration_source, status, spiffe_identity,
metadata, created_by, agent_count, checker_count, first_registered
from being overwritten

+2/-11

Tests

cnpg_registry_test.go

Add regression test for metadata preservation

pkg/db/cnpg_registry_test.go

Add new test TestUpsertPollerStatusSQL_PreservesRegistrationMetadata
Verify UPSERT contains only operational field updates
Assert registration metadata columns are absent from conflict clause
Add strings import for SQL normalization

+22/-0

Documentation

proposal.md `Proposal document for metadata preservation fix` openspec/changes/fix-poller-status-metadata-clobber/proposal.md Document the issue: status updates overwriting registration metadata with defaults Outline changes to CNPG UPSERT semantics Define write ownership boundaries between registration and status paths List affected code files and expected impact	+20/-0
design.md `Design document with context and decision rationale` openspec/changes/fix-poller-status-metadata-clobber/design.md Explain context of shared `pollers` row between registration and status writes Define goals: preserve metadata, keep updates cheap, maintain ownership boundaries Document decision to treat status writes as operational-only Discuss alternatives considered and risk mitigations Note no data migration required	+43/-0
spec.md `Service registry spec for metadata preservation` openspec/changes/fix-poller-status-metadata-clobber/specs/service-registry/spec.md Add requirement that status updates preserve registration metadata Define scenarios for explicitly registered pollers retaining metadata Specify that `first_seen` timestamps are not cleared by status updates Establish registration writes as authoritative for metadata	+27/-0
tasks.md `Implementation and validation task checklist` openspec/changes/fix-poller-status-metadata-clobber/tasks.md Document implementation tasks with completion status Include SQL update, timestamp handling, regression tests, call site audit List validation steps: spec validation and targeted Go tests	+10/-0

Imported from GitHub pull request. Original GitHub pull request: #2162 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2162 Original created: 2025-12-16T23:39:04Z Original updated: 2025-12-16T23:53:23Z Original head: carverauto/serviceradar:2151-poller-status-updates-overwrite-registration-metadata-with-defaults-upsert Original base: staging Original merged: 2025-12-16T23:53:16Z by @mfreeman451 --- ### **User description** ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test? ___ ### **PR Type** Bug fix ___ ### **Description** - Prevent poller status updates from overwriting registration metadata with defaults - Modify UPSERT conflict clause to only update operational fields - Preserve registration identity/provenance across heartbeat updates - Add regression test to prevent metadata clobbering ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Poller Status Update"] -->|"UPSERT with conflict"| B["CNPG Registry"] C["Service Registry Registration"] -->|"Update metadata"| B B -->|"Preserve: component_id, registration_source, spiffe_identity, metadata"| D["Registration Metadata"] B -->|"Update: last_seen, is_healthy, updated_at"| E["Operational State"] ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>cnpg_registry.go</strong><dd><code>Restrict poller status UPSERT to operational fields only</code>  </dd></summary> <hr> pkg/db/cnpg_registry.go <ul><li>Remove registration metadata columns from UPSERT conflict update <br>clause<br> <li> Keep only operational fields: <code>first_seen</code> (with COALESCE), <code>last_seen</code>, <br><code>is_healthy</code>, <code>updated_at</code><br> <li> Prevent <code>component_id</code>, <code>registration_source</code>, <code>status</code>, <code>spiffe_identity</code>, <br><code>metadata</code>, <code>created_by</code>, <code>agent_count</code>, <code>checker_count</code>, <code>first_registered</code> <br>from being overwritten</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2162/files#diff-e31b8b854d9ba774d2f3ed9899b1f5902462cd0e63990a2898dcaf9bc171d571">+2/-11</a>    </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>cnpg_registry_test.go</strong><dd><code>Add regression test for metadata preservation</code>                        </dd></summary> <hr> pkg/db/cnpg_registry_test.go <ul><li>Add new test <code>TestUpsertPollerStatusSQL_PreservesRegistrationMetadata</code><br> <li> Verify UPSERT contains only operational field updates<br> <li> Assert registration metadata columns are absent from conflict clause<br> <li> Add <code>strings</code> import for SQL normalization</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2162/files#diff-e38716ee5b3614089e00e851d22d2c61e9934ebcc2b44b7aa4b79623251af869">+22/-0</a>    </td> </tr> </table></td></tr><tr><td><strong>Documentation</strong></td><td><table> <tr> <td> <details> <summary><strong>proposal.md</strong><dd><code>Proposal document for metadata preservation fix</code>                    </dd></summary> <hr> openspec/changes/fix-poller-status-metadata-clobber/proposal.md <ul><li>Document the issue: status updates overwriting registration metadata <br>with defaults<br> <li> Outline changes to CNPG UPSERT semantics<br> <li> Define write ownership boundaries between registration and status <br>paths<br> <li> List affected code files and expected impact</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2162/files#diff-511a066de4507065281b37a979c77119b90e46e97e206f2e879f8af73d13c854">+20/-0</a>    </td> </tr> <tr> <td> <details> <summary><strong>design.md</strong><dd><code>Design document with context and decision rationale</code>            </dd></summary> <hr> openspec/changes/fix-poller-status-metadata-clobber/design.md <ul><li>Explain context of shared <code>pollers</code> row between registration and status <br>writes<br> <li> Define goals: preserve metadata, keep updates cheap, maintain <br>ownership boundaries<br> <li> Document decision to treat status writes as operational-only<br> <li> Discuss alternatives considered and risk mitigations<br> <li> Note no data migration required</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2162/files#diff-fd19193e9fe30b9b73e8c397a9a274eafa39efe8cef21fd07190038fc867112c">+43/-0</a>    </td> </tr> <tr> <td> <details> <summary><strong>spec.md</strong><dd><code>Service registry spec for metadata preservation</code>                    </dd></summary> <hr> openspec/changes/fix-poller-status-metadata-clobber/specs/service-registry/spec.md <ul><li>Add requirement that status updates preserve registration metadata<br> <li> Define scenarios for explicitly registered pollers retaining metadata<br> <li> Specify that <code>first_seen</code> timestamps are not cleared by status updates<br> <li> Establish registration writes as authoritative for metadata</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2162/files#diff-e8ea81126ba5d49a821706cd6479d176ac1ee40a496dbb38938d830ea94d3e0a">+27/-0</a>    </td> </tr> <tr> <td> <details> <summary><strong>tasks.md</strong><dd><code>Implementation and validation task checklist</code>                          </dd></summary> <hr> openspec/changes/fix-poller-status-metadata-clobber/tasks.md <ul><li>Document implementation tasks with completion status<br> <li> Include SQL update, timestamp handling, regression tests, call site <br>audit<br> <li> List validation steps: spec validation and targeted Go tests</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2162/files#diff-6aa430ae5e044c6f55358d45cd7b2368b1168a735aa4f84739c6102fa7769720">+10/-0</a>    </td> </tr> </table></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-12-16 23:39:55 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2162#issuecomment-3662903204
Original created: 2025-12-16T23:39:55Z

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance

🟢

No security concerns identified

No security vulnerabilities detected by AI analysis. Human verification advised for critical code.

Ticket Compliance

🟡

🎫 #2151

🟢	Modify the CNPG poller status UPSERT so the `ON CONFLICT` update does not overwrite registration metadata with defaults and only updates operational fields (e.g., `last_seen`, `is_healthy`, `updated_at`).
	Ensure status-only calls that omit `FirstSeen` do not clear/overwrite `first_seen`/`first_registered`.
	Add regression coverage to prevent reintroducing registration-metadata clobbering.
⚪	Preserve poller registration metadata (e.g., `component_id`, `registration_source`, `status`, `spiffe_identity`, `metadata`, `created_by`, `first_registered`, `first_seen`) when `UpdatePollerStatus` is called to update operational state.
⚪	Confirm/audit that `UpdatePollerStatus` call sites are intended to be operational-only and that registration/upsert paths remain authoritative for registration metadata.

Codebase Duplication Compliance

⚪

Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance

🟢

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

⚪

Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2162#issuecomment-3662903204 Original created: 2025-12-16T23:39:55Z --- _You are nearing your monthly Qodo Merge usage quota. For more information, please visit [here](https://qodo-merge-docs.qodo.ai/installation/qodo_merge/#cloud-users)._ ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td>🟢</td><td><details><summary><strong>No security concerns identified</strong></summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>🟡</td> <td> <details> <summary>🎫 <a href=https://github.com/carverauto/serviceradar/issues/2151>#2151</a></summary> <table width='100%'><tbody> <tr><td rowspan=3>🟢</td> <td>Modify the CNPG poller status UPSERT so the <code>ON CONFLICT</code> update does not overwrite <br>registration metadata with defaults and only updates operational fields (e.g., <code>last_seen</code>, <br><code>is_healthy</code>, <code>updated_at</code>).</td></tr> <tr><td>Ensure status-only calls that omit <code>FirstSeen</code> do not clear/overwrite <br><code>first_seen</code>/<code>first_registered</code>.</td></tr> <tr><td>Add regression coverage to prevent reintroducing registration-metadata clobbering.</td></tr> <tr><td rowspan=2>⚪</td> <td>Preserve poller registration metadata (e.g., <code>component_id</code>, <code>registration_source</code>, <code>status</code>, <br><code>spiffe_identity</code>, <code>metadata</code>, <code>created_by</code>, <code>first_registered</code>, <code>first_seen</code>) when <br><code>UpdatePollerStatus</code> is called to update operational state.</td></tr> <tr><td>Confirm/audit that <code>UpdatePollerStatus</code> call sites are intended to be operational-only and <br>that registration/upsert paths remain authoritative for registration metadata.</td></tr> </tbody></table> </details> </td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=2>🟢</td><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=4>⚪</td> <tr><td align="center" colspan="2"> - [ ] Update  </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>

qodo-code-review[bot] commented

2025-12-16 23:40:53 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2162#issuecomment-3662909521
Original created: 2025-12-16T23:40:53Z

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Code Suggestions ✨

Explore these optional code suggestions:

Category Suggestion Impact

General

Category	Suggestion	Impact
General	Improve test robustness against regressions To make the test more robust, isolate the `UPDATE` clause of the SQL query and assert that registration-related column names are not present, rather than checking for an exact string match. pkg/db/cnpg_registry_test.go [15-34] func TestUpsertPollerStatusSQL_PreservesRegistrationMetadata(t testing.T) { normalized := strings.Join(strings.Fields(upsertPollerStatusSQL), " ") + updateClauseMarker := "ON CONFLICT (poller_id) DO UPDATE SET " + parts := strings.Split(normalized, updateClauseMarker) + require.Len(t, parts, 2, "SQL statement should contain exactly one ON CONFLICT...DO UPDATE SET clause") + updateClause := parts[1] - assert.Contains(t, normalized, "ON CONFLICT (poller_id) DO UPDATE SET") + // Assert that operational fields are updated + assert.Contains(t, updateClause, "first_seen = COALESCE(pollers.first_seen, EXCLUDED.first_seen)") + assert.Contains(t, updateClause, "last_seen = EXCLUDED.last_seen") + assert.Contains(t, updateClause, "is_healthy = EXCLUDED.is_healthy") + assert.Contains(t, updateClause, "updated_at = EXCLUDED.updated_at") - assert.Contains(t, normalized, "first_seen = COALESCE(pollers.first_seen, EXCLUDED.first_seen)") - assert.Contains(t, normalized, "last_seen = EXCLUDED.last_seen") - assert.Contains(t, normalized, "is_healthy = EXCLUDED.is_healthy") - assert.Contains(t, normalized, "updated_at = EXCLUDED.updated_at") - - assert.NotContains(t, normalized, "component_id = EXCLUDED.component_id") - assert.NotContains(t, normalized, "registration_source = EXCLUDED.registration_source") - assert.NotContains(t, normalized, "status = EXCLUDED.status") - assert.NotContains(t, normalized, "spiffe_identity = EXCLUDED.spiffe_identity") - assert.NotContains(t, normalized, "metadata = EXCLUDED.metadata") - assert.NotContains(t, normalized, "created_by = EXCLUDED.created_by") - assert.NotContains(t, normalized, "agent_count = EXCLUDED.agent_count") - assert.NotContains(t, normalized, "checker_count = EXCLUDED.checker_count") - assert.NotContains(t, normalized, "first_registered = EXCLUDED.first_registered") + // Assert that registration metadata fields are NOT updated + assert.NotContains(t, updateClause, "component_id =") + assert.NotContains(t, updateClause, "registration_source =") + assert.NotContains(t, updateClause, "status =") + assert.NotContains(t, updateClause, "spiffe_identity =") + assert.NotContains(t, updateClause, "metadata =") + assert.NotContains(t, updateClause, "created_by =") + assert.NotContains(t, updateClause, "agent_count =") + assert.NotContains(t, updateClause, "checker_count =") + assert.NotContains(t, updateClause, "first_registered =") } Apply / Chat* Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a weakness in the new test and proposes a more robust implementation that better prevents future regressions, improving the overall quality of the test suite.	Low
Update

Improve test robustness against regressions

To make the test more robust, isolate the UPDATE clause of the SQL query and
assert that registration-related column names are not present, rather than
checking for an exact string match.

pkg/db/cnpg_registry_test.go [15-34]

 func TestUpsertPollerStatusSQL_PreservesRegistrationMetadata(t *testing.T) {
 	normalized := strings.Join(strings.Fields(upsertPollerStatusSQL), " ")
+	updateClauseMarker := "ON CONFLICT (poller_id) DO UPDATE SET "
+	parts := strings.Split(normalized, updateClauseMarker)
+	require.Len(t, parts, 2, "SQL statement should contain exactly one ON CONFLICT...DO UPDATE SET clause")
+	updateClause := parts[1]
 
-	assert.Contains(t, normalized, "ON CONFLICT (poller_id) DO UPDATE SET")
+	// Assert that operational fields are updated
+	assert.Contains(t, updateClause, "first_seen = COALESCE(pollers.first_seen, EXCLUDED.first_seen)")
+	assert.Contains(t, updateClause, "last_seen = EXCLUDED.last_seen")
+	assert.Contains(t, updateClause, "is_healthy = EXCLUDED.is_healthy")
+	assert.Contains(t, updateClause, "updated_at = EXCLUDED.updated_at")
 
-	assert.Contains(t, normalized, "first_seen = COALESCE(pollers.first_seen, EXCLUDED.first_seen)")
-	assert.Contains(t, normalized, "last_seen = EXCLUDED.last_seen")
-	assert.Contains(t, normalized, "is_healthy = EXCLUDED.is_healthy")
-	assert.Contains(t, normalized, "updated_at = EXCLUDED.updated_at")
-
-	assert.NotContains(t, normalized, "component_id = EXCLUDED.component_id")
-	assert.NotContains(t, normalized, "registration_source = EXCLUDED.registration_source")
-	assert.NotContains(t, normalized, "status = EXCLUDED.status")
-	assert.NotContains(t, normalized, "spiffe_identity = EXCLUDED.spiffe_identity")
-	assert.NotContains(t, normalized, "metadata = EXCLUDED.metadata")
-	assert.NotContains(t, normalized, "created_by = EXCLUDED.created_by")
-	assert.NotContains(t, normalized, "agent_count = EXCLUDED.agent_count")
-	assert.NotContains(t, normalized, "checker_count = EXCLUDED.checker_count")
-	assert.NotContains(t, normalized, "first_registered = EXCLUDED.first_registered")
+	// Assert that registration metadata fields are NOT updated
+	assert.NotContains(t, updateClause, "component_id =")
+	assert.NotContains(t, updateClause, "registration_source =")
+	assert.NotContains(t, updateClause, "status =")
+	assert.NotContains(t, updateClause, "spiffe_identity =")
+	assert.NotContains(t, updateClause, "metadata =")
+	assert.NotContains(t, updateClause, "created_by =")
+	assert.NotContains(t, updateClause, "agent_count =")
+	assert.NotContains(t, updateClause, "checker_count =")
+	assert.NotContains(t, updateClause, "first_registered =")
 }

Apply / Chat

Suggestion importance[1-10]: 6

Why: The suggestion correctly identifies a weakness in the new test and proposes a more robust implementation that better prevents future regressions, improving the overall quality of the test suite.

Low

Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2162#issuecomment-3662909521 Original created: 2025-12-16T23:40:53Z --- _You are nearing your monthly Qodo Merge usage quota. For more information, please visit [here](https://qodo-merge-docs.qodo.ai/installation/qodo_merge/#cloud-users)._ ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>General</td> <td> <details><summary>Improve test robustness against regressions</summary> ___ **To make the test more robust, isolate the <code>UPDATE</code> clause of the SQL query and <br>assert that registration-related column names are not present, rather than <br>checking for an exact string match.** [pkg/db/cnpg_registry_test.go [15-34]](https://github.com/carverauto/serviceradar/pull/2162/files#diff-e38716ee5b3614089e00e851d22d2c61e9934ebcc2b44b7aa4b79623251af869R15-R34) ```diff func TestUpsertPollerStatusSQL_PreservesRegistrationMetadata(t *testing.T) { normalized := strings.Join(strings.Fields(upsertPollerStatusSQL), " ") + updateClauseMarker := "ON CONFLICT (poller_id) DO UPDATE SET " + parts := strings.Split(normalized, updateClauseMarker) + require.Len(t, parts, 2, "SQL statement should contain exactly one ON CONFLICT...DO UPDATE SET clause") + updateClause := parts[1] - assert.Contains(t, normalized, "ON CONFLICT (poller_id) DO UPDATE SET") + // Assert that operational fields are updated + assert.Contains(t, updateClause, "first_seen = COALESCE(pollers.first_seen, EXCLUDED.first_seen)") + assert.Contains(t, updateClause, "last_seen = EXCLUDED.last_seen") + assert.Contains(t, updateClause, "is_healthy = EXCLUDED.is_healthy") + assert.Contains(t, updateClause, "updated_at = EXCLUDED.updated_at") - assert.Contains(t, normalized, "first_seen = COALESCE(pollers.first_seen, EXCLUDED.first_seen)") - assert.Contains(t, normalized, "last_seen = EXCLUDED.last_seen") - assert.Contains(t, normalized, "is_healthy = EXCLUDED.is_healthy") - assert.Contains(t, normalized, "updated_at = EXCLUDED.updated_at") - - assert.NotContains(t, normalized, "component_id = EXCLUDED.component_id") - assert.NotContains(t, normalized, "registration_source = EXCLUDED.registration_source") - assert.NotContains(t, normalized, "status = EXCLUDED.status") - assert.NotContains(t, normalized, "spiffe_identity = EXCLUDED.spiffe_identity") - assert.NotContains(t, normalized, "metadata = EXCLUDED.metadata") - assert.NotContains(t, normalized, "created_by = EXCLUDED.created_by") - assert.NotContains(t, normalized, "agent_count = EXCLUDED.agent_count") - assert.NotContains(t, normalized, "checker_count = EXCLUDED.checker_count") - assert.NotContains(t, normalized, "first_registered = EXCLUDED.first_registered") + // Assert that registration metadata fields are NOT updated + assert.NotContains(t, updateClause, "component_id =") + assert.NotContains(t, updateClause, "registration_source =") + assert.NotContains(t, updateClause, "status =") + assert.NotContains(t, updateClause, "spiffe_identity =") + assert.NotContains(t, updateClause, "metadata =") + assert.NotContains(t, updateClause, "created_by =") + assert.NotContains(t, updateClause, "agent_count =") + assert.NotContains(t, updateClause, "checker_count =") + assert.NotContains(t, updateClause, "first_registered =") } ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion correctly identifies a weakness in the new test and proposes a more robust implementation that better prevents future regressions, improving the overall quality of the test suite. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>