Skip to content

(SR-91) Fix Linear API error when viewing issue relationships - AI Generated#84

Open
dane-h wants to merge 8 commits into
DeskproApps:mainfrom
dane-h:fix/sr-91-linear-app-errors-linking-stories-claude
Open

(SR-91) Fix Linear API error when viewing issue relationships - AI Generated#84
dane-h wants to merge 8 commits into
DeskproApps:mainfrom
dane-h:fix/sr-91-linear-app-errors-linking-stories-claude

Conversation

@dane-h

@dane-h dane-h commented Jun 11, 2026

Copy link
Copy Markdown

🤖 AI GENERATED

PR Checklist

This PR is ready for review and meets the requirements set out
in Definition of Done

AI Contribution Level

  • 1. None: No meaningful AI assistance.
  • 2. Assisted: AI provided boilerplate/autocomplete; logic was human-authored.
  • 3. Co-authored: AI generated meaningful sections I reviewed and integrated.
  • 4. Generated: Majority of novel code was AI-generated; I validated and tested.

DOD

  • I have met the Acceptance Criteria
  • I have added Tests
  • I have added Documentation, where required
  • I have reviewed and tested for Performance
  • Frontend - Accessibility standards have been met
  • Backend - Migrations have been added

Changes / Test Instructions

Root cause: getIssuesService(client, {}) was called with an empty filter object. The gql({}) utility serialises this as {"variables": {}}, which causes the Linear API to reject the issues query — it requires a non-empty filter. The caught error was then displayed as a red bubble on every issue in the relationships panel.

Fix applied: Removed the broken getIssuesService call from useIssueRelationships. The hook now returns originalRelationships (the direct relations already fetched with the issue) without attempting reverse-relation discovery. Note: the IssueFilter GraphQL type has no relatedIssue.id comparator, so a server-side filter is not possible with this approach; proper reverse-relation discovery would require using the inverseRelations field on the Issue type — that is a future improvement.

Follow-up improvements (post code review):

  • Wrapped the return value in useMemo so the object reference is stable across renders, preventing spurious re-renders in consumers that use reference equality
  • Added a default value (= []) for originalRelationships to guard against undefined being passed before data has loaded
  • Added a TODO comment in the hook pointing to Issue.inverseRelations as the correct path for future reverse-relation discovery

Test Coverage

New tests added (src/hooks/__tests__/useIssueRelationships.test.ts):

  • Asserts that getIssuesService is never called, relationships equals originalRelationships, and error is null
  • Asserts that an empty-array input returns [] with no error
  • Asserts that undefined input falls back to [] via the default parameter
  • Asserts that changing issueID has no effect on the returned relationships

Existing coverage:
None — no prior tests existed for this hook.

Manual Testing

  • Open a Deskpro ticket with a linked Linear issue
  • Navigate to the issue detail view
  • Confirm the Relationships section renders without red error bubbles
  • Confirm direct relationships (if any) are still displayed

Summary by Sourcery

Simplify issue relationship handling to avoid invalid Linear API calls and ensure stable, error-free relationship data.

Bug Fixes:

  • Remove the Linear issues service call from useIssueRelationships to prevent invalid empty-filter requests that caused API errors in the relationships panel.

Enhancements:

  • Return original issue relationships via a memoized value with a default empty array to provide stable references and handle undefined inputs gracefully.

Build:

  • Bump the Linear Deskpro app manifest version from 1.0.24 to 1.0.25.

Tests:

  • Add unit tests for useIssueRelationships to assert it no longer calls getIssuesService, correctly handles empty and undefined inputs, and is unaffected by issue ID changes.

dane-h and others added 2 commits June 11, 2026 10:14
getIssuesService was called with an empty filter object, causing the Linear
API to reject the request. This error was then displayed on every issue in
the relationships panel. Reverse-relationship discovery via this approach
is not supported by the API (IssueFilter has no relatedIssue.id filter);
removed the broken call and return direct relations only.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Reviewer's Guide

Simplifies the useIssueRelationships hook to return only the already-fetched relationships, eliminating the problematic Linear issues service call, and adds regression tests plus a manifest version bump.

Sequence diagram for simplified useIssueRelationships hook

sequenceDiagram
    participant Component
    participant useIssueRelationships

    Component->>useIssueRelationships: useIssueRelationships(originalRelationships, issueID)
    useIssueRelationships-->>Component: { relationships: originalRelationships, error: null }
Loading

File-Level Changes

Change Details Files
Simplified useIssueRelationships to stop calling Linear and just expose existing relationships with a stable reference.
  • Replaced stateful implementation using useState, useDeskproAppClient, and useInitialisedDeskproAppClient with a pure hook based on useMemo.
  • Removed getIssuesService usage and any reverse-relationship discovery logic from the hook to avoid empty-filter Linear API calls.
  • Ensured the hook always returns an object with relationships and error: null, with relationships defaulting to the input array.
src/hooks/useIssueRelationships.ts
Hardened the hook’s API and behavior around input relationships and issue ID changes.
  • Added a default parameter value of [] for originalRelationships to tolerate undefined inputs before data load.
  • Ignored issueID in the implementation while keeping it as a parameter for API compatibility, ensuring the hook result does not change when the ID changes.
  • Documented a TODO pointing to Issue.inverseRelations as the correct future approach for reverse-relation discovery.
src/hooks/useIssueRelationships.ts
Added regression tests to lock in the new behavior and prevent reintroduction of the Linear issues service call.
  • Created tests that assert relationships equals originalRelationships and error is null, and that getIssuesService is never called.
  • Added tests for empty-array and undefined originalRelationships inputs to ensure they produce an empty relationships array without errors.
  • Verified that changing the issue ID prop does not affect the returned relationships or error.
src/hooks/__tests__/useIssueRelationships.test.ts
Updated the Linear app manifest version to reflect the bugfix release.
  • Bumped the manifest version from 1.0.24 to 1.0.25.
manifest.json

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Hook changes (useIssueRelationships.ts):
- Wrap return value in useMemo to stabilise object reference across renders,
  preventing spurious re-renders in consumers that use reference equality
- Add default value (= []) for originalRelationships to guard against
  undefined being passed before data has loaded
- Add TODO comment pointing to Issue.inverseRelations as the correct
  approach for future reverse-relation discovery

Test changes (useIssueRelationships.test.ts):
- Add comment on the getIssuesService mock clarifying it is a regression
  guard, not dead code
- Add test: empty-array input returns [] with no error
- Add test: undefined input falls back to [] via the default parameter
- Add test: changing issueID has no effect on the returned relationships

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@madrussa madrussa requested a review from djgadd June 11, 2026 13:19
@dane-h dane-h marked this pull request as ready for review June 11, 2026 13:37
@dane-h dane-h requested a review from a team as a code owner June 11, 2026 13:37

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 2 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="src/hooks/__tests__/useIssueRelationships.test.ts" line_range="28-36" />
<code_context>
+    jest.clearAllMocks();
+  });
+
+  test("returns originalRelationships without calling getIssuesService", () => {
+    const { result } = renderHook(() =>
+      useIssueRelationships(mockRelations, "issue-1")
+    );
+
+    expect(result.current.relationships).toEqual(mockRelations);
+    expect(result.current.error).toBeNull();
+    expect(mockGetIssuesService).not.toHaveBeenCalled();
+  });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen regression guard by asserting reference equality for relationships

Also assert that `result.current.relationships` is the same reference as `mockRelations` (e.g. `expect(result.current.relationships).toBe(mockRelations)`) to explicitly verify the hook preserves referential stability rather than cloning the array.

```suggestion
  test("returns originalRelationships without calling getIssuesService", () => {
    const { result } = renderHook(() =>
      useIssueRelationships(mockRelations, "issue-1")
    );

    expect(result.current.relationships).toEqual(mockRelations);
    expect(result.current.relationships).toBe(mockRelations);
    expect(result.current.error).toBeNull();
    expect(mockGetIssuesService).not.toHaveBeenCalled();
  });
```
</issue_to_address>

### Comment 2
<location path="src/hooks/__tests__/useIssueRelationships.test.ts" line_range="56-66" />
<code_context>
+    expect(result.current.error).toBeNull();
+  });
+
+  test("result is unchanged when issueID changes", () => {
+    const { result, rerender } = renderHook(
+      ({ id }) => useIssueRelationships(mockRelations, id),
+      { initialProps: { id: "issue-1" } }
+    );
+
+    rerender({ id: "issue-99" });
+
+    expect(result.current.relationships).toEqual(mockRelations);
+    expect(result.current.error).toBeNull();
+  });
+});
</code_context>
<issue_to_address>
**suggestion (testing):** Explicitly assert memoization across rerenders with unchanged relationships

To fully exercise the memoization behavior, consider storing `const firstResult = result.current` before `rerender` and then asserting `expect(result.current).toBe(firstResult)` afterward. This will verify that the hook returns the same object instance when only `issueID` changes, consistent with the `useMemo` dependencies.

```suggestion
  test("result is unchanged when issueID changes", () => {
    const { result, rerender } = renderHook(
      ({ id }) => useIssueRelationships(mockRelations, id),
      { initialProps: { id: "issue-1" } }
    );

    const firstResult = result.current;

    rerender({ id: "issue-99" });

    expect(result.current).toBe(firstResult);
    expect(result.current.relationships).toEqual(mockRelations);
    expect(result.current.error).toBeNull();
  });
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/hooks/__tests__/useIssueRelationships.test.ts Outdated
Comment thread src/hooks/__tests__/useIssueRelationships.test.ts Outdated
@madrussa

Copy link
Copy Markdown

Fixes SR-91

- Add .toBe(mockRelations) to assert referential identity of the returned
  relationships array — confirms the hook does not clone the input
- Capture firstResult before rerender and assert .toBe(firstResult) when
  issueID changes — confirms useMemo returns the same object instance when
  only the ignored parameter changes

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@djgadd djgadd left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking cos the root cause is wrong, gimme a shout next week and we can see if we can get some better logs out of support to see what's going on

Comment thread manifest.json
Comment thread src/hooks/useIssueRelationships.ts Outdated
Comment thread src/hooks/__tests__/useIssueRelationships.test.ts Outdated
dane-h and others added 2 commits June 23, 2026 14:05
The original hook fetched all issues to find relations pointing TO the
current issue, but this was expensive and error-prone. This replaces it
by querying `inverseRelations` on the issue itself — the correct Linear
API field for bidirectional relationship data.

- Add `inverseRelations` to `getIssueService` query with full issue info
- Add `inverseRelations: Relation[]` to the `Issue` type
- Rewrite `useIssueRelationships` to merge forward relations with
  type-mapped inverse relations (blocks→blocked, duplicate→duplicated)
- Thread `inverseRelations` through Relationships and IssueItem callers
- Replace regression-guard test with behavioural coverage of the merge
  and type-mapping logic

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixes the regex so it only matches literal dots in api.linear.app rather
than any character.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@dane-h

dane-h commented Jun 23, 2026

Copy link
Copy Markdown
Author

I'm not sure how we're going to be able to test this. Perhaps need to get a test trial of Linear going. Unless we can understand the fix directly through the code. This was an issue previously as well (when the fix was removing a feature)

@dane-h dane-h requested a review from djgadd June 23, 2026 16:43

@djgadd djgadd left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks better, claude caught a few things. I'll try catch you before the end of the day to share some tips

Comment thread src/services/linear/getIssueService.ts
Comment thread src/hooks/useIssueRelationships.ts
Comment thread src/hooks/useIssueRelationships.ts
Addresses reviewer findings on the inverse-relations work:

- getIssuesService (plural) now selects `id` on relations.nodes so the
  dedup key uses the real relation id instead of falling back to
  relatedIssue.id (which silently dropped distinct relations sharing a
  related issue), and adds the inverseRelations block so list/search
  contexts (Home, LinkIssues) surface inverse relations too — previously
  IssueItem always received an empty inverseRelations array.
- Model the 'similar' IssueRelationType (verified present in the live
  Linear schema): add it to RELATIONSHIP_LABELS and RelationshipType so
  it renders a label instead of "undefined", and to the hook's
  INVERSE_TYPE map (similar is symmetric, maps to itself).
- Add a test covering the symmetric 'similar' inverse mapping.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dane-h

dane-h commented Jul 1, 2026

Copy link
Copy Markdown
Author

Review summary

TL;DR: All 4 open review comments were valid; each was verified against the code and the live Linear schema before acting. Fixes are pushed in 35bd1f4 (typecheck clean, 40 test suites green). The PR is clear to proceed.


I adversarially reviewed the inverseRelations work for correctness and completeness, then re-checked each finding against the actual code and the upstream Linear API. All four outstanding comments held up:

Finding Verdict Fix
Plural getIssuesService omits id on relations.nodes, so the hook’s dedup key falls back to relatedIssue.id and drops distinct relations ✅ Valid Added id to the plural query
inverseRelations missing from plural getIssuesServiceIssueItem always got [] in Home/LinkIssues ✅ Valid Added the inverseRelations block to the plural query
similar relation type not modelled ✅ Valid — confirmed IssueRelationType = { blocks, duplicate, related, similar } in the live schema (vendored copy is stale) Added to INVERSE_TYPE (symmetric → maps to itself) + test
similar missing from RELATIONSHIP_LABELS ✅ Valid Added similar: "Similar to" to labels and the RelationshipType union

Code changed: yes — 35bd1f4 touches getIssuesService.ts, useIssueRelationships.ts, types/relationships.ts, and adds a similar-mapping test.

Verification: tsc --noEmit clean; full suite 40/40 suites, all passing. Each inline reply above was re-checked against the pushed code and matches.

No further defects found beyond the reviewed comments. One open decision left inline: similar was deliberately not added to RELATIONSHIP_OPTIONS (the manual “Add relationship” dropdown), since Linear generates it automatically — flag if you’d prefer it user-selectable.

Comment thread pnpm-workspace.yaml Outdated
The file used an invalid `allowBuilds:` key (pnpm's real field is
`onlyBuiltDependencies`) filled with literal "set this to true or false"
placeholders, so pnpm ignored it entirely and still warned about all five
packages.

Verified empirically with pnpm 10.18.3 (matching CI's devcontainer
`pnpm: latest`): with zero build scripts approved, `lint`, `tsc --noemit`,
`test` (40 suites, 153 tests) and the production `vite build` all pass.
The native bindings these packages need come from optionalDependencies
(@esbuild/*, @swc/core-*), not their install scripts; core-js's script is
only a funding message. The file also never existed on main, which has
always installed with pnpm-latest and no approvals, so removing it restores
parity and cannot regress the build.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dane-h

dane-h commented Jul 1, 2026

Copy link
Copy Markdown
Author

Review response summary

TL;DR: One open finding this round (the pnpm-workspace.yaml) — valid, and fixed with a code change (8fb5416 removes the file). I also re-verified all previously-answered threads against the current HEAD and every claimed fix is actually present in the code. The PR is clear to proceed once a maintainer approves the CI run (the Feature Build has been sitting in action_required on every commit because it's a fork PR).

This round

Comment Verdict Action
pnpm-workspace.yaml — "claude being silly… work out which ones need to be true" ✅ Valid Deleted the file (8fb5416)

allowBuilds: is not a real pnpm key (the field is onlyBuiltDependencies, a list), and the values were literal set this to true or false placeholders — so pnpm ignored the file entirely and still warned about all five packages. I reproduced a clean install with pnpm 10.18.3 (matching the devcontainer's pnpm: latest) and confirmed that with zero build scripts approved, lint + tsc --noemit + test (40 suites / 153 tests) + vite build all pass. The native bindings come from optionalDependencies, core-js's script is a funding message, @parcel/watcher is watch-only, and @sentry/cli only runs on the release path (PR builds set SENTRY_DISABLED=true). The file was also net-new in this PR and never existed on main, so removal restores parity. Full detail + the offer of a proper onlyBuiltDependencies: ['@sentry/cli'] alternative is in the inline thread.

Re-verification of earlier threads (all accurate)

Reply claim Verified in current code
Root cause reframed to Issue.inverseRelations; merge + type-flip + dedup-by-id useIssueRelationships.ts
id added to relations.nodes in the plural query getIssuesService.ts:42
inverseRelations block added to the plural query getIssuesService.ts:63–85
similar added to INVERSE_TYPE (symmetric → itself) useIssueRelationships.ts:8
similar: 'Similar to' in RELATIONSHIP_LABELS + 'similar' in the union; RELATIONSHIP_OPTIONS left unchanged types/relationships.ts
Empty test replaced with real merge/type-mapping tests test file ✅ (now 9 tests — the reply said "8"; the similar test added later makes 9, an undercount not a wrong claim)
manifest.json proxy dots escaped (api\.linear\.app) + both entries manifest.json:93–100

I also spot-checked two factual assertions inside the replies: the vendored Linear-API@current.graphql IssueRelationType enum genuinely lists only blocks, duplicate, related (no similar) — so the "stale schema" explanation is true — and issues(filter:) is indeed optional in the schema, so djgadd's "filter is not required" correction was correct. No discrepancies found between any reply and the code.

Why the mistakes happened & how to prevent them

The recurring pattern was asserting or committing without checking the source of truth:

  1. Wrong root cause ("empty filter is rejected because filter is required") — stated a GraphQL constraint without reading the schema, where issues(filter:) is plainly optional. → Verify API claims against the actual/live schema before asserting them.
  2. Deleted the feature instead of fixing it — the first pass removed reverse-relation discovery rather than repairing the call. → When a call errors, fix the call; don't quietly drop the capability.
  3. Missed similar — trusted the vendored schema, which is stale (3 of 4 enum values). → Treat the checked-in schema as possibly outdated; regenerate/verify against the live API for enum-completeness.
  4. Incomplete plumbing — fixed the singular query but left the plural one missing id/inverseRelations, silently dropping relations. → When changing a data contract (dedup key, new field), audit every query/call site feeding the consumer.
  5. Empty regression test — asserted "nothing bad happens" instead of behavior. → Tests must assert positive behavior.
  6. The pnpm-workspace.yaml — invented a config schema in response to a pnpm-10 install warning and committed a local-env artifact. → Don't invent tool config keys; verify them. Don't commit local install noise. Reproduce the build before shipping a "fix" for a warning.

The through-line matches our commit-hygiene guidance: local pnpm v10/v11 churn produces stray lockfile/workspace files that shouldn't be committed, and "fixes" should be reproduced locally before pushing.

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.

3 participants