Validate Railway deploy config by rissrice2105-agent · Pull Request #643 · profullstack/sh1pt

rissrice2105-agent · 2026-06-05T23:09:06Z

Fixes #642.

Changes:

require projectId and serviceId before Railway shipping starts
validate IDs as single URL path segments
validate environment names and reject blank values
normalize config before dry-run and real CLI execution
add regression tests with mocked railway CLI execution

Validation:

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

greptile-apps · 2026-06-05T23:11:51Z

Greptile Summary

This PR adds upfront validation to the Railway deployment target, requiring non-empty projectId and serviceId (checked as single URL path segments), validating or defaulting the environment name, and trimming all three values before any CLI work begins.

requireText, requireSegment, and environmentName helpers are introduced and called in a normalization block at the top of ship(), so bad config is rejected before dryRun short-circuits or the Railway CLI is invoked.
Tests cover the dry-run path, three rejection cases, and a full CLI invocation with whitespace-padded inputs to confirm trimming.

Confidence Score: 4/5

The new validation logic is correct and tests exercise all rejection paths; two small stale fragments left over from the pre-normalization version make the code slightly misleading but do not affect runtime behavior.

The normalization block in ship() works correctly end-to-end, and the new tests confirm it. Two leftovers from before the refactor — a dead ?? fallback for env and always-truthy if (config.serviceId) / if (env) guards — are unreachable but create a false impression that the environment could still be undefined or the service flag could be skipped after validation passes.

packages/targets/deploy-railway/src/index.ts — lines 49 and 59–60 carry dead code that should be cleaned up before merging.

Important Files Changed

Filename	Overview
packages/targets/deploy-railway/src/index.ts	Adds input validation helpers (requireText, requireSegment, environmentName) and normalizes config before shipping; leaves two unreachable code fragments (dead ?? fallback and always-true guards) from the pre-normalization version.
packages/targets/deploy-railway/src/index.test.ts	Expands test coverage with mocked exec: validates dry-run idempotency, config rejection for blank/invalid fields, and full CLI invocation with trimmed args; looks correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ship called] --> B[requireSegment projectId]
    B -->|blank or invalid chars| ERR1[throw: requires projectId / must be single path segment]
    B --> C[requireSegment serviceId]
    C -->|blank or invalid chars| ERR2[throw: requires serviceId / must be single path segment]
    C --> D[environmentName]
    D -->|environment blank| ERR3[throw: requires environment]
    D -->|environment invalid chars| ERR4[throw: must contain only letters...]
    D -->|environment undefined| DEFAULT[default to production or staging based on channel]
    D --> E[config normalized]
    DEFAULT --> E
    E --> F{dryRun?}
    F -->|yes| G[return id: dry-run]
    F -->|no| H{RAILWAY_TOKEN set?}
    H -->|no| ERR5[throw: RAILWAY_TOKEN required]
    H -->|yes| I[exec railway up --ci --service --environment]
    I --> J[return id + meta]

Comments Outside Diff (1)

packages/targets/deploy-railway/src/index.ts, line 59-60 (link)

Both guards are always truthy after the normalization block — requireSegment throws when serviceId is empty, and env is now guaranteed non-empty. The if (config.serviceId) guard in particular implies --service could legitimately be omitted, which would cause Railway to deploy to whatever service it picks by default if the guard ever evaluated to false. Removing the guards makes the invariant explicit.

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

greptile-apps · 2026-06-05T23:11:56Z

+      serviceId: requireSegment(config.serviceId, 'serviceId'),
+      environment: environmentName(config, ctx.channel),
+    };
    const env = config.environment ?? (ctx.channel === 'stable' ? 'production' : 'staging');


The ?? fallback for env can never execute. environmentName() is called on line 47 and always either throws or returns a non-empty string, so config.environment is guaranteed to be a non-empty string by the time line 49 runs — the null-coalescing branch is dead code left over from before the normalization block was introduced.

Suggested change

const env = config.environment ?? (ctx.channel === 'stable' ? 'production' : 'staging');

const env = config.environment!;

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:09:11Z