Skip to content

Commit 0584173

Browse files
Merge pull request #121 from paritytech/fix/mod-origin-and-repo-probe
fix(mod): silence false origin warning and verify repo before download
2 parents 63bd8a4 + fe6fe64 commit 0584173

7 files changed

Lines changed: 135 additions & 9 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"playground-cli": patch
3+
---
4+
5+
`dot mod` no longer prints a misleading `[contracts] No origin configured — using dev fallback (Alice) for query dry-run` warning when the user is signed in via `dot init`. The CDM meta-registry lookup that resolves the live playground registry address now receives the signer's address as `defaultOrigin`. `dot mod` also lazy-probes the picked app's repository between picker dismount and the setup steps, surfacing a clear "private or does not exist" error before any files are written when a publisher has flipped repo visibility after deploying.

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ These are things that aren't self-evident from reading the code and have bitten
2323
- **Throttle TUI info updates** — bulletin-deploy logs per-chunk and builds (vite/next) stream thousands of lines/sec. Calling `setState` on every log event floods React's reconciler with so much backpressure the process can balloon past 20 GB and freeze the OS. `RunningStage` coalesces "latest info" updates to ≤10/sec via a ref + timer and caps line length at 160 chars. Any new hot-path event sink should do the same; don't hook raw per-line streams directly into Ink state.
2424
- **Process-guard safety net** (`src/utils/process-guard.ts`) — deploy pipelines open several long-lived WebSockets + child processes and any one of them can keep the event loop alive after the TUI visibly finishes, turning `dot` into a zombie that accumulates retry buffers indefinitely (seen climbing past 25 GB). We defend in depth: (1) `installSignalHandlers()` catches SIGINT/TERM/HUP + `unhandledRejection` and forces cleanup + exit within 3 s; (2) `scheduleHardExit()` installs an `unref`'d timer that kills the process if the event loop doesn't drain within a grace period; (3) `startMemoryWatchdog()` aborts if RSS exceeds 4 GB — a generous cap because legit deploys on Bun SEA binaries routinely touch 1–1.5 GB from runtime-metadata decoding + Bun's JSC heap + Ink yoga. Do NOT re-add a per-window growth detector: we tried 300 MB / 3 s and it false-positived on the single-burst metadata-loading spike, aborting deploys that would have succeeded. Set `DOT_MEMORY_TRACE=1` to stream per-sample RSS/heap/external stats — useful when diagnosing a real leak report. **Telemetry bootstrap** (`src/bootstrap.ts`) is the FIRST import in `src/index.ts`. It sets `BULLETIN_DEPLOY_USE_AMBIENT_SENTRY=1` and `BULLETIN_DEPLOY_HOST_APP=playground-cli` before `bulletin-deploy` can evaluate, then maps `DOT_TELEMETRY`/internal-context detection to `BULLETIN_DEPLOY_TELEMETRY`. Do not leave `BULLETIN_DEPLOY_TELEMETRY` unset while setting the host app: `bulletin-deploy@0.7.6` treats `playground-cli` as an internal host, which would enable deploy telemetry for external users. `BULLETIN_DEPLOY_MEM_REPORT` is not forced off by default anymore because upstream guards the Bun-incompatible memory-report path. Any new long-running command should register a cleanup hook via `onProcessShutdown()`.
2525
- **Parser MUST NOT emit an event per log line.** `DeployLogParser.feed()` is called for every console line bulletin-deploy prints — hundreds per deploy on the happy path, thousands if retries fire. We intentionally emit events ONLY for phase-banner matches and `[N/M]` chunk progress. Everything else returns `null`. Adding a catch-all `info` emit turns the parser into a firehose that allocates ~200 bytes × thousands of lines, and was a measurable contributor to chunk-upload memory pressure.
26-
- **`dot mod` is GitHub-tarball-only and must stay that way.** `src/utils/mod/source.ts` downloads from `codeload.github.com` (no auth, no `git`/`gh` required for the public-repo case) and extracts via `node:zlib` + the pure-JS `tar` package. Do NOT re-introduce `git clone` or `gh repo fork` paths — both would re-add a hard tooling requirement and the fork path was specifically removed because GitHub caps you to one fork per source-repo per account, which broke "mod the same starter twice." A non-modable app (no `metadata.repository`) returns a hard error from `dot mod`; the interactive picker filters those out so the user never sees an unmoddable option.
26+
- **`dot mod` is GitHub-tarball-only and must stay that way.** `src/utils/mod/source.ts` downloads from `codeload.github.com` (no auth, no `git`/`gh` required for the public-repo case) and extracts via `node:zlib` + the pure-JS `tar` package. Do NOT re-introduce `git clone` or `gh repo fork` paths — both would re-add a hard tooling requirement and the fork path was specifically removed because GitHub caps you to one fork per source-repo per account, which broke "mod the same starter twice." A non-modable app (no `metadata.repository`) returns a hard error from `dot mod`; the interactive picker filters those out so the user never sees an unmoddable option. The picker does NOT pre-probe each app's repo visibility, because that would burn the 60 req/hr anonymous GitHub API quota on every `dot mod`. Instead, `runModCommand` lazy-probes the picked app once via `assertPublicGitHubRepo()` between picker dismount and `SetupScreen` mount; `dot deploy --modable` already rejects private repos at deploy time, so this fires only when a publisher has flipped visibility post-publish.
2727
- **`ensureGhAuthed()` does NOT shell out to `gh auth login`.** Even from the interactive deploy, Ink owns stdout + raw-mode stdin and a `stdio: "inherit"` child would race Ink's `useInput` for keystrokes — producing garbled output and dropped key events. Both interactive and non-interactive paths that need GitHub repo creation fail with the same actionable message: run `gh auth login` once outside `dot`, then retry. Existing `origin` URLs do not require `gh` auth. Properly suspending Ink to hand off stdio is possible but requires unmounting + re-rendering with state preservation, and we deemed that complexity not worth it for a one-time-per-machine speedbump. If you ever do implement it, the wiring point is `ModablePreflightStage` in `src/commands/deploy/DeployScreen.tsx`.
2828
- **`metadata.repository` is set ONLY when `--modable` is opted in.** Older code in `publishToPlayground` would silently probe `git remote get-url origin` and stuff whatever it found into the metadata, which surprised users who didn't realise their fork was being advertised. The new contract: `runDeploy` takes an explicit `repositoryUrl: string | null`, and `publishToPlayground` writes the field iff that param is non-null. The CLI command is responsible for resolving the URL upstream via `src/utils/deploy/modable.ts::resolveRepositoryUrl()`, which uses an existing `origin` URL without pushing, or runs `gh repo create --public --push` to set up a fresh public repo when no origin exists. Re-deploys never delete user repos.
2929
- **`startMemoryWatchdog()` runs for both `dot deploy` and `dot mod`.** Mod's tarball download is a streaming pipe through `node:zlib` + `tar.extract()`, and a stuck IPFS gateway or a malformed tarball can leak buffers. Same 4 GB cap, same worker-thread sampler. Any new top-level command that does meaningful I/O should also call `startMemoryWatchdog()` and register `stopWatchdog` via `onProcessShutdown()`.

src/commands/mod/index.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { AppBrowser, type AppEntry } from "./AppBrowser.js";
1010
import { SetupScreen } from "./SetupScreen.js";
1111
import { defaultRepoName } from "../../utils/git/repoName.js";
1212
import { runCliCommand } from "../../cli-runtime.js";
13+
import { assertPublicGitHubRepo, ModablePreflightError } from "../../utils/deploy/modable.js";
1314

1415
export const modCommand = new Command("mod")
1516
.description("Mod a playground app — clone the source as a fresh project to customise")
@@ -56,6 +57,41 @@ async function runModCommand(
5657
metadata = picked;
5758
}
5859

60+
// Lazy verify that the picked app's source repository is publicly
61+
// reachable. The picker filters apps that have NO repository URL, but
62+
// a publisher can flip a repo to private after deploying, which would
63+
// break the anonymous codeload download a few steps down. Bail here
64+
// with a clean message so the user can pick a different app before we
65+
// mount SetupScreen and start writing files.
66+
//
67+
// The direct-domain path (`dot mod some-domain.dot`) has no metadata
68+
// at this point and falls through to SetupScreen, where the same
69+
// 404/401 surfaces from `resolveDefaultBranch` as a step failure.
70+
// Slightly less polished UX, but lifting the metadata fetch up here
71+
// just for symmetry would be a larger refactor.
72+
if (metadata?.repository) {
73+
const repoUrl = metadata.repository;
74+
try {
75+
await withSpan(
76+
"cli.mod.repo-check",
77+
"verify repository is public",
78+
{ "cli.mod.repo": repoUrl },
79+
() => assertPublicGitHubRepo(repoUrl),
80+
);
81+
} catch (err) {
82+
if (err instanceof ModablePreflightError) {
83+
console.error();
84+
console.error(` ${err.message}.`);
85+
console.error(
86+
` Pick a different app or ask the publisher to make the repo public.`,
87+
);
88+
process.exitCode = 1;
89+
return;
90+
}
91+
throw err;
92+
}
93+
}
94+
5995
const targetDir = await withSpan("cli.mod.resolve-target", "resolve target directory", () =>
6096
resolveTargetDir({ domain }),
6197
);

src/utils/contractManifest.test.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,31 @@ describe("resolveLiveContractAddresses", () => {
8080
expect.arrayContaining([
8181
expect.objectContaining({ name: "getAddress", type: "function" }),
8282
]),
83+
expect.any(Object),
8384
);
8485
expect(getAddressQueryMock).toHaveBeenCalledWith(PLAYGROUND_REGISTRY_CONTRACT);
8586
});
8687

88+
it("forwards defaultOrigin to createContractFromClient when provided", async () => {
89+
getAddressQueryMock.mockResolvedValue({
90+
success: true,
91+
value: { isSome: true, value: liveAddress },
92+
});
93+
94+
const assetHub = {} as any;
95+
const origin = "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY";
96+
await resolveLiveContractAddresses(assetHub, [PLAYGROUND_REGISTRY_CONTRACT], {
97+
defaultOrigin: origin,
98+
});
99+
100+
expect(createContractFromClientMock).toHaveBeenCalledWith(
101+
assetHub,
102+
CDM_REGISTRY_ADDRESS,
103+
expect.any(Array),
104+
expect.objectContaining({ defaultOrigin: origin }),
105+
);
106+
});
107+
87108
it("omits libraries when the live registry has no address", async () => {
88109
getAddressQueryMock.mockResolvedValue({
89110
success: true,
@@ -136,4 +157,47 @@ describe("withLiveContractAddresses", () => {
136157
]),
137158
).rejects.toThrow(/CDM meta-registry did not return live address/);
138159
});
160+
161+
it("forwards defaultOrigin through withLiveContractAddresses", async () => {
162+
getAddressQueryMock.mockResolvedValue({
163+
success: true,
164+
value: { isSome: true, value: liveAddress },
165+
});
166+
167+
const assetHub = {} as any;
168+
const origin = "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY";
169+
await withLiveContractAddresses(manifest(), assetHub, [PLAYGROUND_REGISTRY_CONTRACT], {
170+
defaultOrigin: origin,
171+
});
172+
173+
expect(createContractFromClientMock).toHaveBeenCalledWith(
174+
assetHub,
175+
CDM_REGISTRY_ADDRESS,
176+
expect.any(Array),
177+
expect.objectContaining({ defaultOrigin: origin }),
178+
);
179+
});
180+
181+
it("forwards defaultOrigin through withRequiredLiveContractAddresses", async () => {
182+
getAddressQueryMock.mockResolvedValue({
183+
success: true,
184+
value: { isSome: true, value: liveAddress },
185+
});
186+
187+
const assetHub = {} as any;
188+
const origin = "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY";
189+
await withRequiredLiveContractAddresses(
190+
manifest(),
191+
assetHub,
192+
[PLAYGROUND_REGISTRY_CONTRACT],
193+
{ defaultOrigin: origin },
194+
);
195+
196+
expect(createContractFromClientMock).toHaveBeenCalledWith(
197+
assetHub,
198+
CDM_REGISTRY_ADDRESS,
199+
expect.any(Array),
200+
expect.objectContaining({ defaultOrigin: origin }),
201+
);
202+
});
139203
});

src/utils/contractManifest.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,27 @@ function patchContractAddresses(
102102
return patched;
103103
}
104104

105+
/**
106+
* Options for the meta-registry lookup. `defaultOrigin` is forwarded to the
107+
* underlying contract handle so the read-only `getAddress` dry-run uses the
108+
* caller's logged-in account instead of the dev fallback (Alice). Without it,
109+
* `@polkadot-apps/contracts` emits a misleading `[contracts] No origin
110+
* configured` warning even when the user has signed in via `dot init`.
111+
*/
112+
export interface LiveContractLookupOptions {
113+
defaultOrigin?: string;
114+
}
115+
105116
export async function resolveLiveContractAddresses(
106117
assetHub: PolkadotClient,
107118
libraries: readonly string[] = LIVE_CONTRACTS,
119+
options: LiveContractLookupOptions = {},
108120
): Promise<Record<string, HexString>> {
109121
const registry = await createContractFromClient(
110122
assetHub,
111123
CDM_REGISTRY_ADDRESS,
112124
CDM_REGISTRY_ABI,
125+
{ defaultOrigin: options.defaultOrigin },
113126
);
114127
const entries = await withoutReviveTraceNoise(() =>
115128
Promise.all(
@@ -133,17 +146,19 @@ export async function withLiveContractAddresses(
133146
manifest: CdmJson,
134147
assetHub: PolkadotClient,
135148
libraries: readonly string[] = LIVE_CONTRACTS,
149+
options: LiveContractLookupOptions = {},
136150
): Promise<CdmJson> {
137-
const liveAddresses = await resolveLiveContractAddresses(assetHub, libraries);
151+
const liveAddresses = await resolveLiveContractAddresses(assetHub, libraries, options);
138152
return patchContractAddresses(manifest, liveAddresses);
139153
}
140154

141155
export async function withRequiredLiveContractAddresses(
142156
manifest: CdmJson,
143157
assetHub: PolkadotClient,
144158
libraries: readonly string[] = LIVE_CONTRACTS,
159+
options: LiveContractLookupOptions = {},
145160
): Promise<CdmJson> {
146-
const liveAddresses = await resolveLiveContractAddresses(assetHub, libraries);
161+
const liveAddresses = await resolveLiveContractAddresses(assetHub, libraries, options);
147162
const missing = libraries.filter((library) => !liveAddresses[library]);
148163
if (missing.length > 0) {
149164
throw new Error(`CDM meta-registry did not return live address for ${missing.join(", ")}`);

src/utils/registry.test.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,12 @@ describe("getRegistryContract", () => {
4848

4949
await getRegistryContract(rawClient, fakeSigner);
5050

51-
expect(withRequiredLiveContractAddressesMock).toHaveBeenCalledWith(cdmJson, rawClient, [
52-
"@w3s/playground-registry",
53-
]);
51+
expect(withRequiredLiveContractAddressesMock).toHaveBeenCalledWith(
52+
cdmJson,
53+
rawClient,
54+
["@w3s/playground-registry"],
55+
{ defaultOrigin: fakeSigner.address },
56+
);
5457
expect(fromClientMock).toHaveBeenCalledWith(patchedManifest, rawClient, {
5558
defaultSigner: fakeSigner.signer,
5659
defaultOrigin: fakeSigner.address,

src/utils/registry.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,12 @@ export async function getRegistryContract(
2121
) {
2222
let manifest: CdmJson;
2323
try {
24-
manifest = await withRequiredLiveContractAddresses(cdmJson, rawClient, [
25-
PLAYGROUND_REGISTRY_CONTRACT,
26-
]);
24+
manifest = await withRequiredLiveContractAddresses(
25+
cdmJson,
26+
rawClient,
27+
[PLAYGROUND_REGISTRY_CONTRACT],
28+
{ defaultOrigin: signer.address },
29+
);
2730
} catch (err) {
2831
const msg = err instanceof Error ? err.message : String(err);
2932
throw new Error(

0 commit comments

Comments
 (0)