Skip to content

feat(deep-links): Use posthog-code-dev scheme in dev builds#1780

Open
Twixes wants to merge 1 commit intosignals/share-inboxfrom
04-21-feat_deep-links_use_posthog-code-dev_scheme_in_development_builds
Open

feat(deep-links): Use posthog-code-dev scheme in dev builds#1780
Twixes wants to merge 1 commit intosignals/share-inboxfrom
04-21-feat_deep-links_use_posthog-code-dev_scheme_in_development_builds

Conversation

@Twixes
Copy link
Copy Markdown
Member

@Twixes Twixes commented Apr 21, 2026

Problem

In development builds, the app was skipping deep link protocol registration entirely to avoid stealing the posthog-code:// scheme from a production install. This made it impossible to test deep linking as a dev.

Changes

Introducing a posthog-code-dev URL scheme for development builds only, defined via getDeeplinkProtocol(isDevBuild). This function returns posthog-code-dev in dev and posthog-code in production.

  1. Dev builds now register and handle posthog-code-dev:// instead of skipping registration altogether.
  2. Production builds continue to register posthog-code:// along with the legacy twig:// and array:// schemes.

How did you test this?

Updated and added unit tests in service.test.ts covering:

  • registerProtocol registers only posthog-code-dev in dev and all three schemes in production.
  • handleUrl accepts posthog-code-dev:// in dev and rejects posthog-code://, and vice versa in production.
  • getProtocol returns the correct scheme per build.

Copy link
Copy Markdown
Member Author

Twixes commented Apr 21, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Twixes Twixes requested a review from a team April 21, 2026 17:08
@Twixes Twixes marked this pull request as ready for review April 21, 2026 17:08
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 21, 2026

Comments Outside Diff (1)

  1. apps/code/src/main/services/deep-link/service.test.ts, line 44-79 (link)

    P2 Prefer parameterised tests

    The registerProtocol describe block has two independent cases (production vs development) that are a natural fit for it.each. The primary protocol by build and getProtocol describes share the same pattern. Consolidating them would reduce duplication and make adding future schemes a one-line change, in line with the project's preference for parameterised tests.

    it.each([
      { isDev: "false", expectedSchemes: ["posthog-code", "twig", "array"] },
      { isDev: "true", expectedSchemes: ["posthog-code-dev"] },
    ])(
      "registers $expectedSchemes when POSTHOG_CODE_IS_DEV=$isDev",
      ({ isDev, expectedSchemes }) => {
        process.env.POSTHOG_CODE_IS_DEV = isDev;
        service.registerProtocol();
        for (const scheme of expectedSchemes) {
          expect(mockAppLifecycle.registerDeepLinkScheme).toHaveBeenCalledWith(scheme);
        }
        expect(mockAppLifecycle.registerDeepLinkScheme).toHaveBeenCalledTimes(expectedSchemes.length);
      },
    );

    The same pattern applies to the primary protocol by build block (lines 242-271) and getProtocol block (lines 274-284).

    Context Used: Do not attempt to comment on incorrect alphabetica... (source)

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/code/src/main/services/deep-link/service.test.ts
    Line: 44-79
    
    Comment:
    **Prefer parameterised tests**
    
    The `registerProtocol` describe block has two independent cases (production vs development) that are a natural fit for `it.each`. The `primary protocol by build` and `getProtocol` describes share the same pattern. Consolidating them would reduce duplication and make adding future schemes a one-line change, in line with the project's preference for parameterised tests.
    
    ```ts
    it.each([
      { isDev: "false", expectedSchemes: ["posthog-code", "twig", "array"] },
      { isDev: "true", expectedSchemes: ["posthog-code-dev"] },
    ])(
      "registers $expectedSchemes when POSTHOG_CODE_IS_DEV=$isDev",
      ({ isDev, expectedSchemes }) => {
        process.env.POSTHOG_CODE_IS_DEV = isDev;
        service.registerProtocol();
        for (const scheme of expectedSchemes) {
          expect(mockAppLifecycle.registerDeepLinkScheme).toHaveBeenCalledWith(scheme);
        }
        expect(mockAppLifecycle.registerDeepLinkScheme).toHaveBeenCalledTimes(expectedSchemes.length);
      },
    );
    ```
    
    The same pattern applies to the `primary protocol by build` block (lines 242-271) and `getProtocol` block (lines 274-284).
    
    **Context Used:** Do not attempt to comment on incorrect alphabetica... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
    
    How can I resolve this? If you propose a fix, please make it concise.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/code/src/main/services/deep-link/service.test.ts
Line: 44-79

Comment:
**Prefer parameterised tests**

The `registerProtocol` describe block has two independent cases (production vs development) that are a natural fit for `it.each`. The `primary protocol by build` and `getProtocol` describes share the same pattern. Consolidating them would reduce duplication and make adding future schemes a one-line change, in line with the project's preference for parameterised tests.

```ts
it.each([
  { isDev: "false", expectedSchemes: ["posthog-code", "twig", "array"] },
  { isDev: "true", expectedSchemes: ["posthog-code-dev"] },
])(
  "registers $expectedSchemes when POSTHOG_CODE_IS_DEV=$isDev",
  ({ isDev, expectedSchemes }) => {
    process.env.POSTHOG_CODE_IS_DEV = isDev;
    service.registerProtocol();
    for (const scheme of expectedSchemes) {
      expect(mockAppLifecycle.registerDeepLinkScheme).toHaveBeenCalledWith(scheme);
    }
    expect(mockAppLifecycle.registerDeepLinkScheme).toHaveBeenCalledTimes(expectedSchemes.length);
  },
);
```

The same pattern applies to the `primary protocol by build` block (lines 242-271) and `getProtocol` block (lines 274-284).

**Context Used:** Do not attempt to comment on incorrect alphabetica... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "feat(deep-links): Use `posthog-code-dev`..." | Re-trigger Greptile

@Twixes Twixes force-pushed the 04-21-feat_deep-links_use_posthog-code-dev_scheme_in_development_builds branch from 65c167c to 665e5d9 Compare April 21, 2026 17:13
@Twixes Twixes force-pushed the 04-21-feat_deep-links_use_posthog-code-dev_scheme_in_development_builds branch from 665e5d9 to 15e39c8 Compare April 21, 2026 17:16
Copy link
Copy Markdown
Contributor

adboio commented Apr 21, 2026

@ryans-posthog probably not(?) but maybe related to the issues you were telling me about yesterday

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.

2 participants