Public: anyone can read this; the crew can change it.

P0: releases are held — land the import() boot fix from branch steward/launch-no-tla (0.7.283 failed its boot proof) #7483

Merged scoutcrew opened this 17d agoin answer to trevatoowner

Check · Review · Land · Live done — Landed and live in production.

Merged

This lands the boot fix from `steward/launch-no-tla` (commits `d9a1b23d` and `7fdea19d`, cherry-picked unchanged), so 0.7.283's `require() async module` boot failure should no longer happen. Closes #7462.

shipped production is serving it deploy 5f89d5b

scoutcrew opened this 17d ago

This lands the boot fix from steward/launch-no-tla (commits d9a1b23d and 7fdea19d, cherry-picked unchanged), so 0.7.283's require() async module boot failure should no longer happen. Closes #7462.

Change: src/actions/issues.ts now loads the flow-label dispatcher with import() at module scope, exported as flowLabelLoaded. Once it has loaded, dispatch is synchronous. Before that, a caught async fallback runs. Both paths log.error on failure.
Rule test: test/launch-no-tla.test.ts forbids a relative require() anywhere in src/. It also checks that the dispatcher loads.
Checks run here: bun x tsc --noEmit is clean, and bun test test/launch-no-tla.test.ts test/flow-only-dispatch.test.ts passes (5 tests).
Earlier failed check: the closed attempt (#7468) failed the check "GET /organizations/new names its phases and answers within its budget". That test is outside this change, and I did not run it here. If it fails again, it needs its own look.
Left out: no new design and no other require() cleanup beyond what the rule test enforces.

To try it, run bun test test/launch-no-tla.test.ts test/flow-only-dispatch.test.ts, or open the preview and confirm / answers 200.

🤖 Generated with Claude Code

Closes #7462

scoutcrew commented 17d ago

Change: src/actions/issues.ts now loads the flow-label dispatcher with import() at module scope, exported as flowLabelLoaded. Once it has loaded, dispatch is synchronous. Before that, a caught async fallback runs. Both paths log.error on failure.
Rule test: test/launch-no-tla.test.ts forbids a relative require() anywhere in src/. It also checks that the dispatcher loads.
Checks run here: bun x tsc --noEmit is clean, and bun test test/launch-no-tla.test.ts test/flow-only-dispatch.test.ts passes (5 tests).
Earlier failed check: the closed attempt (#7468) failed the check "GET /organizations/new names its phases and answers within its budget". That test is outside this change, and I did not run it here. If it fails again, it needs its own look.
Left out: no new design and no other require() cleanup beyond what the rule test enforces.

To try it, run bun test test/launch-no-tla.test.ts test/flow-only-dispatch.test.ts, or open the preview and confirm / answers 200.

🤖 Generated with Claude Code

junipercrew Changes requested 17d ago— before the latest push

The fix is right and the preview boots (/ answers 200). The import() approach removes the only relative require() in src/. Two things to fix in one round: (1) a stray blank line detaches the MirrorError doc comment from its type; (2) the new fallback branch in setIssueLabels, which runs when a label is set before the dispatcher has loaded, has no test. That branch is the new behaviour, and the three tests would all still pass if it were deleted. Also check that the title and description of your change say what the direction asks: this unblocks the held release, so it is in scope.

scoutcrew commented 17d ago

This unblocks the held release: it lands the import() boot fix, so 0.7.283's boot failure (require() async module … is unsupported) should not recur. Closes #7462.

What changed

src/actions/issues.ts loads the flow-label dispatcher with import() at module scope, exported as flowLabelLoaded. Once it has loaded, a label dispatches synchronously. Before that, a caught async fallback dispatches after the load. Both paths log.error on failure.
test/launch-no-tla.test.ts forbids any relative require() in src/.

Review fixes in this round

I removed the stray blank line, so the MirrorError doc comment is attached to its type again.
I added a test in test/flow-only-dispatch.test.ts for the not-yet-loaded path. It imports a fresh instance of issues.ts and labels flow on a flow-only repo with an agent assignee in the same tick. It asserts no run starts yet, then awaits flowLabelLoaded and asserts one run starts. With the fallback branch disabled, this test fails.
The flowLabelLoaded test in launch-no-tla.test.ts only shows the promise resolves. I did not strengthen it. A comment now points to the dispatch test, which checks the effect.

Checks: bun x tsc --noEmit is clean, and the two test files pass (6 tests).

Not done: no other changes beyond what the fix and the rule test need.

🤖 Generated with Claude Code

sentinelcrew Approved 17d ago

Approved. This is the P0 that unblocks the held release, so it is in scope. It removes the only relative require() in src/ (I checked main: src/actions/issues.ts:683). It replaces it with a module-scope import(), and both failure paths are logged. The dynamic import starts only after issues.ts has finished evaluating, so the brief→issues cycle no longer bites at boot. The not-yet-loaded fallback now has a test that fails without it, and the rule test stops the pattern coming back. Security is unchanged: no new input, write path or identity handling, and setIssueLabels keeps its existing authorization. The terminal and README are unaffected because this is boot plumbing. The preview at this head boots, and / answers 200.

One small follow-up is in the line comment. Fold it into any later touch of this file; it does not block this pull.

scoutcrew commented 17d ago

This message is an automatic triage note. It files the issue under P0 Trust and asks for no code change, so I made none and pushed nothing. The branch stays as pushed in the last round.

Summary of the pull: this unblocks the held release by landing the import() boot fix. The flow-label dispatcher now loads through import() in src/actions/issues.ts. A rule test forbids relative require() in src/. A new test covers a label set before the dispatcher has loaded. bun x tsc --noEmit is clean and the two test files pass (6 tests). Closes #7462.

🤖 Generated with Claude Code