Skip to content

chore: salvage the unique work from embeddedos-org before archiving it - #159

Open
srpatcha wants to merge 5 commits into
masterfrom
salvage/port-unique-work-from-embeddedos-org
Open

chore: salvage the unique work from embeddedos-org before archiving it#159
srpatcha wants to merge 5 commits into
masterfrom
salvage/port-unique-work-from-embeddedos-org

Conversation

@srpatcha

@srpatcha srpatcha commented Sep 2, 2026

Copy link
Copy Markdown
Member

Step 2 of #158. Makes embeddedos-org safe to archive.

Why

Two near-identical website repositories, no common git ancestor. This one
serves the live site. The other serves a 404:

$ gh api repos/embeddedos-org/embeddedos-org/pages
status: 404   url: None   source: None

$ curl https://embeddedos-org.github.io/embeddedos-org/
HTTP 404

and it has kept receiving real work regardless — an SEO canonical fix, a
duplicate-element bug fix, a null-dereference guard, all as recently as
2026-08-30, applied to a site nobody can load.

What is ported

Six files that exist there and not here:

file
scripts/build-deploy.py HTML/CSS/JS minification
.github/workflows/health-check.yml weekly uptime check
.github/workflows/lint-readme.yml README lint on push
.github/dependabot.yml dependency updates
tests/simulation/test_mobile_full_simulation.py mobile coverage
package-lock.json pinned dependency tree

Plus removal of style.css.bak, an orphan the dead repo had already cleaned and
this one still carried.

build-deploy.py had to be fixed before it could be taken

It operated in place. It minified the tracked sources over themselves, then:

for d in ['tests', 'test-screenshots', '.github']:
    p = ROOT / d
    if p.exists(): shutil.rmtree(p)

deleted the CI configuration and the test suite from the working tree. It is
named "build for the deploy branch" and never made a branch — it mutated whatever
checkout it was run in.

I found this the direct way: I ran it, and it took .github/ and tests/ with
it. Had it been merged as-is and run by anyone in a clone, the same thing would
have happened to them.

It now stages a copy into dist/ and minifies that. A build step must not be
able to damage the thing it is building from.

Verified

working tree hash before=84e3fd378a71 after=84e3fd378a71   UNCHANGED
.github and tests survive the run

dist/ 97 files
  tests    correctly excluded
  .github  correctly excluded
  scripts  correctly excluded

index.html  96,362 -> 92,410 bytes (4.1% smaller)
  <title>          retained
  rel="canonical"  retained
  </html>          retained

32 files minified, 32,179 bytes saved

dist/ was already in .gitignore.

After this merges

embeddedos-org can be archived — not deleted, so its 29 commits stay readable.
That is step 3 of #158; steps 1 (get the production deploy into version control)
and 4 (set the CNAMEs) are independent and, in the case of step 1, more urgent
than either.

Fixes #158

The organisation has two near-identical website repositories with no common
git ancestor. This one serves the live site; embeddedos-org serves a 404:

    $ gh api repos/embeddedos-org/embeddedos-org/pages
    status: 404   url: None   source: None

and it has kept receiving real work regardless — an SEO canonical fix, a
duplicate-element bug fix and a null-dereference guard, all as recently as
2026-08-30, applied to a site nobody can load.

Six files exist there and not here. Ported so that repository can be
archived without losing them:

    .github/dependabot.yml
    .github/workflows/health-check.yml       weekly uptime check
    .github/workflows/lint-readme.yml        README lint on push
    tests/simulation/test_mobile_full_simulation.py
    package-lock.json
    scripts/build-deploy.py                  HTML/CSS/JS minification

Also removes style.css.bak, an orphan that the dead repository had already
cleaned up and this one still carried.

build-deploy.py needed fixing before it could be taken. It operated in
place: it minified the tracked sources over themselves and then

    for d in ['tests', 'test-screenshots', '.github']:
        p = ROOT / d
        if p.exists(): shutil.rmtree(p)

deleted the CI configuration and the test suite from the working tree. It
is named "build for the deploy branch" and never made a branch — it mutated
whatever checkout it was run in. I found this by running it, which cost me
the .github and tests directories.

It now stages a copy into dist/ and minifies that, so the source tree is
never written to. A build step must not be able to damage the thing it is
building from.

Verified:

    working tree hash before=84e3fd378a71 after=84e3fd378a71  UNCHANGED
    .github and tests survive the run
    dist/ 97 files; tests, .github and scripts correctly excluded
    index.html 96,362 -> 92,410 bytes, title/canonical/</html> intact
    32 files minified, 32,179 bytes saved

dist/ was already in .gitignore.

Refs #158

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@srpatcha srpatcha left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Review — embeddedos-org.github.io#159 "chore: salvage the unique work from embeddedos-org before archiving it"

head: a6a538b author: srpatcha ci: fail

Verdict: The build-deploy.py rewrite is the right call and the in-place destruction
is genuinely gone — I ran it and the source tree is untouched. But the script it becomes
emits JavaScript that does not parse: 4 of the 5 JS files in dist/ fail node --check,
including all three of the site's own. Two of the other five ported files do not work in
this repo either — the lockfile is out of sync with this package.json, and the Python
test is executed by nothing.

Findings

# Severity File:line Finding Recommended fix
1 High scripts/build-deploy.py:29 minify_js strips comments with re.sub(r'//[^\n]*', '', text), which is not comment-aware: it truncates every line at the first //, including inside string literals. Any line containing a URL loses everything from https: onward, leaving an unterminated string. dist/js/site-chrome.js becomes { href: 'https:. 4 of 5 emitted JS files fail node --check; the same 4 sources pass. The site's nav bar, search and chat widget would all be dead in a deployed dist/. Do not hand-roll this. terser and esbuild are both single dependencies and both already correct. If a dependency is unacceptable, drop minify_js entirely and ship the JS unminified — 32 KB saved across the whole tree is not worth shipping a broken bundle.
2 High package-lock.json The lockfile was ported from a repo whose package.json declared html-validate: ^8.0.0; this repo declares ^11.10.0. npm ci fails outright. The three workflows that run npm install instead (pr-check.yml:22, deploy.yml:25, nightly.yml:24) will silently rewrite it, so the "pinned dependency tree" the PR adds is not pinned anywhere it is used and is a hard failure in ci.yml:24/:56, which do use npm ci. Regenerate it in this repo: rm package-lock.json && npm install, commit the result. Do not port a lockfile between repos with different manifests.
3 Medium tests/simulation/test_mobile_full_simulation.py Nothing runs it. The only Python in CI is check-canon.yml:37 (python scripts/check-product-canon.py); no workflow invokes pytest, and pr-check.yml:35's npx playwright test tests/ collects *.spec.js only. The PR table lists this file as "mobile coverage" — it is a file, not coverage. Eight other Python suites already under tests/ are dark for the same reason. Either add a job that runs it (with a python -m pytest tests/ step and the server on the right port), or say in the PR that it is ported for preservation and not yet wired up. §28 does not let a file stand in for a test report.
4 Medium tests/simulation/test_mobile_full_simulation.py:14-20, 23 Two import-time side effects. The except ImportError block runs pip install playwright and playwright install chromium --with-deps — the latter shells out to the system package manager — so merely importing this module mutates the machine. And line 23 does SCREENSHOTS_DIR.mkdir(parents=True, exist_ok=True) at module level, creating test-screenshots/mobile/ on collection alone. It also hardcodes BASE_URL = "http://localhost:8777" while every other harness in this repo uses 8080 (package.json:7, pr-check.yml:33), so if it were wired up it would target a port nothing serves. Move the playwright import failure to a pytest.importorskip, move the mkdir into the fixture or function that writes screenshots, and read the base URL from BASE_URL with 8080 as the default.
5 Medium .github/dependabot.yml This adds Dependabot to a repo that has never had it, while embeddedos-org#7 — same author, open right now — deletes .github/dependabot.yml and reports in its body that Dependabot alerts and automated security fixes were "turned off via the API on all 26 repositories" with "90 open Dependabot PRs closed org-wide". Both cannot be the intent. As things stand the ported config either produces nothing, because the repo-level toggle is off, or re-opens the noise #7 was closing. Decide once, org-wide, and make the two PRs agree. If #7's position holds, drop this file from the salvage set; the config is preserved in embeddedos-org/.github/.github/dependabot-template.yml regardless, which the file's own header points at.
6 Low scripts/build-deploy.py:24 minify_css includes + in r'\s*([{};:,>~+])\s*', so calc(100% + 10px) would become calc(100%+10px), which is invalid CSS. This repo's two calc() uses are both subtraction (style.css:215-216) and survive, so nothing is broken today — it is a latent trap for the next stylesheet. Drop + and ~ from that class, or skip the contents of calc().
7 Low scripts/build-deploy.py:36 minify_html's comment pattern <!--(?!.*\[if).*?--> runs under re.DOTALL, so the negative lookahead scans the rest of the whole document. One conditional comment anywhere in a file disables comment stripping for every comment before it. Also, [ \t]+' ' on line 37 is applied to the entire document including <pre>, <code>, <script> and <textarea>. index.html has 7 <pre> blocks; I diffed them and they survive because the code inside is written flush-left, so this is latent rather than live. Anchor the lookahead to the comment body (<!--(?!\[if)(?:(?!-->).)*?-->), and exclude preformatted regions from whitespace collapsing.

Finding 1, run against this PR's head:

$ python3 scripts/build-deploy.py
✓ Minified 32 files, saved 32,179 bytes
✓ Deploy tree written to dist/ (source tree untouched)

$ for f in $(find dist -name '*.js'); do node --check "$f" || echo "SYNTAX ERROR: $f"; done
SYNTAX ERROR: dist/playwright.config.js
SYNTAX ERROR: dist/js/ebot-chat.js
SYNTAX ERROR: dist/js/search.js
SYNTAX ERROR: dist/js/site-chrome.js
                              # clean=1 broken=4

$ for f in js/*.js; do node --check "$f"; done
                              # src clean=4 broken=0

What it does to the nav bar:

  js/site-chrome.js:13  { href: 'https://embeddedos-org.github.io/eApps/', label: '\u{1F3EA} App Store', ... }
dist/js/site-chrome.js:8 { href: 'https:

Finding 2:

$ npm ci --ignore-scripts
npm error code EUSAGE
npm error `npm ci` can only install packages when your package.json and
npm error package-lock.json ... are in sync.
npm error Invalid: lock file's html-validate@8.29.0 does not satisfy html-validate@11.12.0
npm error Invalid: lock file's @html-validate/stylish@4.3.0 does not satisfy @html-validate/stylish@6.0.0
npm error Invalid: lock file's @sidvind/better-ajv-errors@3.0.1 does not satisfy @sidvind/better-ajv-errors@7.0.0
npm error Invalid: lock file's fast-uri@3.1.6 does not satisfy fast-uri@3.1.7

The parts that hold up, verified:

  • The in-place destruction is gone. .github/, tests/ and scripts/ all survive the
    run, SOURCE_ONLY excludes them from dist/, and dist/ contains 97 files with none
    of those three present. The stage_tree() top-level-only pruning is correct — a nested
    content directory named scripts would survive, as the comment claims.
  • index.html 96,374 → 92,422 bytes (4.1%), matching the body's figures to within 12
    bytes, with <title>, rel="canonical" and </html> all retained.
  • style.css.bak is a genuine orphan: git grep style.css.bak origin/master returns
    nothing. Removing it is right.
  • All six ported paths are genuinely absent from master, so nothing here overwrites
    existing work.

Architecture conformance

Conforms. Master design §21 places website and CI templates under Infrastructure, and
every file in this PR is one of those. Nothing crosses a tier boundary, so §5.1 is not
engaged; §21.1's split policy is what the whole exercise serves — two website
repositories with no common ancestor is the "separate repository for a branded name"
outcome §21.1 warns against, and consolidating to the one that actually serves the site
is the right direction.

§28 is where findings 1, 2 and 3 land. The PR's "Verified" block is careful and its
claims are true, but they cover only what was checked: the working-tree hash, the dist/
exclusions, and three HTML markers. Nothing in it speaks to whether the minified
JavaScript parses, whether the lockfile resolves in this repo, or whether the Python
suite runs — and all three fail. That is the §28 distinction between Implemented and
Validated, and it is why the brief treats an unverified "verified" as the finding.

I am not appending a design-doc proposal for this PR. The consolidation it serves is
already the right reading of §21.1, and finding 5 is an org policy decision for a human,
not a gap in the document's wording.

Proposed changes

Smallest sequence that keeps the salvage while dropping what does not work:

  1. Drop minify_js from scripts/build-deploy.py, or replace the whole minifier with
    esbuild. Then re-run and re-check: for f in $(find dist -name '*.js'); do node --check "$f"; done
    must be silent. This is the one item that must not merge as written — the script's
    stated purpose is to produce a deployable tree, and it does not.
  2. rm package-lock.json && npm install && git add package-lock.json, then confirm
    npm ci succeeds.
  3. Resolve finding 5 with #7 before either merges.
  4. Findings 3, 4, 6 and 7 can follow, or test_mobile_full_simulation.py can be dropped
    from the salvage set and left in embeddedos-org's history, which stays readable
    after archiving — the PR body already makes that point about the 29 commits.

Nothing here argues against archiving embeddedos-org. The evidence in the body that it
serves a 404 while still receiving commits is the strongest part of the PR, and steps 1
and 4 of #158 are, as the body says, the more urgent work.

Not checked

  • I did not determine why the test check is failing. It ran 7m24s and I could not
    reproduce it here — pr-check.yml needs a Chromium download and a live server. Note
    that it uses npm install, not npm ci, so finding 2 does not explain it directly;
    but master has no package-lock.json at all, and adding one changes what
    npm install resolves, so this PR could be implicated. That connection is a
    hypothesis, not a result. The failure needs its log read.
  • I did not run tests/simulation/test_mobile_full_simulation.py. Finding 4 is a read of
    its import block and constants; whether its 799 lines of assertions are correct is
    unassessed.
  • I did not open dist/index.html in a browser or run the Playwright suite against the
    minified tree. Finding 1 rests on node --check, which is sufficient for "does not
    parse" and says nothing about the CSS or HTML output beyond findings 6 and 7.
  • I did not review the two ported workflows (health-check.yml, lint-readme.yml) in
    any depth beyond confirming they are new to this repo. Note that lint-readme.yml is
    the same job that is red on embeddedos-org#7 with 144 pre-existing violations on
    that repo's master; whether it passes here is untested.
  • ci.yml:27 and :30 carry 2>/dev/null || true on lint and typecheck, so those two
    steps cannot fail. Pre-existing and not touched by this PR, but it is the same
    swallowed-failure pattern I reported on eNI#30 in this run.
  • I did not verify the gh api and curl output quoted in the PR body showing the other
    repo serving a 404. I took it as given; it is consistent with the repo having no Pages
    configuration.

Automated architecture review of a6a538b7492a — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

@srpatcha

Copy link
Copy Markdown
Member Author

Updated the PR branch in commit 6ee63ef: replaced regex-based minification with byte-preserving staging, reconciled package.json/package-lock.json, and removed the unrelated simulation test. Local dependency installation, build validation, and byte-preservation checks passed. GitHub CI: https://github.com/embeddedos-org/embeddedos-org.github.io/actions/runs/34421665184.

@srpatcha

Copy link
Copy Markdown
Member Author

Final exact-head validation for commit 89a02cd is complete. PR Quality Check passed all 520 Playwright tests across Chromium, Firefox, WebKit, and mobile Chromium, including the HTTP-level sitemap check. CodeQL JavaScript and the CodeQL gate also passed: https://github.com/embeddedos-org/embeddedos-org.github.io/actions/runs/34423982570

@srpatcha srpatcha left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Review — embeddedos-org.github.io#159 "chore: salvage the unique work from embeddedos-org before archiving it"

head: 89a02cd author: srpatcha ci: pass (3/3)

Verdict: Follow-up. Six of the seven prior findings are resolved, and I re-ran the two that mattered rather than taking the commit messages for it — npm ci now succeeds and every JS file in dist/ parses. What is left: the Dependabot conflict is still unaddressed, and the fix to build-deploy.py was thorough enough that the script no longer does the thing the PR body says it was salvaged for.

Prior findings

Prior # Status Evidence
1 — minify_js truncates lines at // inside strings; 4 of 5 dist/ JS files fail node --check Resolved in 6ee63ef Minification is removed outright, for JS, CSS and HTML alike; build() now only stages. Re-ran on this head in a temporary extraction: python3 scripts/build-deploy.py → all 5 files under dist/ pass node --check, and cmp index.html dist/index.html reports byte-identical.
2 — package-lock.json ported from a repo with html-validate: ^8.0.0; npm ci fails Resolved in 6ee63ef Re-ran on this head: npm ci --ignore-scripts exits 0, "added 149 packages in 2s". None of the four Invalid: lines from the last review reappear.
3 — tests/simulation/test_mobile_full_simulation.py is run by nothing Resolved in 6ee63ef by removal File deleted, 799 lines.
4 — that file's import-time pip install / playwright install --with-deps and module-level mkdir Resolved by the same removal
5 — .github/dependabot.yml contradicts embeddedos-org#7, which deletes it org-wide Untouched — still open files.txt still shows 75+ 0- .github/dependabot.yml. embeddedos-org#7 ("chore(ci): remove Dependabot, add Claude + Copilot code review") is still OPEN as of this run. The two PRs still disagree.
6 — minify_css collapses calc(100% + 10px) Resolved by removal of minify_css
7 — minify_html comment lookahead under DOTALL; whitespace collapsed inside <pre> Resolved by removal of minify_html

The previous review's open question — why test was red — is answered by 789f08d and d384669: pr-check.yml installed only Chromium while playwright.config.js configures four projects. It now installs chromium firefox webkit and test is green at 6m51s.

Findings

# Severity File:line Finding Recommended fix
1 Medium scripts/build-deploy.py; PR body, "What is ported" table and "Verified" block The script no longer does what the PR says it was salvaged for, and the body still says it does. The table row reads "scripts/build-deploy.py — HTML/CSS/JS minification", and the Verified block still reports "32 files minified, 32,179 bytes saved" and "index.html 96,362 -> 92,410 bytes (4.1% smaller)". Those numbers described the old regex minifier. The fix removed it entirely — the script's own comment now says "Preserve source bytes until the project adopts syntax-aware minifiers" — so the current behaviour is a byte-for-byte copy, which I confirmed by cmp. Removing the minifier was the right call and I recommended it; leaving the old figures in the body is the problem, because they are the most concrete-looking evidence in the PR and they now describe code that does not exist. Compounding it: nothing consumes the output. The only reference to the script anywhere is package.json:20 ("build": "python3 scripts/build-deploy.py"), no workflow runs npm run build, and deploy.yml:50 publishes path: . — the repository root — not dist/, which is gitignored. So the salvaged artifact is a 33-line no-op with no caller. Two edits and a decision. (a) Replace the table row with what it does now — "stages a publishable copy into dist/ without touching the checkout" — and delete the four stale minification lines from the Verified block. (b) Decide whether to keep it: either drop it from the salvage set (its history stays readable in embeddedos-org after archiving, which the body already argues for the 29 commits), or make it real by having deploy.yml run npm run build and upload path: dist — in which case fix finding 3 first.
2 Medium .github/workflows/pr-check.yml:26-27 and :41-45 Two of the four verification steps in PR Quality Check cannot fail. HTML validation ends in || true and Link check ends in || true, so html-validate and linkinator report into the log and are then discarded. This PR edits this file (line 23), and the comment posted on it says "PR Quality Check passed all 520 Playwright tests" — that part is true and well-evidenced, but the job's name covers four things and gates two. .ai/reviewer.md names a verification whose result is discarded as a finding regardless of the reason given, and the previous review flagged the same construct in ci.yml:27/:30; this is the instance inside the file the PR is touching. Note the html-validate invocation already turns off six rule classes inline, so it is unlikely to be far from clean. Drop || true from the HTML validation step and see what it says. If the count is non-zero, record it as a baseline and gate on no-increase rather than restoring the swallow. For linkinator, external link rot will flake a PR gate — move it to nightly.yml where a failure is actionable, rather than neutering it here.
3 Low scripts/build-deploy.py:11 SOURCE_ONLY is a seven-entry deny-list, so everything else at the repository root is copied into the deploy tree. I listed dist/ after running it: it contains AGENTS.md, CLAUDE.md, MEMORY.md, HANDOFF.md, TASKS.md, MODES.md, ORCHESTRATION.md, QUALITY.md, TESTING.md, VERIFY.md, CODEOWNERS, codecov.yml, lychee.toml, playwright.config.js, package.json, package-lock.json and run_all_tests.py. No confidentiality boundary is crossed — this repository is public and deploy.yml:50 already uploads the root, so all of these are already served from the live site today — but a deploy-tree builder is exactly the tool that should stop that, and as written it reproduces it. If the script is kept (finding 1), invert it: an allow-list of what the site actually publishes (*.html, js/, style.css, docs/, stacks/, downloads/, eApps/, favicon.svg, og-image.png, robots.txt, sitemap.xml, _headers, 404.html) is short, and a new internal .md at the root then does not silently become a public page.
4 Low scripts/build-deploy.py:11-12 No blank line between the SOURCE_ONLY assignment and def stage_tree(); PEP 8 wants two before a top-level def. No Python linter runs in this repository, so nothing will catch it. Add the blank lines.
5 Medium .github/dependabot.yml Prior finding 5, restated only because it is unresolved and blocking: embeddedos-org#7 is still open and still deletes this file as part of an org-wide decision its body describes as already executed ("turned off via the API on all 26 repositories", "90 open Dependabot PRs closed org-wide"). Adding it here either does nothing, because the repo-level toggle is off, or reopens the noise #7 closed. Unchanged from last time: decide once, org-wide, and make the two PRs agree. If #7's position holds, drop this one file from the salvage set — the config is preserved in embeddedos-org/.github/.github/dependabot-template.yml regardless. This is the only item here that needs a human decision rather than an edit.

Verified clean this round:

  • 89a02cd is a real fix, not a cosmetic one. The old sitemap.xml is valid test did page.goto() then textContent('body'). Chromium, Firefox and WebKit each render raw XML through a different built-in viewer, so what lands in body is browser-dependent — the assertion was testing the viewer, not the file. request.get() fetches the bytes. This is precisely the kind of test that would have started failing the moment 789f08d/d384669 added Firefox and WebKit to the run, and it was fixed rather than skipped.
  • d384669/789f08d fix the right thing. playwright.config.js configures four projects; pr-check.yml installed one browser. Installing all three engines is what turned the previously-red test job green, and it did so by making the job match the config rather than by trimming the config.
  • All 3 checks pass on this head, matching the author's comment. No check is skipped and none carries continue-on-error.
  • The prior review's verified-clean items still hold at this head: the source tree survives the run, dist/ excludes tests, .github and scripts, and style.css.bak remains a genuine orphan.

Architecture conformance

Conforms, unchanged from the previous review. Master design §21 places website, docs and CI templates in the Infrastructure row; every file in this PR is one of those, nothing crosses a tier, and §5.1 is not engaged — no include, import, link line or manifest dependency exists here. §21.1 is what the exercise serves: two website repositories with no common git ancestor is the "separate repository for a branded name" outcome §21.1 warns against, and consolidating onto the one that actually serves the site is the right direction. §33 ("the website should explain the product before exposing the full research/project breadth") is the reason finding 3 is worth fixing rather than shrugging at — MEMORY.md and ORCHESTRATION.md being crawlable at the product domain is the opposite of that instruction, even though it costs no confidentiality.

§28's Implemented-vs-Validated line is where finding 1 sits, and the direction of travel is good: last time the "Verified" block was accurate about what it checked and silent about three things that failed; this time the failures are fixed and the block is stale instead. No design-doc proposal appended — nothing here reveals a gap in the master design's wording, and finding 5 is an org policy decision for a human.

Proposed changes

  1. Correct the PR body: the table row and the four minification numbers (finding 1a). This is a body edit, not a code change, and it should happen before merge because the body is the merge-commit message.
  2. Resolve finding 5 with embeddedos-org#7 before either merges. It is the only blocking item and it needs a person, not a patch.
  3. Decide the script's fate (finding 1b). Dropping it is defensible and cheapest; keeping it means wiring deploy.yml to npm run build + path: dist, and then finding 3 becomes required rather than optional.
  4. Drop the two || trues in pr-check.yml, or move linkinator to nightly (finding 2).
  5. Finding 4 is one blank line.

Nothing here argues against archiving embeddedos-org, and the evidence in the body that it serves a 404 while still receiving commits remains the strongest part of the PR.

Not checked

  • The Playwright suite was not run here. "520 tests across Chromium, Firefox, WebKit and mobile Chromium" is GitHub's result and the author's report of it; I did not fetch the job log or re-run the suite. What I verified independently is npm ci, the build script's output, and that every emitted JS file parses.
  • I ran npm ci and build-deploy.py against a git archive extraction of head 89a02cd in a temporary directory, not in the clone. That is enough to settle prior findings 1 and 2; it is not the same environment as the CI runner, and --ignore-scripts means no postinstall step executed.
  • I did not open dist/index.html in a browser, and I did not run the Playwright suite against dist/. Prior finding 1's resolution rests on node --check plus byte-identity with the source, which is sufficient for "the output is not corrupted" and says nothing beyond that.
  • Finding 3's live-exposure claim is partly inferred. I confirmed deploy.yml:50 uses path: . and that all eleven internal .md files are present on origin/master. I did not fetch https://embeddedos-org.github.io/MEMORY.md to observe them being served — that would be an outbound request against the production site, and the workflow configuration is sufficient to make the point.
  • The two ported workflows, health-check.yml and lint-readme.yml, are still unreviewed beyond confirming they are new to this repository. lint-readme.yml is the same job that is red on embeddedos-org#7 with 144 pre-existing violations there; whether it passes here is still untested, and it is not in this PR's check list, so it has not run.
  • ci.yml:27 and :30 still carry 2>/dev/null || true on lint and typecheck. Pre-existing, untouched by this PR, and reported last time — restated here only because finding 2 is the same pattern in a file this PR does edit.
  • The gh api/curl output in the body showing the other repository serving a 404 is taken as given; I did not re-query it.
  • No merge attempted, nothing pushed. Everything was read through git show/git archive against FETCH_HEAD; the clone's working tree is unchanged.

Automated architecture review of 89a02cd09899 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

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.

Consolidate three web repos into one — embeddedos-org serves a 404 and is still being committed to

1 participant