Skip to content

fix(incident): read/write lifecycle via SDK 'state', not nonexistent 'status'#55

Merged
srgfrancisco merged 1 commit into
mainfrom
fix/incident-state-field
May 12, 2026
Merged

fix(incident): read/write lifecycle via SDK 'state', not nonexistent 'status'#55
srgfrancisco merged 1 commit into
mainfrom
fix/incident-state-field

Conversation

@srgfrancisco

Copy link
Copy Markdown
Owner

Summary

The Datadog v2 SDK exposes the incident lifecycle as state on IncidentResponseAttributes — there is no status attribute. getattr(attrs, "status", "") therefore always returned an empty string against the real API, so incident list, get, create, and update all displayed empty status for every incident.

Likewise, IncidentUpdateAttributes has no status (or state) attribute — state changes must flow through fields["state"] as a dropdown (same pattern already used for severity). The previous code set attrs_kwargs["status"] = status, which was silently a no-op on the wire.

Verified directly against the installed SDK:

IncidentResponseAttributes  → has: state, severity, fields, title…  (no `status`)
IncidentUpdateAttributes    → has: fields, title…                   (no `status`/`state`)

Why this slipped past tests

The existing fixtures built Mock(status=...). Mock accepts any attribute name silently, so the wrong field name was never detected. This PR switches the fixture to Mock(spec_set=[..., "state", ...]) so any future reversion will fail loudly.

Changes

  • ddogctl/commands/incident.py:
    • All reads: getattr(attrs, "status", "")getattr(attrs, "state", "") (5 sites)
    • update_incident: route --status into fields["state"] = {"type": "dropdown", "value": status} instead of an attribute kwarg
  • tests/commands/test_incident.py:
    • Fixture now uses Mock(spec_set=[...]) with state (not status) so the mock matches the real SDK shape
    • New test_update_incident_status_sent_as_fields_state regression test asserting --status lands in fields["state"] on the request body, and that no status/state attribute is set on IncidentUpdateAttributes
    • New test_update_incident_json_reads_state verifying JSON output's status key sources from attrs.state

CLI-facing JSON output key stays "status" to match the existing --status flag.

Context

This is the underlying bug that PR #48 (closed — hostile fork attempting a project-wide rebrand) was nominally pointing at. This PR fixes only that bug, scoped to two files.

Test plan

  • uv run pytest tests/commands/test_incident.py -v → 17 passed
  • uv run pytest tests/ → 690 passed
  • uv run black --check ddogctl/ tests/ → clean
  • uv run ruff check ddogctl/ tests/ → clean
  • Smoke against a real Datadog tenant: ddogctl incident list shows non-empty status, and ddogctl incident update <id> --status resolved actually resolves the incident

…'status'

The Datadog v2 SDK exposes the incident lifecycle as `state` on
`IncidentResponseAttributes`; `status` is not a field. Reading
`getattr(attrs, "status", "")` therefore always returned "" against the
real API, making list/get/create/update show empty status. Likewise,
`IncidentUpdateAttributes` has no `status` attribute — state changes
must flow through `fields["state"]` as a dropdown (same pattern as
severity), so `--status` was silently a no-op on the wire.

Fixes both directions: reads come from `attrs.state`, writes route
through `fields["state"]`. CLI-facing JSON key stays `"status"` to match
the existing `--status` flag and avoid breaking output consumers.

Tests now use `Mock(spec_set=...)` with `state` so the wrong attribute
name can't slip past again, plus a regression test that asserts
`update --status` lands in `fields["state"]` on the request body.
@srgfrancisco srgfrancisco merged commit 34b0994 into main May 12, 2026
11 checks passed
srgfrancisco added a commit that referenced this pull request May 12, 2026
)

Three small CLAUDE.md tweaks driven by the PR #55 incident-state bug
and a shift in the local worktree workflow:

- Add a Gotcha for the datadog_api_client SDK: verify model field
  names against openapi_types before reading or writing. v2 incidents
  expose `state` (not `status`), and lifecycle changes flow through
  `fields["state"]` as a dropdown, not a top-level attribute.
  `getattr(obj, "wrong_name", "")` masks this silently.
- Sharpen the Mock(spec_set=...) note in Testing Patterns to explain
  why it matters for SDK response mocks: bare Mock(field=...) accepts
  any attribute name, which is exactly how the incident bug shipped.
- Drop the `git gtr` recommendations from Development Workflow and
  Gotchas. The local preference is now `claude --worktree`, falling
  back to plain `git worktree add` / `git worktree remove` when a
  session was started without it.
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