Skip to content

📝 docs(claude): add per-package CLAUDE.md files and monorepo.md - #49

Merged
vtempest merged 3 commits into
masterfrom
claude/dazzling-mayer-tpegbe
Sep 11, 2026
Merged

📝 docs(claude): add per-package CLAUDE.md files and monorepo.md#49
vtempest merged 3 commits into
masterfrom
claude/dazzling-mayer-tpegbe

Conversation

@vtempest

@vtempest vtempest commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Rescoped after #47 and #48 merged. This originally added the root CLAUDE.md and five .claude/architecture/ notes. #48 landed four of those first, and the master merge (6820dbd) resolved every shared file to #48's version. That is the right outcome — so what is left here is only the layer #48 did not carry.

What this adds

9 per-package CLAUDE.md all 8 packages/* plus grab-help-docs — the rules and traps specific to working inside each one
.claude/architecture/monorepo.md workspaces, npm vs pnpm, which packages actually publish, where the tests live
.claude/settings.json pre-approves the repo's own build/test commands; denies reads of .env / .dev.vars

Plus four loose ends the merge left, fixed here:

  • .gitignore had both PRs' .claude blocks stacked. Collapsed to one, keeping the un-ignore for .claude/settings.json and the local-only excludes (settings.local.json, CLAUDE.local.md).
  • CLAUDE.md ground rule 4 said 🔥 chore: delete the vestigial docs/ folder #47 "removes" docs/. It has merged; the folder is gone. The Detailed-notes table now also points at monorepo.md and at the per-package files, which nothing referenced.
  • grab-help-docs/CLAUDE.md still described docs/ as a live landing page. Rewritten: it is deleted, and what matters now is that Vercel's Root Directory still points at the missing folder — the note sends the reader to documentation.md for the dashboard fix.
  • packages/grab-api/CLAUDE.md now carries the zero-runtime-dependencies claim docs: add architecture guides and agent orientation #48 raised to a repo-wide ground rule.

What the per-package files say

Each records what an agent needs to know to work in that package, not how to use it:

  • grab-api — this is grab-url; its public API is the published one. Zero runtime deps. Anything the slim entry imports must stay externalizable or grab-url/slim quietly stops being slim.
  • grab-url-cli — the shebang comes from the build's banner, not source. extract-webpage must stay a runtime import(). Path traversal from a server-supplied filename is the classic failure mode.
  • archiver-web — zip slip and zip bombs, doubly so because two of its three entries are executable bins.
  • quantum-sphere-loading-animation — the two build rules that look removable: React externalized ("Invalid hook call"), and "use client" re-applied in generateBundle because terser discards it.
  • loading-animationssrc/svg/index.ts is a generated barrel; edit the SVGs and re-run npm run make:icons.
  • api2client — the "grab-url" alias in vite.config.ts is what stops the bundle carrying two copies of the client.
  • log-json — colors must degrade off a TTY, or a piped JSON stream breaks every parser downstream.
  • native-app-wrapper — outside the workspace globs; its own CI; a sidecar name wrong on one OS fails only after packaging.
  • grab-help-docs — deploys as a static export via build-static-pages.mjs, not next build; one page is generated from the skill.

CI

Both red checks are inherited from master and documented in the comments above — neither is fixable from this branch:

  • test — 11 failures in test/page-archive.test.ts, red on master since ✨ feat(cli): archive a page into a folder with --page #45 added that file. Verified: test/ and packages/grab-url-cli/src/ are byte-identical to master on this branch.
  • Vercel — the project's Root Directory is docs, now a deleted folder. A dashboard setting; every deployment including production has failed for days.

Documentation only — no code, config or dependency changes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EoGK1G8bnF6riGQekWLtYr

Adopt the qwksearch-research-agent layout: a root CLAUDE.md operating manual
backed by .claude/architecture/ deep notes, plus one CLAUDE.md per package.

- .claude/architecture/{overview,build,monorepo,documentation,conventions}.md
- CLAUDE.md in all 8 packages/ directories and in grab-help-docs
- .claude/settings.json pre-approving the repo's own build/test commands and
  denying reads of .env / .dev.vars

The notes record what is specific to this repo rather than restating the README:
this is a monorepo in layout but a single published package in practice, npm is
the package manager (not bun, unlike its siblings), most packages/* are private
internals compiled into grab-url's dist, all tests live in the root test/ folder,
and CI never runs the build so a broken vite.config.ts passes.

build.md documents the packaging decisions that look removable and are not —
externalizing extract-webpage and React, restoring "use client" in
generateBundle after terser, and the shebang banner the bins depend on.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EoGK1G8bnF6riGQekWLtYr
@vercel

vercel Bot commented Sep 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
grab-url Error Error Sep 11, 2026 1:57pm UTC

@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Collaborator Author

test is failing, but not because of this PR

The test check is red on this branch with one assertion failure:

AssertionError: expected 'download failed — the media may be pr…' to be 'download failed'

Expected: "download failed"
Received: "download failed — the media may be private, region-locked or removed"

 ❯ test/page-archive.test.ts:305:38

Why it isn't this PR's

This branch changes 17 files and not one of them is code — five .claude/architecture/*.md notes, nine CLAUDE.md files, the root CLAUDE.md, .claude/settings.json, and .gitignore. Neither file involved in the failure is touched:

$ git diff --name-only origin/master...HEAD
.claude/architecture/build.md
.claude/architecture/conventions.md
.claude/architecture/documentation.md
.claude/architecture/monorepo.md
.claude/architecture/overview.md
.claude/settings.json
.gitignore
CLAUDE.md
grab-help-docs/CLAUDE.md
packages/api2client/CLAUDE.md
packages/archiver-web/CLAUDE.md
packages/grab-api/CLAUDE.md
packages/grab-url-cli/CLAUDE.md
packages/loading-animations/CLAUDE.md
packages/log-json/CLAUDE.md
packages/native-app-wrapper/CLAUDE.md
packages/quantum-sphere-loading-animation/CLAUDE.md

The Tests workflow is already red on master. It went red at 0ec248a (the #45 merge, "✨ feat(cli): archive a page into a folder with --page") and is still red on 44987c7, which is this PR's base. The run immediately before that, f57f0c4 (#44, yt-dlp routing), was green.

Root cause

The two sides were written against different versions of the same string, and both are on master today:

packages/grab-url-cli/src/transfer/ytdlp-transfer.ts:244 — widened in 98b613f (#44) to explain why a download failed:

1: 'download failed — the media may be private, region-locked or removed',

test/page-archive.test.ts:305 — added in fc2365d (#45) still asserting the old short string:

expect(describeYtDlpExit(1)).toBe('download failed');

describeYtDlpExit returns the map value verbatim, so this is deterministic — it fails on every run, not intermittently. It is not a flake, and a re-run would only reproduce it, so I have not spent one.

Proposed patch

The source message looks like the intended behaviour (the other codes in that map are similarly descriptive), so the test is the side that is stale. Matching the neighbouring expect(describeYtDlpExit(77)).toContain('77') style keeps it robust against future wording changes:

--- a/test/page-archive.test.ts
+++ b/test/page-archive.test.ts
@@ -302,7 +302,7 @@ describe('ytdlp-transfer — misc', () => {
     it('describes known exit codes', () => {
         expect(describeYtDlpExit(0)).toBe('completed');
-        expect(describeYtDlpExit(1)).toBe('download failed');
+        expect(describeYtDlpExit(1)).toContain('download failed');
         expect(describeYtDlpExit(null)).toBe('terminated by signal');
         expect(describeYtDlpExit(77)).toContain('77');
     });

I have not pushed this, because it is a code change to an unrelated test inside a documentation-only PR, and choosing which side is authoritative is the maintainer's call — toBe against the full string is equally defensible if the exact wording is meant to be pinned. Happy to push it here, or open it as its own PR against master so the fix lands independently of these docs. Either way master needs it: every PR opened from here will inherit this red check until it is fixed.


Generated by Claude Code

…tually is

The first pass called the root `docs/` folder "a small static landing page for
grab.js.org". It is a GitHub Pages Jekyll stub — a `_config.yml` and an
`index.html` redirect, kept because `/docs` is the only folder name Pages
accepts besides the repository root — and its own README says so.

The omission that mattered: that folder is what the Vercel project's Root
Directory still points at, and it is why every deploy fails. Turbo resolves no
package from `docs/` (it matches neither workspace glob), so no `.next` is
produced and Vercel reports the missing `docs/.next/routes-manifest.json`.
The fix is a dashboard setting — Root Directory `grab-help-docs`, with the
Install/Build Command overrides cleared so `grab-help-docs/vercel.json`
supplies them — so no commit here can fix it, and an agent should not go
hunting for it in turbo.json or vercel.json.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EoGK1G8bnF6riGQekWLtYr

Copy link
Copy Markdown
Collaborator Author

Two things: the Vercel check, and an overlap with #48

1. The Vercel deploy is also failing, and also not because of this PR

Error: The file "/vercel/path0/docs/.next/routes-manifest.json" couldn't be found.
 WARNING  No tasks were executed as part of this run.

Every one of the project's last 20 deployments is ERROR — including each production deploy from master: 44987c7 (#46), 0ec248a (#45), f57f0c4 (#44), b7eff05 (#43), 1b0cc90 (#42). It has been failing this way for days, on branches nobody has touched since.

The cause is that the Vercel project's Root Directory is docs, which is a GitHub Pages Jekyll stub, not a Next.js app. Turbo resolves no package from it — docs/ matches neither packages/* nor grab-help-docs in the workspace globs — hence "No tasks were executed", no .next, and the missing manifest.

This is not my finding; docs/README.md on master already says it:

Vercel (https://grab.js.org) — builds grab-help-docs, not this folder. The Vercel project's Root Directory must be set to grab-help-docs; install and build commands come from grab-help-docs/vercel.json. If it is left pointing at docs/, the deploy fails with The file "…/docs/.next/routes-manifest.json" couldn't be found.

It is a dashboard setting, not a diff — set Root Directory to grab-help-docs and clear the Install/Build Command overrides so grab-help-docs/vercel.json supplies them. No commit in this repository can fix it, so there is nothing for me to port here. #47 reaches the same conclusion independently.

I have pushed c847ef0, which records exactly this in .claude/architecture/documentation.md — my first pass described docs/ as "a small static landing page for grab.js.org", which was both imprecise and silent about the one operational fact that matters. That is a fix to this PR's own content, not a widening of it.

So both red checks on this branch are inherited from master: test (see my earlier comment) and Vercel. Neither has a fix that can be committed here.

2. This PR substantially duplicates #48

I opened this before noticing #48, which landed 27 minutes earlier and does the same thing: a root CLAUDE.md plus .claude/architecture/{overview,build,conventions,documentation}.md, and the same .gitignore un-ignore. The two will conflict on every one of those files.

They are not equivalent, though:

#48 #49 (this)
Root CLAUDE.md
architecture/overview, build, conventions, documentation
architecture/monorepo.md ✅ npm-vs-pnpm, which packages publish, the root test/ folder
Per-package CLAUDE.md ✅ 8 packages + grab-help-docs
.claude/settings.json
Vercel root-cause recorded ✅ (c847ef0)
Size 6 files, +419 18 files, +800

This one is close to a superset, so the cheapest resolution is probably to take one and close the other rather than untangle a merge. That is the maintainer's call, not mine — I am not touching #48, and I would rather not have both merge and leave the repo with two accounts of the same architecture.

Worth noting alongside: #47 deletes docs/ as vestigial. If that merges, the section I just added stays correct about the Vercel setting but should lose the "what the folder is" half. Happy to rebase onto whichever of #47/#48 lands first.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Correction: it's 11 failures, not 1 — and my proposed patch was not sufficient

I read the test failure off Codecov's errors-only view earlier and reported a single stale assertion. I've now installed the repo and run the file directly. There are 11 failures, all in test/page-archive.test.ts, and the one-line patch I proposed fixes only the last of them. Please don't apply it expecting green.

$ npx vitest run test/page-archive.test.ts
 Test Files  1 failed (1)
      Tests  11 failed | 48 passed (59)

They fall into four groups, and together they say the ytdlp-transfer half of this file was written against a different version of that module:

1. Seven — the vi.mock factory omits exports the tests import

Error: [vitest] No "parseYtDlpSize" export is defined on the
"../packages/grab-url-cli/src/transfer/ytdlp-transfer.js" mock.
Did you forget to return it from "vi.mock"?

3 × parseYtDlpSize, 4 × parseYtDlpEta.

2. Two — parseYtDlpProgress returns null where a parse was expected

AssertionError: expected null not to be null                    (:255)
TypeError: Cannot read properties of null (reading 'percent')   (:262)

3. One — buildYtDlpArgs ignores the supplied filename (:284)

Expected: "My Video.%(ext)s"
Received: "%(title)s [%(id)s].%(ext)s"

4. One — the stale exit-code message (:305), the one I originally reported.

Group 3 is the interesting one: it is not a wording drift but a behaviour mismatch — either buildYtDlpArgs is meant to honour filename and doesn't, or the test encodes an intent that was dropped. That needs whoever wrote #45 to say which, and it is well beyond a docs PR.

Still not this PR's

Unchanged, and now verified by content rather than inference — both files involved are byte-identical to master:

$ git diff --stat origin/master -- test/page-archive.test.ts
$ git diff --stat origin/master -- packages/grab-url-cli/src/transfer/ytdlp-transfer.ts
(both empty)

My branch's entire diff against master is 17 files: five .claude/architecture/*.md, nine CLAUDE.md, .claude/settings.json and .gitignore. The Tests workflow is red on master itself, having gone red at 0ec248a (#45) — the commit that added this file — and stayed red through 44987c7, this PR's base.

So I'm withdrawing the proposed patch rather than narrowing it: reconciling this file with ytdlp-transfer.ts is a real piece of work for the author of #45, not a one-liner, and not something to smuggle into a documentation PR. Both red checks here (test and Vercel) remain inherited from master, and neither has a fix that can be committed on this branch.


Generated by Claude Code

@vtempest
vtempest merged commit 12dce81 into master Sep 11, 2026
2 of 4 checks passed
@vtempest
vtempest deleted the claude/dazzling-mayer-tpegbe branch September 11, 2026 13:59
@vtempest vtempest changed the title 📝 docs(claude): add .claude/ agent docs and per-package CLAUDE.md files 📝 docs(claude): add per-package CLAUDE.md files and monorepo.md Sep 11, 2026
vtempest added a commit that referenced this pull request Sep 11, 2026
📝 docs(claude): tidy the four loose ends left by the #47/#48/#49 merges
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.

2 participants