fix: remove orphaned is_inherited from AttributeInterface GraphQL schema (closes #12)#13
Conversation
|
AGENT_REVIEW_VERDICT: TEST_APPROVED APPROVED WITH SUGGESTIONSThe test is correct, realistic, and directly validates the stated root cause. Minor non-blocking suggestions below. A. Test realismThe test imports the real production class ( Confirmed against source: B. Test correctness
C. Test quality
Non-blocking suggestion: Consider adding positive assertions for the fields that should remain ( def test_attribute_interface_does_not_expose_is_inherited() -> None:
fields = AttributeInterface._meta.fields
assert "is_inherited" not in fields
# Guard against over-removal of legitimate fields
for expected in ("is_default", "is_protected", "updated_at"):
assert expected in fieldsThis is a suggestion only — the single-assertion form is acceptable for merge. D. Alignment with analysisThe analyst's root cause: The docstring Recommended next stepsThe fixer agent should remove AGENT_REVIEW_ITERATION: test-1
|
|
/bug-fix |
|
✅ Bug fixer agent completed successfully! No action needed: empty message received with no task to perform. |
…ema (closes #12) The is_inherited field was removed from BaseAttribute at the runtime level but its Field(Boolean) declaration on AttributeInterface was left behind, causing AttributeError on any query that selected it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Commit pushed:
|
|
AGENT_REVIEW_VERDICT: FIX_APPROVED APPROVED WITH SUGGESTIONSThe fix is correct, minimal, and complete. Two non-blocking suggestions below. A. CorrectnessThe fix removes the single orphaned line
The fix is complete for the stated scope. The one-line deletion is correct; adding a resolver or re-implementing the field would both be wrong. No edge cases are missing given the fix strategy. Any query that previously crashed at runtime with B. Code qualitySingle-line deletion with no unnecessary changes. Changelog entry at Non-blocking suggestion — stale exclusion entry: excluded_fields = [
"id",
"is_default",
"is_from_profile",
"is_inherited", # now dead: field no longer exists on AttributeInterface
"is_protected",
...
]Since the field no longer exists, this entry never matches and is harmless. But it will mislead future maintainers. Safe to remove in a follow-up. C. Documentation alignmentChangelog format matches existing entries. PR body documents scope, non-goals, and impact clearly. The Python SDK ( D. Test qualityThe test at assert "is_inherited" not in AttributeInterface._meta.fields
Non-blocking suggestion (carried from test review): add positive assertions for fields that must remain ( Recommended next steps
AGENT_REVIEW_ITERATION: fix-1
|
Why
is_inherited: Booleanwas advertised byAttributeInterfacein the GraphQL schema, but the corresponding attribute was removed fromBaseAttributein a prior cleanup. Any client that selected this field received anAttributeError: 'String' object has no attribute 'is_inherited'response, withdata: null— a hard runtime crash for introspection-driven clients.Goal: Remove the orphaned schema declaration so the field no longer appears in introspection and the runtime never tries to resolve it.
Non-goals: Re-implementing
is_inheritedat the resolver or data-model level; updating the Python SDK (python_sdk/infrahub_sdk/node/attribute.pystill carries a same-named attribute but that is a separate concern).Closes #12
What changed
is_inheritedno longer appears onAttributeInterfaceor any concrete attribute type in the GraphQL schema. Queries selecting it will now receive a schema validation error rather than a runtimeAttributeError.is_inherited = Field(Boolean)fromAttributeInterfaceinbackend/infrahub/graphql/types/attribute.py(single line).schema/schema.graphqlandfrontend/app/src/shared/api/graphql/generated/graphql-env.d.tswere regenerated to reflect the change.is_default,is_protected,updated_at,source,owner) are untouched. No database migration is required. No REST API contract changes.Fix strategy
The root cause is shallow: a single orphaned field declaration. The
is_inheritedproperty was intentionally removed from the runtime model (FlagPropertyenum has onlyIS_PROTECTED;FlagPropertyMixin._flag_propertiesdrives__init__and never containedis_inherited). The schema declaration should have been removed at the same time.A targeted one-line deletion is the correct fix. Adding a
resolve_is_inheritedresolver returningNonewould keep a dead field in the public schema; addingis_inheritedback toFlagProperty/BaseAttributewould re-introduce removed functionality. Neither alternative is correct.How to review
backend/infrahub/graphql/types/attribute.py— the one-line deletion atAttributeInterface.schema/schema.graphql— 11 occurrences ofis_inheritedremoved (interface + 10 implementors). Verify withgrep is_inherited schema/schema.graphqlreturning nothing.frontend/app/src/shared/api/graphql/generated/graphql-env.d.ts— updated bypnpm run codegen:graphqlto match the new schema.backend/tests/unit/graphql/types/test_attribute.py— asserts"is_inherited" not in AttributeInterface._meta.fields.How to test
Expected: 1 test passes, 705 unit tests pass, grep returns no output.
Impact & rollout
is_inherited— but those clients were already broken at runtime withAttributeError. Removing the field from the schema converts the hard crash into a clean schema validation error, which is the correct behavior. Any client generating code from introspection that auto-selects all fields will now work correctly.Checklist
uv run towncrier create ...)AGENT_FIX_COMPLETE