Skip to content

Fixes from the real-app upgrades, and from reviewing them - #11

Merged
hazeliscoding merged 23 commits into
mainfrom
fix/real-app-findings
Nov 8, 2026
Merged

hazeliscoding merged 23 commits into
mainfrom
fix/real-app-findings

Conversation

@hazeliscoding

Copy link
Copy Markdown
Owner

Summary

Fixes from the two real-app upgrades (pr-sweep #4 and readme-wizard #1) and their reviews, then fixes from an adversarial review of this branch.

  • The app check never saw dev-server errors once the server was ready. It read a copy of the log taken at that moment, and its patterns didn't know Vite's wording, so readme-wizard's Failed to resolve import "marked-katex-extension" hid behind a page that rendered nothing. The log is read live now, Vite's lines count, a failed page with no error line shows the server's last lines, and a failure points to the troubleshooting guide. It also passes PORT, which Angular 22's dev server takes over --port.
  • The plan says more where ng update and the plan differ. A shared package's line gives the ranges that fit, since ng update installs the newest release in them, and that newest release's license is checked too. Moved libraries and shared packages name their repository, where the release notes are when a package ships none. A peer two libraries newly need is picked once, to fit both.
  • Line endings: eol.mjs puts back each changed file's endings and final newline as the repo commits them, following .gitattributes and not the machine's core.autocrlf, so a dev container on the same files sees only the real change. Both real runs had written their own.
  • The skill's text: the branch is named after the plan; no unit test running stops the run to ask; a failed check is looked up in the troubleshooting guide first; a migration that reformats whole files is redone straight after ng update; the report keeps a Library changes line and lists only the optional migrations that would change the app.
  • Evals: the scripted upgrades keep a version ng update chose inside the plan's range. readme-wizard's real case answers the no-test question in its prompt, and a new case grades the stop itself.
  • Docs: the roadmap records what the real apps found, M6 as met, two decisions, and M7, an enterprise upgrade on video.

What the review of this branch found

Four reviewers (scripts, skill text, tests, safety), then a skeptic for each serious finding. Seven held, and each is fixed here:

Finding Fix
With Git for Windows' core.autocrlf=true, eol.mjs rewrote a dev container's LF files to CRLF HEAD's form is read with core.autocrlf off
The repository address let shell syntax into a link the agent is told to visit Only URL-safe characters, up to 200
A second library's range was added to a new peer without picking it again Ranges are collected, then the peer is picked once
Shared packages named no repository They do now
The control-flow rerun came after the hand fixes and checks It's done straight after ng update, before any hand fix
readme-wizard's eval contradicted the new no-test rule Its prompt answers the question, and a new case grades the stop
No test covered eol.mjs's checkout conversion Tests for core.autocrlf and .gitattributes

The reviewers also caught that keeping ng update's in-range version skipped that version's license check, now checked, and that eol.mjs could write through a linked folder, now skipped. One finding was refuted: in 75 trial runs, agents created the branch before committing on the patch-only path. The bullet now says so anyway.

Checked

  • npm test: 249 tests, 240 pass and 9 POSIX-only tests skip on Windows.
  • The new app-check and line-ending tests fail on the old code or on mutants of the fix. The Vite test also fails with the new patterns but the old copy of the log, so both fixes were needed.
  • Agent evals on this branch, before the review's fixes: yarn and bun 16/16 with the skill and 11/15 without; the pnpm catalog 14/14 with the skill, once fix(evals): place a pnpm catalog's commits on their Angular major #10 fixed how its commits are graded.

From Angular 22, @angular/build's dev server takes PORT over --port. A PORT left in the shell or a container moved the server, so the app check fetched an empty port and failed at exactly the major where the behaviour changed.
The check for late errors read a copy of the log taken when the server became ready, so it never saw anything written after, and its patterns didn't know Vite's wording. On readme-wizard's Angular 22 hop that hid Failed to resolve import "marked-katex-extension" behind a page that rendered nothing. The log is now read live, Vite's lines count, a failed page with no error line shows the server's last lines, and a failure points to the troubleshooting guide.
For a shared package the plan picks the smallest move, while ng update takes the newest release in the peer's range: TypeScript 5.9.3 where the plan said 5.8.3, marked 18 where it said 17. Each line now gives the ranges that fit, so a version ng update already chose inside them isn't taken for a mistake. The scripted upgrades keep it too, instead of going back to the plan's pick.
Many packages ship no changelog, so 'read it in node_modules' found nothing: on readme-wizard no changelog was read across four majors of NgRx, ngx-markdown and marked. Each library that moves now names its repository, where the release notes are. Only an https address is kept.
Both real apps needed this, and each run wrote its own script. readme-wizard's left package.json without its final newline, and rebuilt 18 migrated files by hand. eol.mjs restores each changed file's endings and final newline as git checks it out, following .gitattributes. The guide now says it happens on any OS, keeps migration output as written, and gives Angular 21's control-flow migration its --format=false rerun.
The branch was named for the target before the plan ran, so a run the plan stops short named its branch for a major it never reached.
On pr-sweep, ng test failed with Unknown argument: watch, because the app has no test target, and only the agent's own initiative found the project's real checks. On readme-wizard, one spec that didn't compile stopped Karma before any spec ran, at the start and after every major, and the report called it one failing test. Both now have a rule, and the report lists a check that couldn't run.
… update chose in range

NgRx, ngx-markdown and marked ship no changelog in their packages, so on readme-wizard nothing was read across four majors and the report had no library changes. The plan now names each moved library's repository, and the report never leaves that line out. A shared package ng update put inside the plan's range stays, and a library it took further than the plan is reported.
The guide was only pointed to for ng update's output. On readme-wizard, both later failures were already in it, and one fix went against it. A failed check now goes there first, and the dev-server entry quotes Vite's wording, its symptom, and Angular 22's PORT.
…e app

On pr-sweep the report listed every optional migration any of four majors offered: three would change nothing in the app, and one had already run as a required migration. It now lists what the last major still offers and would change, the settings migrations added to keep the old behaviour, and says an unused deprecated package only needs removing.
Some of Angular's update steps apply to @angular/build only, such as Angular 22's PORT change, and others to the older webpack builder.
Both app upgrades are open for review, and the skill fixes they called for are ticked under M5, with one left. OpenCode 2.2 took the test app from Angular 20 to 22 on Windows and passed all 17 checks, so the README's agent table says so. Two decisions came out of the runs: keep the version ng update picks inside a peer range, and stop when no unit test can run.
An Angular 21 app on pnpm whose internal UI libraries have to reach 22 first, set up for a dev container and upgraded from the Windows host, then turned into a video tutorial.
A second library's range was added to a peer the first had already picked for, without picking again, so the plan could name a version the second rejects next to a 'fits' that included its range. The pick depended on the libraries' alphabetical order. Each new peer now collects every range first and is picked once; when nothing fits all of them, it's listed as unchecked.
The address comes from the package's author, and an agent may paste it into a shell, but \S+ let $(...), quotes, control characters and any length through. Only URL-safe characters, up to 200, are kept now.
…release that fits

Shared packages are the ones ng update can take past the plan's pick, as with marked 18 on readme-wizard, yet their lines named no repository for release notes. They do now. And since the skill keeps what ng update chose inside the range, the plan also compares the license of the newest release that fits, not only its own pick.
On an app already on Angular 22, --next planned 22.3.0-next.4 under 'Only patch releases are available'.
…y inside the repo

Git for Windows turns core.autocrlf on, so eol.mjs rewrote LF files to CRLF from a Windows host. The host's git couldn't see it, and a dev container's git on the same files saw every line changed: M7's exact setup. HEAD's form is now read with core.autocrlf off, so .gitattributes and the endings the repo commits decide. A file reached through a linked folder is no longer written, a file it can't read or write is reported and skipped, and outside a repo it says what it needs in one line.
…ins a failure

A console error alone already says what went wrong, so it no longer adds a dozen startup lines. Tests now cover the text report's server log lines, its pointer to the troubleshooting guide, and Vite's Internal server error.
…t run

readme-wizard's suite can't compile on main, so the skill now stops at the start to ask, which a headless run can't answer, and the full-upgrade case would score a compliant run below one that ignored the rule. Its prompt now answers ahead of time, and the report has to say no unit test ran. A new case, real-readme-wizard-no-tests, grades the stop itself.
… after

The control-flow rerun came at step g, after the hand fixes and checks, so restoring templates from the last commit could throw away hand fixes and other migrations' edits, and the result was committed unchecked. It now happens straight after ng update, only for files whose whole change is the conversion, and the run carries on from step e, so the checks and eol.mjs cover it. Step g no longer restores or reruns anything.
The patch-only path creates the branch first, and the branch is named for the last major the run will commit, as far as a license change or Node allows. With no test target, the project's own test command runs through npm or npx --no-install, never a deploy or publish step. Fixing one broken spec first is the one exception the person can approve, and the scope rule says so. Step e points at the plan from step a. The guide quotes Angular 22's PORT line and says what else Karma's load error can mean. A test pins the no-test stop.
M6's done-when is met: the catalog, yarn and bun cases pass with the skill, --next planned Angular's real 22.3.0-next.4, and the app check fails on a plain console.error. M7 records its audience and two decisions: the internal libraries come from a private registry, and the video is built with HyperFrames from the real run. The demo has to be recorded again for the plan's new lines, the no-test item now says what the skill really does, and the in-range decision notes the license check on the newest release.
@hazeliscoding
hazeliscoding merged commit 6ff918f into main Nov 8, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant