Skip to content

fix(cli): scaffold output survives a clean clone and a non-interactive run - #983

Merged
rickylabs merged 4 commits into
mainfrom
fix/scaffold-hygiene
Jul 31, 2026
Merged

fix(cli): scaffold output survives a clean clone and a non-interactive run#983
rickylabs merged 4 commits into
mainfrom
fix/scaffold-hygiene

Conversation

@rickylabs

Copy link
Copy Markdown
Owner

Summary

Grouped beta.12 fix: everything netscript init emits should survive a clean clone and a
non-interactive run.

Four issues, one verification path — scaffold a project without a TTY, clone it clean, start it:

Status

Draft. Implementation is delegated to a Codex slice on this branch; the run dir is
.llm/runs/fix-scaffold-hygiene--966/. This PR body will be filled in with root cause, the
regression guard and its fails-before evidence, and scoped gate output before it leaves draft.

Closes #966
Closes #975
Closes #967
Closes #968

@rickylabs

Copy link
Copy Markdown
Owner Author

IMPL-EVAL verdict — BLOCKED, staying in draft

The implementation slice wrote a coherent fix but never committed or pushed it. origin/fix/scaffold-hygiene is at 361a5e767, identical to local HEAD; both commits on this branch are run artefacts (context-pack.md, implement.md). git diff origin/main...HEAD touches zero source files. All 13 modified + 4 untracked files live only in the worktree /home/codex/repos/b12-scaffoldhygiene.

Cause: the pre-commit asset-barrel check exited 1 over the intended packages/cli/src/kernel/assets/embedded.generated.ts diff, and the agent turn ended without retrying. The work is on disk and looks correct — it is one regenerate-and-commit away — but nothing is on the remote, so it is one git checkout . from being lost.

The four evaluator checks

1. Gate evidence covers the changed files — CANNOT BE ASSESSED. There is no committed diff to gate. Every PASS in the implementation report is Codex's self-report from worklog.md, not an observed run against a pushed tree. The one claim I did re-run independently, the focused regression suite, gives 24 passed / 36 steps, not the reported 25 / 37 — a small but real discrepancy in the self-report. asset_barrel_check is NOT RUN (it is the blocker) and scaffold.runtime E2E is NOT RUN.

2. Root cause vs symptom — the grouping hypothesis DID NOT HOLD. This group existed on the belief that four issues shared one cause. They do not. The slice landed four independent patches in four unrelated places:

Codex was explicit that it rejected the single-cause hypothesis, naming two contract clusters instead (generated-artifact contract, invocation contract). I agree with the finding, and it is honest work — but it should be recorded plainly: this is four symptom-local patches, not one root-cause fix. #966 and #975 are genuinely adjacent (both are "the scaffold's contract disagrees with what it emits"); #967 and #968 share nothing with them or with each other beyond touching init.

3. The issue was wrong — CONFIRMED, and the correction is properly posted. #968's filed cause was stale: main has bypassed init prompts on non-TTY stdin since beta.5. The correction is a real comment on the issue itself (issues/968#issuecomment-5144895872), not buried in a PR body. This is the one check that fully passes. Consequence worth flagging: what remains of #968 is a discoverability change, not a defect fix — it adds a --non-interactive spelling for behaviour that already worked.

4. Regression guard fails-before — PARTIAL, and #968 has no real guard. Guards exist for #966 (plan-init_test.ts), #967 and #968 (init-command_test.ts), #975 (config_test.ts, schema_test.ts). Codex's own pre-implementation run reports 19 passed / 5 failed, which is credible fails-before evidence for #966, #967 and #975. But the #968 guard PASSED before the fix — by definition it is not a regression guard for the filed defect; it is a characterisation test for behaviour that was already correct. I did not run the revert-and-confirm-fails loop myself, because there is no committed hunk to revert.

What Eric must decide

The recovery is cheap and the diff on disk is good. Recommended: resume Codex thread 019fb8e4-f036-7290-bff2-0d140ec053e2 in the worktree with a single instruction — regenerate the asset barrel, commit, push, verify with git ls-remote. Do not relaunch the implementation; a fresh launch assumes a clean tree and would discard the work.

Two things to weigh at merge time:

Branch is also 1 commit behind origin/main (37047e283) and will need a rebase before merge.

PR stays draft and keeps status:in-progress until the implementation is actually on the remote.

…eractive run

Closes #966, #967, #968, #975.

The Codex slice completed this work but could not commit it: `check:assets-barrel` runs
`gen:assets-barrel && git diff --exit-code`, so an intended generated diff makes the gate fail
until it is committed. Regenerated the barrel and landed it.

- #966 the generated `.gitignore` no longer excludes `appsettings.json`, which a clean clone
  needs. The template change flows into `embedded.generated.ts` via the assets barrel.
- #975 `Parameters` becomes a declared top-level key in the appsettings schema, so the block the
  scaffold already writes stops being rejected.
- #967 `init` no longer nests a project directory when the cwd is already the target.
- #968 the filed cause was stale — `main` has bypassed prompts for a non-terminal stdin since
  beta.5. The real gap was the missing `--non-interactive` spelling and the absence of a
  non-terminal regression guard; both added, and the issue corrected in a comment.

One pre-existing test changed deliberately. `resolve-appsettings-path_test.ts` asserted that
`Parameters.postgres-password` resolves as `unknown` — true only while the schema lacked
`Parameters`, which is precisely the #975 defect. That test carried two assertions and only the
status was stale: the one that mattered (a top-level key is not silently re-homed under
`NetScript`) is unchanged. Rather than widen the schema and quietly delete the coverage, the
undeclared-key case is kept as its own test against a key the schema really does not declare.
Only conflict was an import line in validate-init.ts: this branch added `basename` for #967's
cwd-is-already-the-target check, while #978 added `USER_PORT_RANGE` for the widened host-port
validation. Both are used; the resolution is the union.
@rickylabs

Copy link
Copy Markdown
Owner Author

Supervisor recovery + verification

The implementation was complete but stranded in the worktree — 13 modified and 4 untracked
files, never committed. The cause is in the slice's own worklog: check:assets-barrel runs
gen:assets-barrel && git diff --exit-code, so the intended generated diff in
embedded.generated.ts (from #966's .gitignore template change) made the gate exit 1, and the
turn ended there. Regenerated the barrel, committed, pushed — the gate passes now that the diff is
committed, which confirms the diagnosis rather than working around it.

Also merged origin/main (the branch was behind 37047e283). One conflict, in
validate-init.ts: this branch added basename for #967's cwd-is-already-the-target check while
#978 added USER_PORT_RANGE for widened host-port validation. Both are used; resolution is the
union.

One pre-existing test changed deliberately

resolve-appsettings-path_test.ts asserted Parameters.postgres-password resolves as unknown
true only because of the #975 defect. That test carried two assertions and only the status was
stale; the one that mattered (a top-level key is not silently re-homed under NetScript) is
untouched. The undeclared-key case is preserved as its own test against a key the schema genuinely
does not declare, so widening the schema did not quietly retire the guard that the widening was
scoped.

Gates — scoped, because root lint/fmt:check exclude packages/cli

Gate Result
Tests (packages/cli + packages/aspire) 497 passed / 518 steps, 0 failed
Lint (scoped to both roots) 0 occurrences
Format (scoped to both roots) 0 findings
check:assets-barrel PASS (clean once the generated diff is committed)
CI on 382d2795 all green, incl. scaffold-runtime and close-gate

close-gate passes because #975's two acceptance criteria were verified against the diff and ticked
with evidence on the issue — not ticked to make the gate go quiet.

Two things for Eric, both honest limitations

  1. The grouping hypothesis did not hold here. These are four symptom-local patches in four
    unrelated places (gitignore template, AppSettingsZod, target resolution in validate-init.ts,
    a --non-interactive alias) — not one shared-cause fix. Only fix(scaffold): generated .gitignore excludes appsettings.json, which a clean clone needs #966 and fix(aspire): scaffold writes a top-level 'Parameters' block the appsettings schema drops #975 are genuinely
    adjacent. Worth recording, because the same grouping did pay off on the plugin cluster (fix(plugin): generated plugin glue reaches its runtime #988),
    so this is evidence about which groupings are real rather than a failure of the method.
  2. fix(cli): non-interactive invocations still hit selection prompts #968's regression guard is a characterisation test, not a guard — it passed against the
    broken code. Its filed cause was stale (main has bypassed non-TTY prompts since beta.5,
    corrected at fix(cli): non-interactive invocations still hit selection prompts #968#issuecomment-5144895872), so what remained was the missing --non-interactive
    spelling. Whether fix(cli): non-interactive invocations still hit selection prompts #968 should stay in this PR's Closes list is your call — closing it via
    the correction alone may be more honest than closing it via a flag alias.

Not merged.

@rickylabs
rickylabs marked this pull request as ready for review July 31, 2026 17:27
@augmentcode

augmentcode Bot commented Jul 31, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: Hardens the netscript init scaffold output so a clean clone (including non-interactive/CI runs) can start reliably.

Changes:

Technical Notes: Non-interactive resolution is centralized by mapping --non-interactive to ci before calling the interactive input resolver; schema changes flow through to generated JSON Schema tests.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

await scaffoldRoot(context(scaffolder), options());

const gitignore = scaffolder.files.get('/workspace/deploy-app/.gitignore') ?? '';
assertEquals(gitignore.split(/\r?\n/).includes('appsettings.json'), false);

@augmentcode augmentcode Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

packages/cli/src/kernel/application/scaffold/plan-init_test.ts:191 — This assertion only checks for an exact appsettings.json line, so it would miss ignore patterns like /appsettings.json or **/appsettings.json and may not reliably guard #966 regressions.

Severity: low

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.


await t.step('contains host-side Aspire parameters in generated schema', () => {
const schema = generateAppSettingsJsonSchema();
assertEquals(JSON.stringify(schema).includes('Parameters'), true);

@augmentcode augmentcode Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

packages/aspire/tests/schema_test.ts:39 — JSON.stringify(schema).includes('Parameters') can pass even if Parameters isn’t actually a top-level property (e.g., the word appears in a description/$defs), so this guard may be weaker than intended.

Severity: low

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

@rickylabs
rickylabs merged commit a9d9bc0 into main Jul 31, 2026
22 of 26 checks passed
@rickylabs
rickylabs deleted the fix/scaffold-hygiene branch July 31, 2026 17:58
rickylabs added a commit that referenced this pull request Jul 31, 2026
Only conflict was packages/cli/src/kernel/assets/embedded.generated.ts — a generated barrel, so it
was regenerated with `gen:assets-barrel` rather than hand-merged. Three of the eight PRs merged
ahead of this one (#982 aspire codegen, #983 scaffold hygiene, and this branch) all rewrite that
file, which is exactly the sort of file a textual three-way merge gets wrong.

The .mjs -> .mts change from #982 composed cleanly with this branch's generator assertions.

Composed verification: 504 tests pass, and scoped lint/fmt/check over packages/cli + packages/aspire
are all clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

1 participant