Repository navigation
fix(tools): correct diagram, DataHub export, and AWS contracts - #331
Merged
Merged
Conversation
adamgajzlerowicz
approved these changes
Oct 6, 2026
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
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.
Diagram selectors were stripped by the API, component loading did not honor its defaults, and generated DataHub export discarded the cursor response header. This change sends the correct diagram DTO, verifies requested IDs through scoped discovery, limits component reads to five distinct layers, and returns an export-only
{data, pageToken}envelope.skip_emptyfilters layers while retaining diagrams.AWS supported-feature lookup validates 12-digit account IDs. Dataset updates expose displayName/logoName, rendering lookup is marked as a mutation with the directory-required confirmation annotation, and descriptions reflect API contracts. Cost trends use percentage units; the upstream cost-total calculation is unchanged. Other generated response shapes are unchanged.
Actual built stdio MCP tools were compared against
https://api-dev.doit.comon 2026-10-06. Isolated snapshots used baseline15b6b1c, featured1847b7, and pre-review456507afor review regressions. No fixture results are counted as live evidence.Cleanup: no remote resource was created or successfully updated; no remaining resource IDs. Negative PATCH targets were independently 404 before and after. A nonexistent-name DELETE was 404, but current deletion timeout/202-completion limitations prevented establishing reliable cleanup for a newly created dataset, so no dataset/event was created. A synthetic unmatched find lookup reached the API (there is no server-side dry-run gate); independent search found zero matches, and the backend exits before filter/image creation for that case. No matched rendering, ingestion, workflow execution, deployment, publication, or PR merge was performed. Temporary validation directories were removed.
Remaining gaps: positive dataset set/clear, event ingestion/atomicity/constraints, successful AWS/standalone lookup and bucket semantics, non-null trend units, rendering artifacts, relationship truncation, and hosted OAuth. Built metadata, fixture tests, and empty/null samples do not prove those behaviors.
Main advanced during validation, so the branch was synchronized with
e5845c4to resolve a test expectation conflict and integrate upstream API error handling. Both latest main and the synchronized feature were built in fresh isolated directories. Actual dev MCP calls reproduced the diagram-selector and CSV/JSONL cursor defects again and verified the fixes, component guard/defaults, selector errors, empty exports, API 400/401/404 errors, and local AWS ID rejection. Rendering lookup retains mutation retry advice; simulated 503 coverage is a repository test, not live rendering evidence. No dependency pins changed relative to main.Repository checks are separate: Node 20.20.2 and 22.23.2 passed 1,213 unit tests, 289 integration tests, and builds, retaining all 106 schema-parity cases.
yarn check:dev,yarn check:ci, and diff checks passed. CI was green on the previously validated692e4fbhead. All temporary validation directories were independently confirmed removed.Current branch integrates main
0d2f856in1142b64, resolving the generated override conflict by retaining both the DataHub export response cursor and the explicit read-only POST component lookup. The combined branch passed 1,238 unit tests, 293 integration tests,yarn check:dev, build, and whitespace checks. All four required CI checks pass on1142b64. Existing real dev evidence above remains tied to its recorded revisions; this merge resolution did not make new live API calls.Directory annotation follow-up in
fa629af:find_cloud_diagramsretainsreadOnlyHint: falseand now setsdestructiveHint: true, because its persisted sheet filter/render request modifies data. This follows Claude directory requirements. Removed the mutation exception from the server listing test; all tools must expose the applicable true hint. Type/lint checks, build, and 245 focused diagram/server/schema-parity tests pass. This metadata correction adds no new live API claim.