Validate Discord app distribution config by rissrice2105-agent · Pull Request #611 · profullstack/sh1pt

rissrice2105-agent · 2026-06-05T18:22:04Z

Fixes #610.

Changes:

validate Discord applicationId as numeric runtime input
validate distribution against private/public/directory before manifest generation
validate OAuth scopes before creating invite URLs or dry-run ship metadata
add regression tests for invalid applicationId, distribution, and scopes

Validation:

vitest run packages/targets/chat-discord/src/index.test.ts
tsc -p packages/targets/chat-discord/tsconfig.json --noEmit

greptile-apps · 2026-06-05T18:25:28Z

Greptile Summary

This PR adds runtime input validation to the Discord chat target: applicationId is now checked to be a numeric snowflake, distribution is validated against the private/public/directory allowlist, and OAuth scopes are validated against the supported bot/applications.commands set. Three regression tests cover each new validation path.

requireDistribution and the scopesFor scope-check guard manifestFor, which is shared by both build and ship, so both paths benefit from the new guards.
requireApplicationId and scopesFor are each called twice per manifestFor invocation (once directly, once inside inviteUrl), because inviteUrl re-validates without receiving the already-computed values — this is pre-existing but worth noting.
Error messages for invalid distribution and invalid scope omit the rejected value, which reduces debuggability when configs are assembled programmatically.

Confidence Score: 4/5

Safe to merge — the new validations are additive guards with no changes to the happy-path logic, and all existing tests should continue to pass.

The validation logic is straightforward and correct. The only live concerns are that error messages drop the rejected value (harder to debug in production) and the test suite misses the build path for an invalid distribution. Neither affects current runtime correctness of the validated paths.

Both files are worth a second look: index.ts for the double-validation in manifestFor/inviteUrl and the error message detail, index.test.ts for the missing build-path distribution test.

Important Files Changed

Filename	Overview
packages/targets/chat-discord/src/index.ts	Adds runtime validation for applicationId (numeric check), distribution (allowlist), and OAuth scopes (allowlist); requireApplicationId and scopesFor are each called twice per manifestFor invocation, and error messages omit the rejected value.
packages/targets/chat-discord/src/index.test.ts	Adds three regression tests for the new validations; invalid-distribution test only exercises the ship code path, leaving the equivalent build path untested.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[build / ship called] --> B[manifestFor]
    B --> C[normalizeCommands]
    C --> D{requireDistribution}
    D -- invalid --> E[throw: distribution must be one of]
    D -- valid --> F[requireApplicationId]
    F -- missing/empty --> G[throw: requires applicationId]
    F -- non-numeric --> H[throw: must be numeric snowflake]
    F -- valid --> I[scopesFor]
    I -- invalid scope found --> J[throw: scope must be one of]
    I -- valid / defaults used --> K[inviteUrl]
    K --> L[requireApplicationId again]
    K --> M[scopesFor again]
    L --> N[build manifest object]
    M --> N
    N --> O{ctx.dryRun?}
    O -- yes --> P[return dry-run meta]
    O -- no --> Q[lookup secret token]
    Q --> R[callDiscord PATCH / PUT]

_{Reviews (1): Last reviewed commit: "Validate Discord app distribution config" | Re-trigger Greptile}

greptile-apps · 2026-06-05T18:25:33Z

+  const invalid = scopes.find((scope) => !SCOPES.includes(scope as typeof SCOPES[number]));
+  if (invalid) throw new Error(`chat-discord scope must be one of: ${SCOPES.join(', ')}`);


The error omits the invalid value, making it harder to debug when a bad scope is passed programmatically. Including the rejected value in the message gives the caller something actionable to act on immediately.

Suggested change

const invalid = scopes.find((scope) => !SCOPES.includes(scope as typeof SCOPES[number]));

if (invalid) throw new Error(`chat-discord scope must be one of: ${SCOPES.join(', ')}`);

const invalid = scopes.find((scope) => !SCOPES.includes(scope as typeof SCOPES[number]));

if (invalid) throw new Error(`chat-discord scope "${invalid}" is not supported; must be one of: ${SCOPES.join(', ')}`);

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!

greptile-apps · 2026-06-05T18:25:34Z

+  if (!DISTRIBUTIONS.includes(distribution as Config['distribution'])) {
+    throw new Error(`chat-discord distribution must be one of: ${DISTRIBUTIONS.join(', ')}`);
+  }


Same pattern: the error message for an invalid distribution doesn't include the rejected value. When config is assembled dynamically (e.g. from a deserialized config file) the error gives no hint of what value was actually received.

Suggested change

if (!DISTRIBUTIONS.includes(distribution as Config['distribution'])) {

throw new Error(`chat-discord distribution must be one of: ${DISTRIBUTIONS.join(', ')}`);

}

if (!DISTRIBUTIONS.includes(distribution as Config['distribution'])) {

throw new Error(`chat-discord distribution "${distribution}" is not supported; must be one of: ${DISTRIBUTIONS.join(', ')}`);

}

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!

greptile-apps · 2026-06-05T18:25:35Z

+  it('rejects unsupported distributions in dry-run shipping', async () => {
+    await expect(adapter.ship(fakeShipContext({
+      version: '1.2.3',
+      dryRun: true,
+    }) as any, {
+      applicationId: '123456',
+      distribution: 'server-listing',
+    } as any)).rejects.toThrow('distribution must be one of');
+  });


Missing build-path coverage for invalid distribution

The new distribution test only exercises adapter.ship. Both build and ship reach manifestFor → requireDistribution, so the validation fires on both paths. Adding a parallel test for adapter.build with an invalid distribution would close the gap and guard against a future refactor that separates the two paths.

github-actions · 2026-06-06T07:13:39Z