Conversation
…#10634) The GitHub schema-library repo is superseded by the Infrahub Marketplace as the way teams find and install schemas, so the Getting Started card now links there instead, renamed to "Marketplace" to reflect its broader scope beyond schemas. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
A long rebase, merge or diff update of a large branch could fail with "Cannot release a lock that's no longer owned" after all its work was done: the worker was busy on a CPU-heavy step with no `await`, the heartbeat scheduled on the same event loop never ran, its liveness key expired, and the deadlock cleanup running on another worker deleted the locks it still held. The refresh now runs on a dedicated thread with its own event loop and its own cache connection. A pure-Python stall releases the interpreter lock every few milliseconds, which is all the refresh needs, so the key means "this process is alive" rather than "this process's event loop is idle". Three things then spent the 15-second budget that key allows: - A beat could never return. The cache clients are built with no socket timeout, so a connection that stops answering without closing blocked the beat indefinitely, the stop flag is only read between beats, and `stop` then kept a thread nothing could end. Each beat now carries its own deadline; dropping the connection on failure is what makes that safe, since cancelling a command in flight can leave a response unread on it. - The next beat was scheduled after the current one finished, so the period was the interval plus however long the beat took. It is now anchored before it. - A failed beat waited a full interval before retrying, with the reconnect still ahead of it. It backs off briefly instead. The interval drops to 5 seconds, three beats per expiry rather than one and a half, so one failed or timed-out beat still leaves room for the next to write the key before it expires. This narrows the window rather than closing it: a worker that genuinely cannot reach the cache still loses its key, and the deadlock cleanup still reaps on a single sample of the active-worker set with no grace period. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…y node Updating the diff of a branch with an open proposed change stalled for minutes after the diff was saved when the branch held tens of thousands of changed objects, and because the loop runs without an `await`, it blocked every other task on that worker for the duration. Three scans of a set that the loop around them grows: - `_update_diff_conflicts` scanned `retrieved_diff.nodes` for every updated node and added into that same set on a miss. - `_update_diff_relationship_conflicts` did the same one level down over a relationship's elements. The retrieved diff is fetched with `only_conflicted=True`, so it starts near-empty and almost every element is a miss: measured on a single cardinality-many relationship, 1.1s at 10k peers, 5.4s at 20k and 24.2s at 40k. - The hierarchy enricher called `EnrichedDiffRoot.get_node` in a loop whose `add_parent` grows `root.nodes`. That loop awaits a query per node, so it never starved the event loop; it was wasted CPU on hierarchical diffs. Each now goes through a map built once, and the enricher uses the identifier map the parent adder already maintains. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NATSCache.close_connection() was unimplemented, so nothing ever closed a cache connection, and NATSCache.new() orphaned one whenever anything after the connect failed: the caller never receives the cache, so nothing else holds a reference. The heartbeat reaches that second case by bounding each beat with a deadline, so a KV setup stalling during a cache outage leaked one connection per beat. The cleanup catches BaseException rather than Exception on purpose: the case that matters is cancellation, and CancelledError does not inherit from Exception. With `except Exception` the new test fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The testing guidelines already say to inject a clock rather than freeze one, and to poll rather than sleep, but nothing covered guarding an optimisation with a stopwatch. A threshold like `assert elapsed_seconds < N` passes on a fast runner with the regression present and flakes on a loaded one without it, so it is both a weak guard and a flake; count the work instead, and keep the measurement in the commit message or the pull request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The shed-everything controller, the quiet load signal and the app.state wiring were copied between the admission and CORS component tests. Move them to tests/helpers/admission.py so both import the same fixtures. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A shed 429 now carries an `X-Infrahub-Admission: shed` header. A client may replay a non-idempotent request against a 429 only if the handler never ran, and the body alone cannot promise that: the REST exception handler emits the same integer-code envelope for any error, so a future error with code 429 on a mutating route would have looked like a shed. CORS is registered outside the admission gate so a shed response still passes back through it; without those headers a cross-origin browser blocks the 429 and the client sees an opaque network error instead of the Retry-After hint. The CORS middleware exposes `Retry-After` and the marker itself: which response headers a browser may read is decided by what the API sends, not by the deployment, so there is no setting for them. The preflight exemption stays so the guarantee does not rest on middleware ordering. The gate's docstring, the startup comment, the ASGI middleware guideline and the backpressure page describe the new order; the CORS component test checks the preflight in both orders and exercises the server's own CORS middleware. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The API answers a shed request with 429 and a Retry-After hint. The web UI used to fail such a request at once; it now honours the hint in the transport, below the auth layer and below TanStack Query, which keeps `retry: false`. A shared `retryingFetch` wraps fetch for the GraphQL client, the REST client, the raw fetch helper and the GraphiQL fetcher: at most 3 retries inside a 15s window, an advised wait honoured up to a 10s clamp as the SDK does so the server's escalated 20s/30s advice still yields a replay, jitter on top so a page-load burst does not come back as one wave, and a full-jitter backoff from 300ms when no wait is advised. GET, HEAD and OPTIONS are always replayable. Anything else is replayed only when the 429 carries the admission layer's marker header, which proves the request never reached a handler. An abort rejects with the signal's reason as fetch does, and `init` overrides a Request's method and signal as it does for fetch, including an explicit null signal. Once the retries are spent, a shed surfaces as a single "Infrahub is busy" message, in the toast and in the thrown error alike, recognised before the error catalogue because the shed envelope's integer code is not a catalogue identifier. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… reference The JSON schema carries no default for a field built by a `default_factory`, so the reference showed `None` for every such list, top-level and nested, including ones whose shipped default is not empty. Render the factory's result when it is a plain collection and resolve nested settings models from their schema definition name so their fields are covered too; a factory such as `generate_uuid` yields a fresh value on every call and stays undocumented. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A shed request that is about to be replayed used to wait in silence, so a page under load looked hung until the replay landed. The retry driver now reports each replay it has decided on, just before waiting, and the transport turns that into one informational toast: "Infrahub is busy. Your request will be retried automatically in N seconds." A burst of shed requests shares the notice, which follows the latest wait. A sub-second replay gets no notice, since it is over before anyone could read it. The driver also tells the notice how each wait ended, and the notice counts the replays it has announced: it closes at once when the last pending one is abandoned by an abort, and shortly after the last one fires so a replay that is shed again can update it instead of flashing a new one. It never promises a replay that will not be sent. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The "Infrahub is busy" wording was applied only on the GraphQL surface, so once the retries were spent on a REST call the server's own "shedding load" message reached the user: through the raw fetch helper's error on the SSO callback page, and through the ten or so use-cases that rethrow the envelope's first message off the REST client's error. Reword the shed envelope once, at the two REST seams: a response middleware on the REST client, registered before the auth middleware so it also covers the 401 replay, and the raw fetch helper before it builds its error. A 429 from something in front of the API carries no marker and is left untouched, and GraphiQL keeps the server's words. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The loop read only the status and the Retry-After header of a 429 it was about to retry, then dropped the response with its body unread, holding the stream open until garbage collection: up to three per request, during the burst that means the server is saturated. Cancel the body before scheduling the replay; a response that is handed back keeps its body readable. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The retry loop releases the body of a 429 it discards, but `cancel()` on a stream that has already errored rejects with that error. Nothing observed the returned promise, so it surfaced as an unhandled rejection from inside the loop. Catch and drop it: there is nothing the loop can do about a body it is throwing away either way. Cover the three fallbacks in `withShedWording` that hand a marked shed back untouched — an unparseable body, a non-object body, a non-array `errors` — the paths where the server's "shedding load" wording reaches the user instead of the one the toast shows. Share one stub helper and teardown between the two `fetchUrl` test blocks, which set up `localStorage` and `fetch` identically. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ting a branch The retirement step added to branch deletion collected every candidate field of the branch and ran the retention predicate over the whole list in the outer transaction; only the closures were batched. The predicate's per-field-per-branch aggregation is eager, so a branch of 60k nodes with one agnostic attribute and ten open siblings exhausted Neo4j's transaction memory pool (716.8 MiB on a 1 GiB heap) and the delete failed. The query now seeds candidates from the branch's IS_PART_OF edges with two index seeks joined by UNION ALL, reads the retaining-branch windows once, and evaluates and closes inside CALL ... IN TRANSACTIONS, so a batch is bounded by its size times the branch count. The shared predicate gives up its branch read as UNRETAINED_AGNOSTIC_FIELD_EVALUATION, which each batch runs against the imported windows; its composed text is unchanged for the other call sites. Same graph, same heap: 240k edges closed in 4.5 s; 300k nodes with 30 siblings, 1.2M edges closed in 36.7 s. Fixes #10623 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#10657) * docs: always load the comment rule and drop the full-docstring mandate The code-doc-style rule was path-scoped, so it reached only sessions that opened a matching file through the Read, Edit or Write tools. Sessions that read with cat, and subagents working in another worktree, never saw it. Loaded at launch it applies everywhere. python.md and testing.md prescribed the opposite of the rule: a Google docstring with Args, Returns and Raises on every public function, and an inline docstring on every dataclass field. With both loaded, the guideline won and reviewers kept trimming the result. Both now ask for a one-line contract, with a section or field docstring only where the signature or the name does not already say it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(rules): exception types a function raises are contract, not references Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
The labs catalogue at docs.infrahub.app/labs now describes every lab in full: overview, what you'll learn, prerequisites, duration and level, all in a dialog on the card. It also covers labs this repository knows nothing about, including the AutoCon workshop labs and the labs OpsMill hosts for partners and the community. That left the five pages under learn/labs/ as a second, smaller index of the same labs, and readers arriving from the catalogue landed on them with no way back. Remove them and leave a single "Infrahub Labs" link to the catalogue in their place.
A TEXT index is only usable when the planner knows the looked-up value is a string, which it does not for a uuid read out of a map parameter, so the diff save looked its root up with a scan of every DiffRoot on each row. RANGE indexes seek whatever the value's type, and Node.uuid already uses one. Diff roots also gain indexes on their branch and tracking id, the two other properties they are looked up by; stored diffs are never deleted, so their number only grows. Migration 079 creates the RANGE indexes before dropping the TEXT ones, so no diff lookup runs unindexed in between, and its validation checks both. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Every node of a save batch was looked up three times as
MERGE/MATCH (root)-[:DIFF_HAS_NODE]->(node {uuid, db_id}). On a large diff
the planner expands every DIFF_HAS_NODE edge of the root and filters, so a
batch costs more the more nodes the root already holds and a 38k-node save
grew from under a second to 15 s per batch.
Create the missing nodes in a first pass and anchor every node lookup on
the DiffNode uuid index with the edge from the root checked as a predicate,
so a batch costs the same at any diff size. A write on the root takes its
lock for the transaction, so two saves of one diff that overlap still
create each node once, as the MERGE did. The hierarchy link query looks
its children and parents up the same way.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…at a time The field- and property-level calculation queries page their rows with SKIP/LIMIT, so every page re-ran the match over each edge changed on the branch and sorted it all before keeping one page: 312 pages of 1666 rows on a 38k-node branch cost 11.6 minutes, almost all of it repeated work. List the nodes with a changed field or property once, with the same time window the paths queries apply, then run the paths queries for one chunk of those uuids at a time through a new node_uuids scope on the calculation queries. Each match now covers a chunk of nodes instead of the whole branch; the row pagination inside a chunk stays as the memory bound. The base-branch run, already scoped by its field specifiers, is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The save batch held at most 1000 properties whatever the deployment's database.query_size_limit. Derive it as a fifth of that limit instead, with the previous 1000 kept as the floor: the default is unchanged, an operator who raises the limit moves the batches with it, and a limit tuned down for smaller reads does not shrink a save into one query per property. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The labels enricher asked NodeManager.get_many for every changed node and peer only to call get_display_label on the result: one Node object per id with every attribute and relationship manager, then a Jinja2 compile of the display label template per node because the stored display_label attribute was never part of the requested fields. On a 38k-node diff that kept the worker at 100% CPU for 42 s. NodeListGetDisplayLabelQuery reads the display label stored on a list of nodes as seen from a branch, and the enricher resolves every label through it, computing through node objects only the ids without a non-empty stored label (schema nodes, kinds without a template, nodes created before labels were stored). Same diff: 3.4 s wall, 0.8 s CPU, identical labels. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…bels A node deleted on a branch, or moved to another kind by a migration, holds several HAS_ATTRIBUTE edges to the same display_label attribute, and the stored label read ran its branch-resolution subqueries once per edge and returned the label once per edge. Collapse the node and attribute pairs before resolving them, as the node info query does. The new test runs a kind migration on a branch and checks that the read returns one row per node on both branches, with the label of the vertex that is active on each: the old vertex carries a deleted IS_PART_OF edge at the migration branch's level, so the per-vertex resolution already picks the right one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The docstring pointed at another function for the cost comparison and described only part of the omission rule. Describe the batching and the full rule instead: a node is left out when it is not active on the branch or when its stored label is empty or absent. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A kind without a display_label template still stores a display_label attribute, and an attribute with no value is written as the string sentinel NULL_VALUE. The stored label read returned that sentinel as a real label, so the diff of a node related to such a kind showed "NULL" where the node's representation belonged (integration diff/test_diff_update failed on the manufacturer peer of a deleted car). The read now treats the sentinel like an empty value, so those nodes keep going through get_display_label. Covered at query level (a template-less node is left out of the stored map) and at enricher level (its label is computed and equals the node's representation). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…10652) * docs(events): state the run-context append behavior of Prefect 3.8.6 The events worker attaches only as many run-context resources as fit under the maximum and drops the rest, so an event emitted on the maximum reaches the API without the run context its tags are carried in. The related-resource budget already reserves headroom for that append; this records the reason the reservation is needed. Also records the task manager index migrations the Prefect 3.8.6 upgrade applies on first start. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(changelog): fold the migration note into the existing upgrade fragment Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(changelog): describe the upgrade's effect rather than its mechanism Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
#10646) (#10671) * test: add failing test for #10646 * fix: return 404 when converting an already-converted node convert_object_type looked up the source node with the default raise_on_error=False, then dereferenced the possibly-None result with .get_kind(), raising AttributeError (HTTP 500) when the id had been consumed by a prior conversion. Pass raise_on_error=True on both source-node lookups so a missing id surfaces as NodeNotFoundError (404). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: add changelog fragment for #10646 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: pin the not-found message as GraphQL surfaces it and cover the HTTP envelope graphql-core's located_error reads the exception's `.message` attribute and only falls back to str() when it is absent, so a NodeNotFoundError reaches `result.errors` as its single-line message, not the multi-line __str__ the component test had copied. The fix already worked; the expectation was wrong. Corrected component test shown to bite: with the pre-fix mutation restored it fails with ["'NoneType' object has no attribute 'get_kind'"] against the pinned not-found line, and passes on the fixed code. The direct graphql() call never runs the catalogue error formatter, which is wired only into the HTTP app, so it cannot observe extensions.http_status. Add a functional test that posts the mutation twice over HTTP and pins the public envelope: status line 200, data {"ConvertObjectType": null}, the exact message, path ["ConvertObjectType"], and extensions {code: NODE_NOT_FOUND, http_status: 404, data: {node_kind: Node, identifier}}. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: opsmill-bug-pipeline[bot] <282019593+opsmill-bug-pipeline[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Saltaferis Dimitrios <d.saltaferis@gmail.com>
…mponent (INFP-586) Customers running a private PKI had to rebuild the Docker image to add their root CA, because git trusted only the system store and the credential helper and S3 storage had no CA setting at all, while cache, broker, database, HTTP, trace, syslog and LDAP each had their own. - New `tls.ca_bundle` section (`INFRAHUB_TLS_CA_BUNDLE`): a CA bundle that fills every component CA setting left unset. Precedence is component setting, then the global bundle, then the system store; a component with `tls_insecure` is left alone, and the trace exporter only inherits it when its connection is already encrypted, since a bundle would switch a plaintext gRPC exporter to TLS. - Every component CA setting accepts a file path or the PEM text itself. Text is validated at startup and written to `$TMPDIR/infrahub-tls/ca-bundle-<sha256>.pem`, so components that can only read a file — git, boto3, the Neo4j driver, redis-py — and every subprocess converge on the same file without coordination. - `*_TLS_INSECURE` outranks the CA bundle everywhere, so verification can be switched off for a temporary problem without dropping the bundle first. The clients that reach the Infrahub API used to force verification back on whenever a bundle was configured. - Git: `git.tls_ca_file` and `git.tls_insecure`, written to the global git config as `http.sslCAInfo` / `http.sslVerify` at task-worker startup and cleared when unset so a persisted gitconfig cannot keep a stale value. The git config helpers move to `infrahub.git.global_config`. - SDK clients to the Infrahub API — the injected worker client, the client the worker puts on InfrahubServices, and the git credential commands, which share one builder — all follow `http.tls_*` and the global bundle. - S3 storage: `storage.s3.tls_ca_file` (`INFRAHUB_STORAGE_TLS_CA_FILE`, alias `AWS_CA_BUNDLE`) is passed to boto3 as `verify`, and is rejected on a plaintext endpoint instead of being silently ignored. - An untrusted certificate reaches the `error-connection` status with the certificate hint for every wording git emits. The message depends on the TLS backend libcurl is built against and changes between curl releases, so the classifier matches the stable fragment of each family, including a certificate issued for another host. - Compose env blocks, configuration reference, a "Trust a private CA" guide, the git connect-repository page (no more custom image), and a dev knowledge page on outbound TLS. - Tests cover the precedence rules including insecure-plus-bundle, PEM-text materialization, the git config writes, the SDK client and S3 wiring, every TLS error wording, and an end-to-end clone over HTTPS from a server signed by a throwaway private CA. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The branch merge test opened the Tasks tab and required the loading skeleton to be visible before waiting for it to clear. That state only exists while the tasks query is in flight, so the assertion passes only when the query is slower than Playwright's first poll; when it resolves first the panel already shows "No task" and the expect times out after 30s. Seen on unrelated branches — a backend PR on 2026-08-14 and a frontend PR on 2026-09-18 — in 2 of 9 recorded branches_repo runs. Waiting for the skeleton to be absent and then for the empty state still covers the panel loading, and matches how the rest of the suite waits on loading indicators. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…10622) * fix(api): report one 422 error per schema write-contract violation POST /api/schema/load and /check reject an invalid schema with one request-validation error per violation, located on the offending field, carrying the value received there in `input` and the contract wording alone in `msg`. The SDK validator's error details supply the location, value and reason directly, so the response is built without parsing text and `infrahubctl` renders each field again. Closes #10601 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(api): assert schema load errors on rendered field paths The 422 detail of a schema load carries each error location as a list of segments. The assertions now compare the location rendered as a dotted field path, through a shared helper that reuses the SDK renderer, so an expectation reads as the address of the offending field and the server path is compared directly to the SDK's `field` where the two are meant to agree. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs: describe the 422 mapping dependency and the schema error test helper The write-contract section of the schema knowledge doc states that the 422 mapping relies on the SDK error detail exposing its location, value and reason, so the submodule pointer moves with any change to that shape. The testing knowledge doc lists the helper that renders a 422 detail list as dotted field paths. The deprecation guide returns to its released wording, which already described the field-level reporting its readers see. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test: render a 422 location without the SDK formatter The helper that flattens a 422 detail list renders the dotted field path from the location itself, so the backend consumes only the structured data the SDK validator produces and the assertions keep naming the offending field. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(deps): pin python_sdk to the structured schema error details The write-contract validator builds its 422 entries from the location, the received value and the reason carried by the SDK validation error detail. The pointer moves to the SDK head that exposes those three fields, so an invalid schema write reaches the request-validation response instead of failing while building it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(test): index a location that starts on a list element The assignment target is evaluated whatever the condition selects, so a location whose first element is an integer is rendered by appending a new bracketed segment rather than by extending a segment that does not exist yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(deps): move python_sdk to the released error detail contract The pointer follows the SDK branch that matches this one, now that the structured location, value and reason the 422 mapping reads are part of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: describe the rejection a schema write returns over HTTP The deprecation guide showed only the offline validator's rendering, so a caller of the load and check endpoints had no description of the response they receive. The guide now gives the request-validation body and what each of its three parts addresses, and the offline section names the same three on the validator's error details. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: explain why a rejection carries its reason twice The schema knowledge doc gives the constraint behind the duplicated reason on a request-validation entry, so a reader does not collapse the template and its placeholder into a runtime string the type checker refuses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: rewrap the offline error detail paragraph The sentence naming the structured fields left the rest of its paragraph on one long line; the paragraph now wraps at the width the rest of the guide uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: record the response shape change under Changed The load and check endpoints return a request-validation body whose shape a released version already published, so the entry names what a consumer reading the previous shape has to do and what is left untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
A neo4j session carries a single connection that cannot serve two coroutines at once, and the module-scoped db fixture hands the same session to every test in a module. The concurrent coordinator calls here shared it, so an unlucky interleaving wedged the connection: the coroutine that lost the race parks on the socket forever and every later test in the module dies on it. Build each racing request on a session of its own, as an API request and a flow each get one, and count the calculated diffs and stored-diff reads across the pair. One merger helper replaces the construction duplicated by the two merge tests, including its second driver, which was never closed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A session carries one connection and cannot serve two coroutines at once, and the module-scoped db fixture hands the same one to every test in a module, so racing two calls through it wedges the connection for the rest of the module. Write the rule down where tests get written, with the shape that keeps each racing call on its own session. Also drop the claim that the wipe is a bare MATCH (n) DETACH DELETE n: it deletes in batches of its own transactions unless the caller already holds one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…10689) Bumps the uv group with 1 update in the / directory: [anyio](https://github.com/agronholm/anyio). Bumps the uv group with 2 updates in the /python_testcontainers directory: [anyio](https://github.com/agronholm/anyio) and [pygments](https://github.com/pygments/pygments). Updates `anyio` from 4.13.0 to 4.14.2 - [Release notes](https://github.com/agronholm/anyio/releases) - [Commits](agronholm/anyio@4.13.0...4.14.2) Updates `anyio` from 4.11.0 to 4.14.2 - [Release notes](https://github.com/agronholm/anyio/releases) - [Commits](agronholm/anyio@4.13.0...4.14.2) Updates `pygments` from 2.19.2 to 2.20.0 - [Release notes](https://github.com/pygments/pygments/releases) - [Changelog](https://github.com/pygments/pygments/blob/master/CHANGES) - [Commits](pygments/pygments@2.19.2...2.20.0) --- updated-dependencies: - dependency-name: anyio dependency-version: 4.14.2 dependency-type: indirect dependency-group: uv - dependency-name: anyio dependency-version: 4.14.2 dependency-type: indirect dependency-group: uv - dependency-name: pygments dependency-version: 2.20.0 dependency-type: indirect dependency-group: uv ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…to-develop-20260920 # Conflicts: # backend/infrahub/workers/infrahub_async.py # frontend/app/src/entities/homepage/ui/getting-started.tsx # python_sdk
Each branch added a different module-level helper at the same spot in the async worker; both are called from the merged class body.
stable rewrote the card copy, develop replaced the literal gray class with the muted foreground token.
Neither branch's recorded SDK commit contains the other; the SDK's infrahub-develop tip is the first commit that contains both.
develop enforces BLE001, which stable did not; migrations that report a failure through their MigrationResult carry the same exemption.
github-actions
Bot
added
type/documentation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
40 issues found across 151 files
Confidence score: 3/5
backend/infrahub/services/heartbeat.pycan reuse a main-loop-bound cache client and close the shared service cache during heartbeat shutdown, disrupting other service operations — isolate the heartbeat client and its lifecycle.frontend/app/src/shared/api/rest/fetch.tsmay retry a one-time SSO callback exchange after a 429, whilefrontend/app/src/shared/api/rate-limit/retrying-fetch.tscan replay a consumedReadableStreambody and fail — restrict retries to safe callbacks and replayable request bodies.frontend/app/src/shared/api/rate-limit/retry-notice.tscan let an earlier grace timer dismiss the shared toast during a later replay’s grace period, hiding the current rate-limit state — use one cancellable or generation-checked timer.- Configuration and documentation changes need correction before relying on them:
backend/infrahub/storage.pysilently ignoresAWS_CA_BUNDLEfor HTTP S3,backend/infrahub/tls/bundle.pycrashes on an empty CA setting, anddocs/sidebars.tsremoves published Labs URLs; validate the incompatible settings, preserve unset behavior, and redirect or retain removed routes.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/app/src/shared/api/rate-limit/retry-notice.ts">
<violation number="1" location="frontend/app/src/shared/api/rate-limit/retry-notice.ts:67">
P2: When two replays settle within `NOTICE_GRACE_MS`, the first replay's timer can dismiss the shared toast before the later replay's grace period ends. Keep one cancellable or generation-checked grace timer so only the latest settlement controls dismissal.</violation>
</file>
<file name="docs/docs/deploy-manage/install-configure/production-deployment/private-ca.mdx">
<violation number="1" location="docs/docs/deploy-manage/install-configure/production-deployment/private-ca.mdx:220">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
When `INFRAHUB_GIT_GLOBAL_CONFIG_FILE` is set to a non-default path, this verification command reads `/opt/infrahub/.gitconfig` and can falsely report that Git lacks the CA configuration. Expand `INFRAHUB_GIT_GLOBAL_CONFIG_FILE` inside the task-worker container, falling back to `/opt/infrahub/.gitconfig` only when unset.</violation>
</file>
<file name="backend/infrahub/services/heartbeat.py">
<violation number="1" location="backend/infrahub/services/heartbeat.py:41">
P2: When `config.OVERRIDE.cache` is a Redis/NATS client created for the service, this factory reuses the main-loop-bound singleton on the heartbeat loop and then closes the shared service cache during shutdown. Always create a separate connection for the heartbeat thread; keep loop-agnostic test doubles in the injected `cache_factory`.</violation>
<violation number="2" location="backend/infrahub/services/heartbeat.py:44">
P3: The class and method docstrings mix durable behavior with implementation rationale, concrete symbols, and caller instructions. Keep source documentation focused on observable heartbeat, retry, and shutdown guarantees, and move the design rationale and usage guidance to the PR or architecture documentation.
(Based on your team's feedback about concise, contract-focused source documentation.)</violation>
</file>
<file name="dev/guidelines/backend/python.md">
<violation number="1" location="dev/guidelines/backend/python.md:185">
P3: The new "❌ Bad" example (`source_id: UUID` with docstring `"""UUID of the Source Node."""`, `peer_kind: str` with `"""Kind of the Peer Node."""`) is not hypothetical — it is the exact pattern still used in production by the same-named class `RelationshipPeerData` in `backend/infrahub/core/query/relationship.py:80-96`, where every field carries a restating docstring. This merge therefore lands a standard that directly contradicts current shipped code, and a developer grepping that class after reading this section will find it labeled "bad" with no migration in the merge. Either migrate `backend/infrahub/core/query/relationship.py` in a companion change or note that the codebase has not adopted the new rule yet.</violation>
<violation number="2" location="dev/guidelines/backend/python.md:296">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
This recommended example documents `BranchExistsError`, but that exception does not exist in the repository; branch creation currently raises `ValidationError` for duplicate names. Use the implemented exception or remove the fabricated contract from the example.</violation>
</file>
<file name="frontend/app/src/shared/api/rest/fetch.ts">
<violation number="1" location="frontend/app/src/shared/api/rest/fetch.ts:60">
P2: When the SSO callback receives a 429 after attempting the one-time code exchange, this line retries the GET up to three times, even without the admission marker. Bypass automatic retries for this callback or restrict replay to responses proven to come from the admission layer.</violation>
</file>
<file name="frontend/app/src/shared/api/rate-limit/retrying-fetch.ts">
<violation number="1" location="frontend/app/src/shared/api/rate-limit/retrying-fetch.ts:17">
P3: This docstring names internal auth/query-cache layering and explains implementation placement rather than the exported retry contract. Remove that paragraph and keep the documentation focused on observable 429 retry behavior.
(Based on your team's feedback about contract-focused source documentation.)</violation>
<violation number="2" location="frontend/app/src/shared/api/rate-limit/retrying-fetch.ts:33">
P2: When a URL plus init uses a `ReadableStream` body and the first attempt gets Infrahub's marked 429, the replay reuses the consumed stream and throws. Exclude non-replayable init bodies from `canReplay`, or buffer them before the first attempt.</violation>
</file>
<file name="backend/infrahub/storage.py">
<violation number="1" location="backend/infrahub/storage.py:52">
P2: When `AWS_S3_USE_SSL=False` and `AWS_CA_BUNDLE` are supplied directly, boto3 connects over HTTP and ignores `verify`, so the configured CA setting silently has no effect. Reject this incompatible combination in the storage constructor, not only in the settings model.</violation>
</file>
<file name="backend/infrahub/core/query/branch_agnostic_retirement.py">
<violation number="1" location="backend/infrahub/core/query/branch_agnostic_retirement.py:39">
P3: Custom agent: **Flag AI Slop and Fabricated Changes**
`UNION ALL` preserves duplicate rows, so nodes matching both existence predicates are not deduplicated here; the following `WITH` also has no `DISTINCT`. Update this comment to describe duplicate preservation, consistent with the class documentation that says such nodes are evaluated twice.</violation>
<violation number="2" location="backend/infrahub/core/query/branch_agnostic_retirement.py:69">
P2: The stated memory bound omits field and peer fan-out. The retention evaluation expands every candidate field to its linked peers before multiplying rows by `branch_windows`, so a batch can use substantially more memory than `batch_size × branch count`; update both the inline comment and docstring to include this fan-out or enforce a corresponding cap.
(Based on your team's feedback about row fan-out in batched reads.)</violation>
</file>
<file name="frontend/app/src/shared/api/rate-limit/shed-envelope.ts">
<violation number="1" location="frontend/app/src/shared/api/rate-limit/shed-envelope.ts:10">
P3: The new comments couple this utility to concrete middleware and callers and narrate implementation rationale. Replace them with concise observable contracts that do not name exception handlers, CORS middleware, callers, or UI toasts.</violation>
<violation number="2" location="frontend/app/src/shared/api/rate-limit/shed-envelope.ts:72">
P2: A rewritten shed response retains the original `Content-Length` even though `JSON.stringify` produces a different body, so callers can observe an incorrect byte length. Remove or recompute `Content-Length` before constructing the replacement response.</violation>
</file>
<file name="backend/infrahub/core/diff/calculator.py">
<violation number="1" location="backend/infrahub/core/diff/calculator.py:209">
P2: When a branch changes local-only nodes, this new chunking path collects their UUIDs and runs path queries that cannot return paths for them, adding empty database queries. Filter the changed-node queries to branch-aware nodes before collecting UUIDs so local-only changes do not create empty chunks.
(Based on your team's feedback about filtering branch-local diff nodes.) .</violation>
</file>
<file name="backend/infrahub/tls/context_builder.py">
<violation number="1" location="backend/infrahub/tls/context_builder.py:23">
P2: When a raw invalid path reaches `TlsContextRegistry.validate`, this call leaks `OSError` instead of the documented `ValueError` because `validate()` catches only `ssl.SSLError`. Normalize `OSError` to `ssl.SSLError` here or broaden the registry catch.</violation>
</file>
<file name="backend/infrahub/core/query/diff.py">
<violation number="1" location="backend/infrahub/core/query/diff.py:931">
P2: Local-only field edges enter this candidate list because the final node pattern has no branch-support filter, but `DiffFieldPathsQuery` rejects them; they create empty chunks and extra path queries. Restrict the matched field to `branch_support = $branch_aware`.
(Based on your team's feedback about filtering branch-local diff candidates.) .</violation>
<violation number="2" location="backend/infrahub/core/query/diff.py:950">
P2: Local-only property edges enter this candidate list because the intermediate field has no branch-support filter, but `DiffPropertyPathsQuery` rejects it; they create empty chunks and extra path queries. Restrict the matched field to `branch_support = $branch_aware`.
(Based on your team's feedback about filtering branch-local diff candidates.) .</violation>
</file>
<file name="backend/infrahub/events/limits.py">
<violation number="1" location="backend/infrahub/events/limits.py:32">
P2: The rewritten docstring asserts that Prefect's events worker trims run-context resources to the maximum ("attaching only as many as still fit"). The pinned Prefect 3.8.6 worker does the opposite: `EventsWorker.attach_related_resources_from_context` runs `event.related += await related_resources_from_run_context(...)`, an unbounded in-place append that skips client-side validation (`Event` has no `validate_assignment`). An event emitted on the maximum arrives above it, and the API's `enforce_maximum_related_resources` raises a `ValidationError` that closes the `/events/in` websocket. This matches the removed wording and `dev/knowledge/backend/events.md`, which still documents the websocket-closing behavior. Keep the accurate description so the headroom's purpose isn't misdocumented; the test docstring in `test_limits.py` repeats the same false claim.</violation>
</file>
<file name="docs/sidebars.ts">
<violation number="1" location="docs/sidebars.ts:74">
P2: Removing the Infrahub Labs category drops five published URLs (`/learn/labs/overview`, `/learn/labs/fundamentals-to-expert`, `/learn/labs/infrahub-introduction`, `/learn/labs/schema-deep-dive`, `/learn/labs/deploy-first-configuration`) that now return 404 with no redirect. docs/AGENTS.md hard rule: add a redirect entry in `docs/redirects-pending/` in the same PR for every deleted published URL. The docs tree confirms the pages are gone (`docs/docs/learn/` contains only `tutorials`, no `learn/labs`), and no redirect is recorded for them anywhere. Add a `redirects-pending` YAML entry mapping the old lab URLs to the labs catalogue.</violation>
</file>
<file name="backend/infrahub/services/scheduler.py">
<violation number="1" location="backend/infrahub/services/scheduler.py:34">
P3: This class docstring hard-codes implementation references and rationale instead of stating a durable scheduler contract. Remove the concrete references so the documentation remains correct when the heartbeat implementation moves or is renamed.</violation>
</file>
<file name="backend/infrahub/tls/bundle.py">
<violation number="1" location="backend/infrahub/tls/bundle.py:11">
P3: This module comment names concrete consumers and narrates implementation rationale instead of stating the durable contract. Make it implementation-independent and describe only that inline PEM bundles are materialized under `TMPDIR` for file-based consumers.
(Based on your team's feedback about concise contract-focused source documentation.) .</violation>
<violation number="2" location="backend/infrahub/tls/bundle.py:104">
P2: An empty-string CA setting now crashes the process at startup instead of being treated as unset. The new validators in config.py route every value that is not None through `_resolve_ca_bundle_setting`; for `""` `is_pem_text` is False, `Path("").is_file()` is False, and `resolve_ca_bundle` raises `must be the path to an existing file or PEM text, got ''`. Operators commonly blank an env var to "" (docker-compose/helm placeholders), and before this change `tls_ca_file=""` was simply falsy, so consumers using `if not ca_bundle` treated it as unset. Blanking a var now makes the whole server fail to boot with a confusing message.</violation>
</file>
<file name="backend/tests/component/core/migrations/graph/test_079_range_diff_indexes.py">
<violation number="1" location="backend/tests/component/core/migrations/graph/test_079_range_diff_indexes.py:13">
P2: When the component suite runs with db_type set to memgraph (the `db` fixture in backend/tests/conftest.py:176 selects the memgraph container for that config), this test fails: Migration079.execute/validate_migration return early for non-Neo4j databases (backend/infrahub/core/migrations/graph/m079_range_diff_indexes.py:46,60), so `before.errors` is empty and the assertion at the pre-state check fails, and the `SHOW INDEXES` / `CREATE RANGE INDEX` calls are Neo4j-specific syntax. Guard the test with a Neo4j-only skip, e.g. `if db.db_type != DatabaseType.NEO4J: pytest.skip(...)` at the top of the test.</violation>
</file>
<file name="backend/tests/unit/workflows/test_initialization.py">
<violation number="1" location="backend/tests/unit/workflows/test_initialization.py:16">
P3: When the checkout path contains a space, `CA_BUNDLE_QUOTED` (computed with `quote` → `%20`) no longer matches the actual connection string, built with `urlencode` (`quote_plus` → `+`), so these tests fail on any machine whose repo path has a space. Compute the expected value with the same quoting the implementation uses, e.g. `quote_plus(CA_BUNDLE, safe="")` (or `urlencode`) instead of `quote`.</violation>
</file>
<file name="backend/tests/component/services/adapters/nats/test_nats.py">
<violation number="1" location="backend/tests/component/services/adapters/nats/test_nats.py:119">
P3: If `await asyncio.wait_for(connected.wait(), timeout=30)` times out or `nats.connect` raises before setting the event, the `initialising` task is left hanging forever on `never_finishes`, holding an open NATS connection for the rest of the test session (and the connect exception goes unreported as "Task exception was never retrieved"). Wrap the body so the task is cancelled and awaited in a `finally`; a completed/cancelled task makes this cleanup a safe no-op on the happy path.</violation>
</file>
<file name="backend/infrahub/core/diff/enricher/labels.py">
<violation number="1" location="backend/infrahub/core/diff/enricher/labels.py:182">
P3: The second sentence of this comment explains the avoided approach and the performance rationale ("building a node object per id ... dominates the enrichment"). Per `.agents/rules/code-doc-style.md`, rationale about rejected approaches belongs in the PR description, not inline. Keep only the behavior/why line or drop the comment entirely.</violation>
</file>
<file name="backend/infrahub/core/query/node.py">
<violation number="1" location="backend/infrahub/core/query/node.py:1379">
P3: The class docstring and the `get_display_label_map` docstring name `Node.get_display_label`, which `.agents/rules/code-doc-style.md` forbids: "Do not name other classes, functions, methods, callers, or call sites in docstrings or comments." Reword to describe the contract without naming the method.</violation>
</file>
<file name="backend/infrahub/api/admission/middleware.py">
<violation number="1" location="backend/infrahub/api/admission/middleware.py:33">
P3: This comment mixes the marker’s client-visible contract with replay rationale and references the REST exception handler and CORS middleware. Keep it to the durable guarantee; move implementation rationale to the PR or architecture documentation.
(Based on your team's feedback about concise, contract-focused source documentation.)</violation>
</file>
<file name="backend/infrahub/server.py">
<violation number="1" location="backend/infrahub/server.py:243">
P3: This three-line inline comment duplicates the backpressure documentation and explains implementation rationale instead of stating the durable contract. Replace it with one sentence that CORS stays outside admission so shed responses retain CORS headers.
(Based on your team's feedback about code documentation contracts.)</violation>
</file>
<file name="backend/infrahub/services/component.py">
<violation number="1" location="backend/infrahub/services/component.py:43">
P3: These added docstrings couple the documentation to the concrete `WorkerHeartbeat` caller and narrate startup/thread rationale instead of stating a durable cache contract. Remove the caller references and keep each function's docstring concise and focused on its observable heartbeat behavior.
(Based on your team's feedback about keeping source docstrings contract-focused.)</violation>
</file>
<file name="backend/infrahub/git/base.py">
<violation number="1" location="backend/infrahub/git/base.py:48">
P3: The new TLS documentation duplicates curl/backend version history instead of stating the stable classification contract. Reduce both the module comment and function docstring to a concise statement that backend-specific TLS fragments are classified as connection errors.
(Based on your team's feedback about concise, contract-focused source documentation.)</violation>
</file>
<file name="backend/infrahub/core/diff/query/save.py">
<violation number="1" location="backend/infrahub/core/diff/query/save.py:115">
P2: The new `USING INDEX existing_node:DiffNode(uuid)` hint is attached to an `OPTIONAL MATCH`, and the subsequent `WHERE existing_node IS NULL`/`CREATE` only runs for rows whose existing-node lookup failed. This works only while the `WHERE (diff_root)-[:DIFF_HAS_NODE]->(existing_node)` predicate is parsed as part of the `OPTIONAL MATCH` clause (retaining the row with a null `existing_node`). It also depends on the RANGE index on `DiffNode(uuid)` actually existing before this query runs, which in turn depends on `Migration079` having executed — but that migration is a no-op on non-Neo4j databases. If either assumption is not met, every node in a save batch silently fails to be created (empty create pass) or the query errors on a non-satisfiable hint (when `dbms.cypher.hints_error=true`). Verify both: confirm the hint pattern/satisfiability on the target drivers, and confirm the save path cannot run before the migration has run on a given deployment.</violation>
</file>
<file name="backend/infrahub/git/global_config.py">
<violation number="1" location="backend/infrahub/git/global_config.py:56">
P3: Keep this public docstring focused on the durable contract: apply the configured Git TLS settings and remove stale managed keys when unset. Move option-parsing rationale and operator guidance to the relevant documentation.
(Based on your team's feedback about concise, contract-focused code documentation.) .</violation>
</file>
<file name="backend/infrahub/services/adapters/cache/nats.py">
<violation number="1" location="backend/infrahub/services/adapters/cache/nats.py:146">
P3: `close_connection()`'s docstring records an implementation choice and upstream-client rationale that can drift. Replace it with a concise contract describing that this method closes the cache-owned NATS connection.</violation>
</file>
<file name="docs/docs/release-notes/deprecation-guides/schema-load-write-contract.mdx">
<violation number="1" location="docs/docs/release-notes/deprecation-guides/schema-load-write-contract.mdx:194">
P3: The claim that reading the offline result's `.loc`, `.input` and `.reason` "reproduces the server's `detail` entry" is overstated for `.loc`. The server's 422 `detail` loc includes the request-envelope prefix `["body", "schemas", 0, ...]` (as shown in the example JSON above), while `validate_schema()` runs purely on the schema dict and can have no knowledge of the HTTP body envelope, so its `.loc` starts at the schema root (e.g. `["nodes", 0, "attributes", 0, "optionl"]`). The offline loc therefore matches the server's loc only after stripping the `body`/`schemas`/index prefix. Recommend rewording to say the three fields carry the same information as the server's detail entry, minus the request-envelope prefix.</violation>
</file>
<file name="dev/specs/ifc-2437-merge-failure-recovery/data-model.md">
<violation number="1" location="dev/specs/ifc-2437-merge-failure-recovery/data-model.md:131">
P3: The Active-worker set row still says "refreshed every 10 s", but the heartbeat now beats every 5 s: `HEARTBEAT_INTERVAL_SECONDS = 5.0` in backend/infrahub/services/heartbeat.py:17, and WorkerHeartbeat is instantiated at backend/infrahub/services/__init__.py:100 with that default. The 15 s TTL is correct (KVTTL.FIFTEEN in backend/infrahub/services/component.py), so the key is written three times per expiry, not once per 10 s — re-verify the inherited "every 10 s" claim against current code and update it.</violation>
</file>
<file name="dev/knowledge/backend/tls.md">
<violation number="1" location="dev/knowledge/backend/tls.md:88">
P3: The trap and the persistence bullet pin the gitconfig to `/opt/infrahub/.gitconfig`, but that path is operator-configurable: `Settings` exposes `git.global_config_file` (default `/opt/infrahub/.gitconfig`), which the worker exports as `GIT_CONFIG_GLOBAL` when the env var is not already set (backend/infrahub/config.py:830-835). An operator who sets `git.global_config_file` / `INFRAHUB_GIT_GLOBAL_CONFIG_FILE` gets a wrong inspection command and a misleading persistence trap. Read the file from `INFRAHUB_GIT_GLOBAL_CONFIG_FILE`, falling back to `/opt/infrahub/.gitconfig` when unset, and note in the persistence bullet that the path is configurable.</violation>
</file>
<file name="backend/tests/unit/services/test_scheduler.py">
<violation number="1" location="backend/tests/unit/services/test_scheduler.py:94">
P3: When run, this test may leave an unraised-value-error background task behind. `start_schedule()` only guards `heartbeat.start()` with `running`; the `branch_refresh` schedule registered by the `GIT_AGENT` constructor is still started as a task. Since `start_delay = random.randint(0, 5)` (backend/infrahub/services/scheduler.py:36), one run in six has `start_delay == 0`, so `run_schedule` skips its `if not self.running: return` check and reaches `if self.service is None: raise ValueError(...)`; this scheduler was built directly, so `service` is None and the exception is raised inside an unretrieved background task ("Task exception was never retrieved" noise). Clear `scheduler.schedules = []` before calling `start_schedule()`, as `test_scheduler_starts_and_stops_the_heartbeat_thread` already does.</violation>
</file>
<file name="frontend/app/src/shared/api/rate-limit/retrying-fetch.test.ts">
<violation number="1" location="frontend/app/src/shared/api/rate-limit/retrying-fetch.test.ts:145">
P3: The race probe in `tells the notice how long a replay will wait` cannot fail: `Promise.race([pending, Promise.resolve("still waiting")])` always settles to "still waiting" because `Promise.resolve("still waiting")` is already fulfilled, so the assertion passes regardless of whether the replay is still waiting. The test's only real assertion is the `notifyRetryScheduled` call with 2000. Assert that the replay has not fired yet instead (e.g., no second fetch call yet, or a promise whose resolution would fail the test).</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| dismissIfIdle(); | ||
| return; | ||
| } | ||
| setTimeout(dismissIfIdle, NOTICE_GRACE_MS); |
Contributor
There was a problem hiding this comment.
P2: When two replays settle within NOTICE_GRACE_MS, the first replay's timer can dismiss the shared toast before the later replay's grace period ends. Keep one cancellable or generation-checked grace timer so only the latest settlement controls dismissal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/app/src/shared/api/rate-limit/retry-notice.ts, line 67:
<comment>When two replays settle within `NOTICE_GRACE_MS`, the first replay's timer can dismiss the shared toast before the later replay's grace period ends. Keep one cancellable or generation-checked grace timer so only the latest settlement controls dismissal.</comment>
<file context>
@@ -0,0 +1,69 @@
+ dismissIfIdle();
+ return;
+ }
+ setTimeout(dismissIfIdle, NOTICE_GRACE_MS);
+ };
+}
</file context>
| Confirm that the task worker configured git with the bundle. Infrahub writes its git settings to the file named by `INFRAHUB_GIT_GLOBAL_CONFIG_FILE` (default `/opt/infrahub/.gitconfig`) and points git at it through `GIT_CONFIG_GLOBAL` in its own process only, so read that file explicitly: a `git config --global` call from an exec shell does not see the variable and reads a different file. | ||
|
|
||
| ```shell | ||
| docker compose exec task-worker git config --file /opt/infrahub/.gitconfig --get http.sslCAInfo |
Contributor
There was a problem hiding this comment.
P2: Custom agent: Flag AI Slop and Fabricated Changes
When INFRAHUB_GIT_GLOBAL_CONFIG_FILE is set to a non-default path, this verification command reads /opt/infrahub/.gitconfig and can falsely report that Git lacks the CA configuration. Expand INFRAHUB_GIT_GLOBAL_CONFIG_FILE inside the task-worker container, falling back to /opt/infrahub/.gitconfig only when unset.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/docs/deploy-manage/install-configure/production-deployment/private-ca.mdx, line 220:
<comment>When `INFRAHUB_GIT_GLOBAL_CONFIG_FILE` is set to a non-default path, this verification command reads `/opt/infrahub/.gitconfig` and can falsely report that Git lacks the CA configuration. Expand `INFRAHUB_GIT_GLOBAL_CONFIG_FILE` inside the task-worker container, falling back to `/opt/infrahub/.gitconfig` only when unset.</comment>
<file context>
@@ -0,0 +1,240 @@
+Confirm that the task worker configured git with the bundle. Infrahub writes its git settings to the file named by `INFRAHUB_GIT_GLOBAL_CONFIG_FILE` (default `/opt/infrahub/.gitconfig`) and points git at it through `GIT_CONFIG_GLOBAL` in its own process only, so read that file explicitly: a `git config --global` call from an exec shell does not see the variable and reads a different file.
+
+```shell
+docker compose exec task-worker git config --file /opt/infrahub/.gitconfig --get http.sslCAInfo
+```
+
</file context>
Suggested change
| docker compose exec task-worker git config --file /opt/infrahub/.gitconfig --get http.sslCAInfo | |
| docker compose exec task-worker sh -c 'git config --file "${INFRAHUB_GIT_GLOBAL_CONFIG_FILE:-/opt/infrahub/.gitconfig}" --get http.sslCAInfo' |
| Must be awaited on the heartbeat thread's own event loop: the asyncio cache clients bind to the | ||
| loop that creates them, so the main-loop connection cannot be reused from the thread. | ||
| """ | ||
| return config.OVERRIDE.cache or await InfrahubCache.new_from_driver(driver=config.SETTINGS.cache.driver) |
Contributor
There was a problem hiding this comment.
P2: When config.OVERRIDE.cache is a Redis/NATS client created for the service, this factory reuses the main-loop-bound singleton on the heartbeat loop and then closes the shared service cache during shutdown. Always create a separate connection for the heartbeat thread; keep loop-agnostic test doubles in the injected cache_factory.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/services/heartbeat.py, line 41:
<comment>When `config.OVERRIDE.cache` is a Redis/NATS client created for the service, this factory reuses the main-loop-bound singleton on the heartbeat loop and then closes the shared service cache during shutdown. Always create a separate connection for the heartbeat thread; keep loop-agnostic test doubles in the injected `cache_factory`.</comment>
<file context>
@@ -0,0 +1,175 @@
+ Must be awaited on the heartbeat thread's own event loop: the asyncio cache clients bind to the
+ loop that creates them, so the main-loop connection cannot be reused from the thread.
+ """
+ return config.OVERRIDE.cache or await InfrahubCache.new_from_driver(driver=config.SETTINGS.cache.driver)
+
+
</file context>
| ) -> Branch: | ||
| # ✅ Good - one line; the signature already documents the parameters | ||
| async def create_branch(db: InfrahubDatabase, name: str, description: str | None = None) -> Branch: | ||
| """Create a branch, raising BranchExistsError when the name is already taken.""" |
Contributor
There was a problem hiding this comment.
P2: Custom agent: Flag AI Slop and Fabricated Changes
This recommended example documents BranchExistsError, but that exception does not exist in the repository; branch creation currently raises ValidationError for duplicate names. Use the implemented exception or remove the fabricated contract from the example.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/guidelines/backend/python.md, line 296:
<comment>This recommended example documents `BranchExistsError`, but that exception does not exist in the repository; branch creation currently raises `ValidationError` for duplicate names. Use the implemented exception or remove the fabricated contract from the example.</comment>
<file context>
@@ -276,16 +283,31 @@ Name the validator after the invariant it enforces. Name the offending fields in
-) -> Branch:
+# ✅ Good - one line; the signature already documents the parameters
+async def create_branch(db: InfrahubDatabase, name: str, description: str | None = None) -> Branch:
+ """Create a branch, raising BranchExistsError when the name is already taken."""
+
+
</file context>
Suggested change
| """Create a branch, raising BranchExistsError when the name is already taken.""" | |
| """Create a branch, raising ValidationError when the name is already taken.""" |
| }; | ||
|
|
||
| const rawResponse = await fetch(url, newPayload); | ||
| const rawResponse = await withShedWording(await retryingFetch(url, newPayload)); |
Contributor
There was a problem hiding this comment.
P2: When the SSO callback receives a 429 after attempting the one-time code exchange, this line retries the GET up to three times, even without the admission marker. Bypass automatic retries for this callback or restrict replay to responses proven to come from the admission layer.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/app/src/shared/api/rest/fetch.ts, line 60:
<comment>When the SSO callback receives a 429 after attempting the one-time code exchange, this line retries the GET up to three times, even without the admission marker. Bypass automatic retries for this callback or restrict replay to responses proven to come from the admission layer.</comment>
<file context>
@@ -55,7 +57,7 @@ export const fetchUrl = async (url: string, payload?: RequestInit) => {
};
- const rawResponse = await fetch(url, newPayload);
+ const rawResponse = await withShedWording(await retryingFetch(url, newPayload));
if (!rawResponse.ok) {
</file context>
| rels: list[RelData] | None = None | ||
| """Both relationships pointing at this Relationship Node.""" | ||
|
|
||
| # ❌ Bad - the docstring restates the field name |
Contributor
There was a problem hiding this comment.
P3: The new "❌ Bad" example (source_id: UUID with docstring """UUID of the Source Node.""", peer_kind: str with """Kind of the Peer Node.""") is not hypothetical — it is the exact pattern still used in production by the same-named class RelationshipPeerData in backend/infrahub/core/query/relationship.py:80-96, where every field carries a restating docstring. This merge therefore lands a standard that directly contradicts current shipped code, and a developer grepping that class after reading this section will find it labeled "bad" with no migration in the merge. Either migrate backend/infrahub/core/query/relationship.py in a companion change or note that the codebase has not adopted the new rule yet.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/guidelines/backend/python.md, line 185:
<comment>The new "❌ Bad" example (`source_id: UUID` with docstring `"""UUID of the Source Node."""`, `peer_kind: str` with `"""Kind of the Peer Node."""`) is not hypothetical — it is the exact pattern still used in production by the same-named class `RelationshipPeerData` in `backend/infrahub/core/query/relationship.py:80-96`, where every field carries a restating docstring. This merge therefore lands a standard that directly contradicts current shipped code, and a developer grepping that class after reading this section will find it labeled "bad" with no migration in the merge. Either migrate `backend/infrahub/core/query/relationship.py` in a companion change or note that the codebase has not adopted the new rule yet.</comment>
<file context>
@@ -167,24 +167,31 @@ class NodeDiffBuilder:
+ rels: list[RelData] | None = None
+ """Both relationships pointing at this Relationship Node."""
+# ❌ Bad - the docstring restates the field name
+@dataclass(frozen=True)
+class RelationshipPeerData:
</file context>
| - **`--global` lies in an exec shell.** The worker selects `/opt/infrahub/.gitconfig` by exporting | ||
| `GIT_CONFIG_GLOBAL` in its own process; `docker compose exec task-worker git config --global ...` does | ||
| not inherit it and reads `$HOME/.gitconfig`, which only holds what the Dockerfile baked in. Inspect | ||
| the file directly: `git config --file /opt/infrahub/.gitconfig --get http.sslCAInfo`. |
Contributor
There was a problem hiding this comment.
P3: The trap and the persistence bullet pin the gitconfig to /opt/infrahub/.gitconfig, but that path is operator-configurable: Settings exposes git.global_config_file (default /opt/infrahub/.gitconfig), which the worker exports as GIT_CONFIG_GLOBAL when the env var is not already set (backend/infrahub/config.py:830-835). An operator who sets git.global_config_file / INFRAHUB_GIT_GLOBAL_CONFIG_FILE gets a wrong inspection command and a misleading persistence trap. Read the file from INFRAHUB_GIT_GLOBAL_CONFIG_FILE, falling back to /opt/infrahub/.gitconfig when unset, and note in the persistence bullet that the path is configurable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/knowledge/backend/tls.md, line 88:
<comment>The trap and the persistence bullet pin the gitconfig to `/opt/infrahub/.gitconfig`, but that path is operator-configurable: `Settings` exposes `git.global_config_file` (default `/opt/infrahub/.gitconfig`), which the worker exports as `GIT_CONFIG_GLOBAL` when the env var is not already set (backend/infrahub/config.py:830-835). An operator who sets `git.global_config_file` / `INFRAHUB_GIT_GLOBAL_CONFIG_FILE` gets a wrong inspection command and a misleading persistence trap. Read the file from `INFRAHUB_GIT_GLOBAL_CONFIG_FILE`, falling back to `/opt/infrahub/.gitconfig` when unset, and note in the persistence bullet that the path is configurable.</comment>
<file context>
@@ -0,0 +1,114 @@
+- **`--global` lies in an exec shell.** The worker selects `/opt/infrahub/.gitconfig` by exporting
+ `GIT_CONFIG_GLOBAL` in its own process; `docker compose exec task-worker git config --global ...` does
+ not inherit it and reads `$HOME/.gitconfig`, which only holds what the Dockerfile baked in. Inspect
+ the file directly: `git config --file /opt/infrahub/.gitconfig --get http.sslCAInfo`.
+- **The TLS failure wording depends on the git build.** git's HTTPS helper reports an untrusted
+ certificate with the wording of the TLS backend libcurl is linked against, and curl rewords those
</file context>
Suggested change
| the file directly: `git config --file /opt/infrahub/.gitconfig --get http.sslCAInfo`. | |
| the file directly: `git config --file ${INFRAHUB_GIT_GLOBAL_CONFIG_FILE:-/opt/infrahub/.gitconfig} --get http.sslCAInfo`; the path is `git.global_config_file` (`INFRAHUB_GIT_GLOBAL_CONFIG_FILE`), defaulting to `/opt/infrahub/.gitconfig`. |
Comment on lines
+11
to
+12
| # CA bundles supplied inline as PEM text are written here so that git, boto3, the Neo4j driver and | ||
| # redis-py, which only accept a file, can read them. Honours TMPDIR like every other temporary file. |
Contributor
There was a problem hiding this comment.
P3: This module comment names concrete consumers and narrates implementation rationale instead of stating the durable contract. Make it implementation-independent and describe only that inline PEM bundles are materialized under TMPDIR for file-based consumers.
(Based on your team's feedback about concise contract-focused source documentation.) .
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/tls/bundle.py, line 11:
<comment>This module comment names concrete consumers and narrates implementation rationale instead of stating the durable contract. Make it implementation-independent and describe only that inline PEM bundles are materialized under `TMPDIR` for file-based consumers.
(Based on your team's feedback about concise contract-focused source documentation.) .</comment>
<file context>
@@ -0,0 +1,110 @@
+
+PEM_MARKER = "-----BEGIN "
+
+# CA bundles supplied inline as PEM text are written here so that git, boto3, the Neo4j driver and
+# redis-py, which only accept a file, can read them. Honours TMPDIR like every other temporary file.
+MATERIALIZED_BUNDLE_DIRECTORY = Path(tempfile.gettempdir()) / "infrahub-tls"
</file context>
Suggested change
| # CA bundles supplied inline as PEM text are written here so that git, boto3, the Neo4j driver and | |
| # redis-py, which only accept a file, can read them. Honours TMPDIR like every other temporary file. | |
| # Inline PEM bundles are materialized under TMPDIR for consumers that require a file. |
| scheduler.schedules = [] | ||
| scheduler.running = True | ||
|
|
||
| await scheduler.start_schedule() |
Contributor
There was a problem hiding this comment.
P3: When run, this test may leave an unraised-value-error background task behind. start_schedule() only guards heartbeat.start() with running; the branch_refresh schedule registered by the GIT_AGENT constructor is still started as a task. Since start_delay = random.randint(0, 5) (backend/infrahub/services/scheduler.py:36), one run in six has start_delay == 0, so run_schedule skips its if not self.running: return check and reaches if self.service is None: raise ValueError(...); this scheduler was built directly, so service is None and the exception is raised inside an unretrieved background task ("Task exception was never retrieved" noise). Clear scheduler.schedules = [] before calling start_schedule(), as test_scheduler_starts_and_stops_the_heartbeat_thread already does.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/tests/unit/services/test_scheduler.py, line 94:
<comment>When run, this test may leave an unraised-value-error background task behind. `start_schedule()` only guards `heartbeat.start()` with `running`; the `branch_refresh` schedule registered by the `GIT_AGENT` constructor is still started as a task. Since `start_delay = random.randint(0, 5)` (backend/infrahub/services/scheduler.py:36), one run in six has `start_delay == 0`, so `run_schedule` skips its `if not self.running: return` check and reaches `if self.service is None: raise ValueError(...)`; this scheduler was built directly, so `service` is None and the exception is raised inside an unretrieved background task ("Task exception was never retrieved" noise). Clear `scheduler.schedules = []` before calling `start_schedule()`, as `test_scheduler_starts_and_stops_the_heartbeat_thread` already does.</comment>
<file context>
@@ -50,3 +57,61 @@ async def test_scheduler_task_with_error(fake_log: FakeLogger) -> None:
+ scheduler.schedules = []
+ scheduler.running = True
+
+ await scheduler.start_schedule()
+ try:
+ deadline = time.monotonic() + 2
</file context>
| // THEN | ||
| expect(notifyRetryScheduled).toHaveBeenCalledWith(2000); | ||
| // The race settles with the probe only while the replay is still waiting. | ||
| await expect(Promise.race([pending, Promise.resolve("still waiting")])).resolves.toBe( |
Contributor
There was a problem hiding this comment.
P3: The race probe in tells the notice how long a replay will wait cannot fail: Promise.race([pending, Promise.resolve("still waiting")]) always settles to "still waiting" because Promise.resolve("still waiting") is already fulfilled, so the assertion passes regardless of whether the replay is still waiting. The test's only real assertion is the notifyRetryScheduled call with 2000. Assert that the replay has not fired yet instead (e.g., no second fetch call yet, or a promise whose resolution would fail the test).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/app/src/shared/api/rate-limit/retrying-fetch.test.ts, line 145:
<comment>The race probe in `tells the notice how long a replay will wait` cannot fail: `Promise.race([pending, Promise.resolve("still waiting")])` always settles to "still waiting" because `Promise.resolve("still waiting")` is already fulfilled, so the assertion passes regardless of whether the replay is still waiting. The test's only real assertion is the `notifyRetryScheduled` call with 2000. Assert that the replay has not fired yet instead (e.g., no second fetch call yet, or a promise whose resolution would fail the test).</comment>
<file context>
@@ -0,0 +1,212 @@
+ // THEN
+ expect(notifyRetryScheduled).toHaveBeenCalledWith(2000);
+ // The race settles with the probe only while the replay is still waiting.
+ await expect(Promise.race([pending, Promise.resolve("still waiting")])).resolves.toBe(
+ "still waiting"
+ );
</file context>
This branch has not been deployed
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
4 participants
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.
Supersedes #10659, which was blocked on merge conflicts.
Summary
Merges
stableintodevelop.Conflicts resolved
backend/infrahub/workers/infrahub_async.py— each side added a different module-level helper at the same spot:inject_service_parameterondevelop,build_worker_client_configonstable. Both are called from the merged class body, so both definitions are kept (107c0c75c).frontend/app/src/entities/homepage/ui/getting-started.tsx—stablerewrote the Marketplace card copy (feat(frontend): point homepage Schema Library card to the Marketplace #10634),developreplaced the literaltext-gray-500class with thetext-foreground-mutedtoken (refactor(frontend): tokenize primary and muted text colors #10247). Keptstable's copy ondevelop's token (111cec830).python_sdk— neither branch's recorded SDK commit contains the other.68f742b8is the only commit on the SDK'sinfrahub-developbranch that contains both, and is its current tip, which is the branchdeveloptracks (9c58c9f0d).Conflicts raised for review
backend/infrahub/core/migrations/graph/m079_range_diff_indexes.py— silent conflict, no markers.developdroppedBLE001from the ruff ignore list;stableadded this migration, which catchesExceptionto report the failure through itsMigrationResult. Applied the# noqa: BLE001that every other migration in that directory already carries (79eedfeb1).Validation
Full pre-CI gate run locally, all green: ruff (format + check, repo-wide),
ty check ., mypy (1708 files), yamllint, huge-runner gate, bothuv.lockfiles, Biome CI, knip, betterer, frontend unit tests (1561) and@infrahub/graph(23), backend unit tests (2802), and every generated-file validation — backend, OpenAPI types, error bindings, GraphQL/JSON schema and generated docs all regenerate to no diff.Not run: backend integration/functional/e2e suites (need a running stack).
backend-testcontainers-unitis not runnable on macOS — collection dies inpsutil.cpu_freq(), unrelated to this merge.