feat(mt#1730): deployment-platform-aware MCP tools with Railway as v1 adapter#1041
Conversation
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: unknown
Good structural work: a clear adapter interface, a Railway v1 adapter, service-level config, and shared commands are in place with tests. However, there are blocking issues to address before merge. First, the documentation’s defineDeployment import path is wrong and will lead authors to copy a broken example. Second, there’s a naming/registration gap between the documented MCP tool names (deployment_wait_for_latest etc.) and the actual command IDs (deployment.wait-for-latest etc.) with no visible bridge mapping — tools may not be discoverable under the promised names. Third, the Railway adapter treats “not found in the last 5 deployments” as a terminal CANCELLED, which risks false terminal states.
Non-blocking notes: ensure default parameter values are applied (or documented as adapter-side defaults), and consider surfacing clearer auth guidance for the ~/.railway/config.json requirement. Fix the blocking items and this will be a solid, platform-agnostic deployment observation layer.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: unknown
Prior blocking issues are addressed. The docs now use the correct @minsky/shared/deployment-config import and the repo exports it; MCP tool naming is clarified with the documented dot-to-underscore bridge and commands are registered under dotted IDs; and the Railway adapter no longer infers CANCELLED when a deployment falls out of the recent-window, instead polling the targeted deployment by ID and erroring if it disappears. I found no new critical defects introduced by these changes. Event is APPROVE.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: unknown
Good structural work and tests, but two blocking issues remain. The platform-agnostic docs use an incorrect import path for defineDeployment, which will cause authors to copy a broken example. In the Railway adapter, waitForLatestDeployment treats a target deployment falling out of the last-5 window as terminal CANCELLED, which can yield false terminal states under churn. Please correct the docs to import from @minsky/shared/deployment-config and adjust the adapter’s missing-target handling (e.g., keep waiting or widen/direct lookup) rather than fabricating a terminal status. Also, please confirm the MCP tool name mapping from command IDs to the documented names; if not present, add aliases.
Findings
- [BLOCKING] docs/deployment-platforms.md:1 — Documentation uses incorrect import path for
defineDeployment(points to../../scripts/railway/libinstead of@minsky/shared/deployment-config)
Indocs/deployment-platforms.md, the configuration examples show:
import { defineDeployment } from "../../scripts/railway/lib";However, the implemented and exported factory lives at @minsky/shared/deployment-config per packages/shared/package.json and packages/shared/src/deployment/config.ts, and is consumed that way in services/minsky-mcp/deploy.config.ts:
import { defineDeployment } from "@minsky/shared/deployment-config";Authors copying the doc’s snippet will get a broken import. Please update both instances in the doc (the base example and the variant that imports IDs from railway.config.ts) to use @minsky/shared/deployment-config.
Resolves additive conflict in src/adapters/shared/commands/index.ts — both branches added new command registrations (mt#1730: deployment; main: observability + principal-corpus). Kept all three.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 3
Prior BLOCKING findings are addressed and verified: docs import paths fixed, MCP tool naming clarified with the dot→underscore (dash-preserving) convention, and the Railway adapter now polls the targeted deployment by ID instead of inferring from a recent-N window. The new registry, service resolver, adapter, and MCP tools are coherent and covered by unit tests. I found no new critical defects in the fixes; only a minor comment mismatch in one file’s header about the wire-form tool name. Overall, the implementation meets the spec and is ready to merge.
Findings
- [NON-BLOCKING] src/adapters/shared/commands/deployment.ts:5 — Tool name in header comment doesn’t match the documented wire form
The header comment listsdeployment_wait_for_latest(all underscores), but the documented and registered tool uses a dash-preserving form:deployment_wait-for-latest(dots→underscores, dashes preserved). See the registration atsrc/adapters/shared/commands/deployment.ts:53with ID"deployment.wait-for-latest"and the naming convention indocs/deployment-platforms.md. Suggest updating the comment to avoid confusion for copy/paste readers.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| Phase 1: Configuration shape — Define how a project declares its deployment target; platform-neutral with discriminator and platform-specific block. | Met | packages/shared/src/deployment/config.ts defines DeploymentConfig discriminated union and defineDeployment(); services/minsky-mcp/deploy.config.ts uses it with platform: "railway" and a railway block. |
Phase 1: Platform abstraction interface — TypeScript interface DeploymentPlatformAdapter with wait/status/logs; platform-neutral types. |
Met | src/domain/deployment/types.ts defines DeploymentPlatformAdapter, DeploymentRecord, DeploymentStatus, LogType, LogLine, WaitForLatestOptions with no Railway-specific leakage. |
| Phase 1: Adapter registry — Registry keyed by platform name; typed error on unknown; no fallback. | Met | src/domain/deployment/registry.ts implements registerAdapter, resolveAdapter, UnknownDeploymentPlatformError, getRegisteredPlatforms; tests at src/domain/deployment/registry.test.ts validate behavior. |
| Phase 1: MCP tool surface — Three platform-neutral tools with correct naming convention and parameters. | Met | src/adapters/shared/commands/deployment.ts registers deployment.wait-for-latest, deployment.status, deployment.logs; docs/deployment-platforms.md describes dot→underscore convention yielding deployment_wait-for-latest wire name. |
| Phase 1: Decide scope of platforms — v1 Railway only, path for others documented. | Met | docs/deployment-platforms.md states Railway v1, shows how to add Vercel; PlatformName union currently "railway" in packages/shared/src/deployment/config.ts. |
Summary
Wraps the project's deployment platform (Railway for this repo) behind three platform-neutral MCP tools:
deployment_wait-for-latest,deployment_status,deployment_logs. The agent no longer needs to know the underlying platform's CLI shape to wait on a deploy — same shape assession_pr_wait-for-review(encapsulates GitHub's webhook plumbing) andsession_pr_checks.Closes mt#1730. Sibling structural fix to mt#1725 (Minsky-side pr_watch wiring); both originate from the 2026-05-11 PR #1036 incident where the agent reached for
ScheduleWakeupand a 30-iteration bash poll loop instead of the platform-native wait primitives.Design principle: platform-agnostic from day one
The MCP tool surface, configuration shape, and adapter interface are platform-neutral. Railway is the v1 concrete adapter; v2 candidates (Vercel, Cloudflare Pages, Fly.io, AWS Amplify) implement the same
DeploymentPlatformAdapterinterface and slot in via a registry — no agent-facing change required. Seedocs/deployment-platforms.mdfor the full abstraction and the path for adding a new platform.Key Changes
Phase 1 — Design (commit f5d38bf)
docs/deployment-platforms.md(new) — adapter interface, registry, configuration shape, MCP tool surface, naming-convention bridge note, Railway adapter strategy, path for adding a new platform.docs/deploy-minsky-railway.md(extended) — Railway-specific section pointing at the platform-agnostic doc.Phase 2 — Implementation (commit c24d622, R1 fixes in 225faf6)
Types and configuration (
@minsky/sharedfor boundary safety):packages/shared/src/deployment/config.ts—defineDeploymentfactory,DeploymentConfigdiscriminated union,RailwayDeploymentConfig,PlatformName.packages/shared/package.json— new./deployment-configexport.Runtime side (
src/domain/deployment/):types.ts—DeploymentPlatformAdapterinterface, normalizedDeploymentStatusunion,DeploymentRecord,DeploymentWaitTimeoutError,isTerminalStatus.config.ts— re-export from@minsky/shared.registry.ts—registerAdapter/resolveAdapter/getRegisteredPlatforms. ThrowsUnknownDeploymentPlatformErroron miss (no silent fallback).service-resolver.ts— enumeratesservices/<svc>/deploy.config.tsfiles, auto-selects when exactly one is configured, throws typed errors for ambiguous / missing cases.railway/graphql-client.ts— extracted Railway GraphQL primitives + auth fromscripts/railway/{lib,status,logs}.ts. AddsfetchDeploymentByIdfor direct deployment-ID querying. No fresh CLI shell-out introduced.railway/adapter.ts—RailwayDeploymentAdapterwith status normalization and pollingwaitForLatestDeployment. R1 fix: polls the targeted deployment by ID rather than filtering a recent-N window.railway/index.ts— self-registers"railway"with the adapter registry on module load.index.ts— barrel + side-effect import.MCP tools (
src/adapters/shared/commands/deployment.ts):deployment.wait-for-latest(wire form:mcp__minsky__deployment_wait-for-latest).deployment.status.deployment.logs.Service config (
services/minsky-mcp/deploy.config.ts):platform: "railway", imports IDs from the existingrailway.config.tsto avoid duplication.Phase 3 — Adoption
.claude/skills/implement-task/SKILL.md(+ source) — new### 10. Post-merge deploy verificationsection..claude/skills/orchestrate/SKILL.md(+ source) — cross-epic deploy verification section.feedback_external_system_event_wait_surveymemory entry — extended.Reviewer iteration
R0 review (CHANGES_REQUESTED, reviewId 4267321505) raised 3 BLOCKING findings:
defineDeploymentcode blocks imported from../../scripts/railway/libinstead of the correct@minsky/shared/deployment-config. Fixed in 225faf6.deployment_wait_for_latest(all underscores) but the MCP bridge convention is "dots become underscores, dashes are preserved" (verified against existingsession.pr.wait-for-review→mcp__minsky__session_pr_wait-for-review). Updated all docs/skills/memory to the correct wire formdeployment_wait-for-latestand added a "Naming convention" subsection todocs/deployment-platforms.md. Fixed in 225faf6.fetchDeploymentByIdRailway GraphQL primitive and switched the polling loop to fetch the targeted deployment directly. On null, throws an explicit error rather than masking as CANCELLED. Fixed in 225faf6.R1 review (APPROVED, reviewId 4267357573) confirmed all three findings addressed.
Concurrency analysis
(N/A — no check-then-act pattern introduced.) The polling loop re-fetches the targeted deployment's authoritative state on each tick from the platform's API; there is no decision-then-action gap. The pre-R1 code had a "if not in recent-5, assume CANCELLED" inference that conflated absence-from-window with terminal-state — the R1 fix eliminates that inference.
Spec verification
The mt#1730 spec's Phase 1 / Phase 2 / Phase 3 success criteria are met:
docs/deployment-platforms.md).RailwayDeploymentAdapter, three platform-neutral MCP tools, service config, 22 tests passing.railway logs --buildreferences remain only in anti-pattern guidance.Documentation impact
docs/deployment-platforms.md(new) — full design reference.docs/deploy-minsky-railway.md— extended with## Deployment platform MCP toolssection..claude/skills/implement-task/SKILL.md+.minsky/skills/implement-task/skill.ts— new step 10..claude/skills/orchestrate/SKILL.md+.minsky/skills/orchestrate/skill.ts— new section.feedback_external_system_event_wait_surveymemory entry — extended.Execution evidence:
CI: build + Prevent Placeholder Tests both green on 225faf6.
Live verification
This change is structural per
/implement-task §7a(new external-system wait primitive). End-to-end live verification withdeployment_wait-for-latestagainst the actualminsky-mcpRailway service requires a Railway auth token and a fresh deploy in flight. Deferred to main-agent context post-merge.Test plan
deployment_wait-for-latest(Phase 3 acceptance)deployment_wait-for-latestexercise on next Railway redeploy (post-merge, main agent)