Validate Signal runtime config by rissrice2105-agent · Pull Request #617 · profullstack/sh1pt

rissrice2105-agent · 2026-06-05T18:34:02Z

Fixes #616.

Changes:

validate phoneNumber as E.164 before build or ship
validate runtime against signal-cli/signald before build or ship
use normalized values in runtime plans and ship IDs
add regression tests for invalid phone numbers and unsupported runtimes

Validation:

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

greptile-apps · 2026-06-05T18:36:21Z

Greptile Summary

This PR adds upfront validation of phoneNumber (E.164 regex) and runtime (allow-list against signal-cli/signald) to both the build and ship entry points, replacing raw config values with the normalised results throughout the output plan and ship ID.

requirePhoneNumber and requireRuntime helpers throw early with descriptive messages before any filesystem I/O or network calls are made.
Two regression tests cover the invalid-phone-number path in build and the invalid-runtime path in ship; the runtime guard in build has no corresponding test.

Confidence Score: 4/5

Safe to merge; the validation logic is straightforward and both entry points are guarded. The two minor gaps noted are worth addressing but do not block the change.

The E.164 regex lower-bound of 8 digits after + is slightly too strict and could silently reject valid numbers from a small set of countries, while the invalid-runtime path through build has no test to catch a future regression. Neither issue affects the common case for this adapter.

Both changed files are straightforward; index.ts deserves a quick look at the E.164 regex bound, and index.test.ts is missing one build-side runtime test.

Important Files Changed

Filename	Overview
packages/targets/chat-signal/src/index.ts	Adds `requirePhoneNumber` (E.164 regex) and `requireRuntime` (allow-list) guards called at the top of both `build` and `ship`; validated values replace raw config fields in the output plan and ship ID. Minor regex tightness concern.
packages/targets/chat-signal/src/index.test.ts	Adds two regression tests: one for non-E.164 phone numbers in `build`, one for an unsupported runtime in `ship`. Runtime rejection in `build` is not covered by a parallel test.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[build / ship called] --> B[requirePhoneNumber]
    B -->|invalid E.164| C[throw Error]
    B -->|valid| D[requireRuntime]
    D -->|not in RUNTIMES| E[throw Error]
    D -->|valid| F[proceed with phoneNumber + runtime]
    F --> G{build?}
    G -->|yes| H[write signal-runtime-plan.json]
    G -->|no - ship| I{ctx.dryRun?}
    I -->|yes| J[return id: dry-run]
    I -->|no| K[return id: signal:phoneNumber@version]

_{Reviews (1): Last reviewed commit: "Validate Signal runtime config" | Re-trigger Greptile}

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

+
+function requirePhoneNumber(config: Config): string {
+  const phoneNumber = config.phoneNumber?.trim();
+  if (!/^\+[1-9]\d{7,14}$/.test(phoneNumber)) {


The E.164 regex requires at minimum 9 characters after + ([1-9] plus \d{7,14}), but the ITU-T standard allows as few as 4 total digits for some assigned country codes (e.g., Niue +683XXXX = 7 digits). Any phone number with fewer than 8 digits after + will be incorrectly rejected. Lowering the lower-bound to {6,14} brings coverage down to the smallest real-world allocations while staying well within E.164's 15-digit ceiling.

Suggested change

if (!/^\+[1-9]\d{7,14}$/.test(phoneNumber)) {

if (!/^\+[1-9]\d{6,14}$/.test(phoneNumber)) {

greptile-apps · 2026-06-05T18:36:26Z

+  it('rejects unsupported runtimes before dry-run shipping', async () => {
+    await expect(adapter.ship(fakeShipContext({
+      version: '1.2.3',
+      dryRun: true,
+    }) as any, {
+      ...sampleConfig,
+      runtime: 'signal-web',
+    } as any)).rejects.toThrow('runtime must be one of');
+  });


Missing coverage: build with invalid runtime. The new runtime-rejection test only exercises ship. Since build also calls requireRuntime directly, a counterpart test that passes an invalid runtime to build would confirm the guard fires on both entry points. Without it, a future refactor that accidentally drops the requireRuntime call from build would go undetected.

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!

github-actions · 2026-06-06T07:11:36Z