fix(monitor): correct status_code assertion docs + orphan-cleanup safety net#16
Merged
Merged
Conversation
…ety net
Round-2 DevEx surfaced two issues with `assertions { type = "status_code" }`:
1. **Docs example used `expected = 200`** (number). The API contract is
`String expected` (so it can carry "200", "2xx", "200-299"), and Jackson
silently coerces the number to "200" on store. The round-trip then trips
Terraform's "Provider produced inconsistent result after apply" check
AND, because the API call had already succeeded by then, leaves an
orphaned monitor server-side that no Terraform state file references.
Fix: example now uses `expected = "200"` with an inline comment
explaining why. Schema description for `assertions[].config` calls out
the type-strictness rule explicitly (status_code.expected is STRING,
response_time.thresholdMs is NUMBER).
2. **Create-flow had no orphan rollback.** If `mapToState` or
`resp.State.Set` failed AFTER the API create, the monitor leaked.
Fix: thread a `cleanupOrphan` closure through Create and call DELETE
if either downstream step returns an error. Logged at Debug on success
and Warn on failure (so the leaked-id is visible to operators in
either case).
This does NOT cover the framework's post-Create consistency check
(which runs after our function returns and is what produced the
round-2 reports). The fix for that path is the doc/example change
above — once user values match the API's wire format, the matching
logic in `mapToState` produces consistent state and the framework
never errors. The cleanup closure protects against any future
internal failure in the same window.
Round-2 DevEx friction P0 #1 + P0 #3 (see /tmp/devex-cli-test/FRICTION-TABLE.md).
Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Round-2 DevEx surfaced two issues with the documented
assertions { type = "status_code" }example:P0 #1 — Docs example uses wrong JSON value type (causes orphan monitor)
StatusCodeAssertion.expectedisStringAPI-side (so it can carry"200","2xx","200-299"). The example usedjsonencode({ expected = 200, ... })— a JSON number. Jackson coerces number → string on the API side, then the round-trip diff trips Terraform's"Provider produced inconsistent result after apply"check.Worse: the API call had already succeeded by the time the consistency check runs, so the monitor is left orphaned server-side with no Terraform state pointing to it. Reproduced live against prod (orphan id
b0ee7ba2-…cleaned up via direct API call).Fix: example uses
expected = "200"with an inline comment explaining the type rule. Schema description forassertions[].configcalls out the type-strictness rule explicitly so a fresh dev sees it in the Registry schema reference.P0 #3 — Create flow had no orphan rollback
If
mapToStateorresp.State.Setfails AFTER the APICreatecall succeeds, the monitor was leaked. Now: acleanupOrphanclosure DELETEs the orphan and logs at Debug (success) / Warn (failure) so the leaked id is visible.Limitation (documented in code): this does NOT catch the framework's post-Create consistency check — that runs after our function returns and there's no hook for it. The doc fix above is the actual fix for the round-2 reports; the cleanup closure protects against any future internal failure in the same window.
Verification
make lint(0 issues)go test ./...(all green incl. 27.9s API integration suite)make docs— committed regenexpected = 200) and corrected variant (expected = "200") both tested against prod; right variant applies + destroys cleanly with no orphan.Releases
After merge, cut
0.2.0-beta.2. Registry will refresh ~5min later.Test plan
terraform applyof the corrected example against prodRound-2 DevEx friction P0 #1 + P0 #3 (see `/tmp/devex-cli-test/FRICTION-TABLE.md`).
Made with Cursor