Skip to content

fix(framework): five issues surfaced by the git-module field test - #267

Merged
antosubash merged 14 commits into
mainfrom
worktree-framework-fixes
Aug 20, 2026
Merged

fix(framework): five issues surfaced by the git-module field test#267
antosubash merged 14 commits into
mainfrom
worktree-framework-fixes

Conversation

@antosubash

@antosubashantosubash commented Aug 19, 2026

Copy link
Copy Markdown
Owner

Summary

Five framework issues surfaced by the git-module field test, fixed with tests — then hardened by a 5-pass code-review loop and a full browser QA cycle via /ship:

  1. register_csp_sources module hook — modules declare external asset origins (registry.add("style-src", "https://rsms.me")); fetch directives only, single-token validation at boot, CSP3 fallback-chain-aware merging (new clauses seed from their fallback chain; extras mirror into shadowing -elem/worker-src/frame-src clauses). Honored by dev and prod CSPs.
  2. JSON error bodies for API callers — 403/404/500 negotiate by q-value-aware Accept parsing + the /api/* path rule; explicit text/html (browser navigations) and X-Inertia requests keep the rendered error page; JSON responses preserve exc.headers (WWW-Authenticate etc.); 422 follows the same rule.
  3. cwd-independent .env + SQLite — shared find_env_file() walk-up (bounded; stops at repo boundaries, $HOME, world-writable ancestors; anchors at boundary dirs so fresh scaffolds work pre-.env; Path.home() guarded for rootless containers), used by settings, workers, and the doctor CLI alike; relative sqlite URLs anchor at the project root; file: URI-mode URLs pass through.
  4. Scaffolded Vite port from SM_VITE_DEV_URL — one .env value drives the dev server port and origin (vite.dev-url.ts), mirroring the backend's walk rules; handles blank/scheme-less/portless values sanely.
  5. Opt-in CSRF primitivesimple_module_hosting.csrf (RequiresCsrf + get_csrf_token, X-CSRF-Token), bytes-safe compare, async dep, no session mutation on rejection.

Docs across framework-conventions, lifecycle, middleware, module-authoring, env-vars, configuration, CLAUDE.md — and the 5 shipped agent skills.

Verification

Report links are Claude Artifacts and start private — shareable from claude.ai/code/artifacts if reviewers need them.

Test plan

  • Reviewer: make test-py && make lint green
  • Reviewer: with a dev server up, a browser hit on a missing /api/* path renders the error page while curl -H 'Accept: application/json' gets JSON detail
  • CI green

https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc

403/404/500 under /api/* — or with an explicit Accept: application/json —
now return the JSON detail instead of the Inertia HTML error page, which
hid the actual error (permission name, CSRF hint) from API clients.
Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc
Settings now find the project .env by walking up from the cwd (bounded,
stopping at repo boundaries and $HOME), and relative sqlite URLs resolve
against the project root instead of the process cwd — so CLI tools run
from host/ or modules/<name>/ hit the same database as the app.
Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc
A module whose frontend loads assets from an external origin (pagebuilder's
rsms.me font was the field case) can now declare it:
def register_csp_sources(self, registry):
registry.add("style-src", "https://rsms.me")
Only fetch directives are extendable (never default-src/base-uri/
form-action/frame-ancestors); sources are validated single tokens, so a
typo fails at boot instead of weakening the header. Both the dev (Vite-
widened) and production CSPs honor the registry.
Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc
…_URL
The template hardcoded port 5050 + origin while the backend read
SM_VITE_DEV_URL from .env — running on another port meant editing the
generated file and the env var in sync. The config now derives both from
the single .env value (process env wins; 5050 stays the default).
Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc
RequiresCsrf router dependency + get_csrf_token view helper, lifting the
design pagebuilder shipped first into the framework so every module shares
one header (X-CSRF-Token) and one token-discovery convention. Docs cover
the new register_csp_sources hook and the CSRF opt-in.
Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc
@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Aug 19, 2026

Copy link
Copy Markdown

Deploying simple-module-python with Cloudflare Pages Cloudflare Pages

Latest commit:024590d
Status: ✅ Deploy successful!
Preview URL:https://c3f59ed6.simple-module-python.pages.dev
Branch Preview URL:https://worktree-framework-fixes.simple-module-python.pages.dev

View logs

lifecycle.md gains the register_csp_sources section + hook listings;
middleware.md points at the CSP hook and the CSRF opt-in; module-authoring
gains External asset origins (CSP) and CSRF on mutation endpoints sections;
env-vars/configuration document SM_PROJECT_ROOT, relative-sqlite anchoring,
and SM_VITE_DEV_URL as the single dev-port knob; framework-conventions adds
the HTML-vs-JSON error contract.
Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc
creating: register_csp_sources in the hooks table; conventions: CSRF
section rewritten around the opt-in hosting primitive + new CSP-sources
convention; registries: phase-5 hook chain extended; inertia-pages: JSON
error bodies and X-CSRF-Token notes on the fetch() path; cli: SM_VITE_DEV_URL
as the single dev-port knob. The CLI bundle is a symlink to skills/, so
smpy skills add/update ships the same content.
Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc
Ten confirmed findings from /code-review high: CSP3 fallback chains when
seeding new directives, bytes-based CSRF token compare (non-ASCII header
500), shared walk-up .env discovery in core dotenv resolved per
instantiation, Accept: text/html wins over the /api prefix (and 422
aligned onto the same rule), scaffold Vite helper rebuilt on loadEnv with
env-dir walk-up + origin normalization, register_csp_sources hook order
matched to docs, overview.md hook list updated.
Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc
Ten more confirmed findings: -elem shadow clauses now mirror base-directive
extras, CSP source validation rejects commas and bare wildcards, Vite dev
URL helper handles scheme-less/portless values sanely (single parse, .git
boundary walk, SM_PROJECT_ROOT), sqlite file: URI-mode URLs pass through,
JSON errors keep exc.headers and respect X-Inertia, doctor CLI anchors on
the same walked-up .env, plus async CSRF dep and settings cleanup.
Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc
Boundary dirs now anchor .env discovery (fresh scaffolds pre-.env), the
walk refuses world-writable ancestors, run_worker loads the root .env,
error negotiation parses Accept q-values and reuses the shared X-Inertia
constant, CSP extras mirror into every shadowing clause from the fallback
chains, the Vite env walk gains the $HOME stop, and chdir-isolation tests
clear SM_PROJECT_ROOT.
Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc
Path.home() guarded for $HOME-less containers, run_worker routes through
find_env_file (SM_PROJECT_ROOT honored), TS env walk gains the
world-writable guard and boundary-dir anchoring to match Python, explicit
_env_file no longer pays the discovery walk and _env_file=None keeps the
project-root sqlite anchor, Accept: text/html;q=0 rules out the HTML page
globally, CSRF cold-session race documented, cross-references between the
two project-root resolvers.
Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc
Guard os.homedir() in the scaffold Vite helper (rootless containers) and
treat a blank SM_VITE_DEV_URL as unset instead of feeding new URL('').
Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc
Browser navigations (view and /api paths alike) render the Inertia error
page; fetch callers get JSON detail via the Accept rule and the /api path
rule. Runs in the e2e suite against a live server.
Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc
@antosubash
antosubash marked this pull request as ready for review August 20, 2026 09:20
@antosubash
antosubash merged commit 7105114 into mainAug 20, 2026
13 checks passed
antosubash pushed a commit that referenced this pull request Aug 20, 2026
Resolves the conflict in the scaffold's vite.config.ts and refreshes comments
that main's #267 made stale.
**Conflict.** Both sides restructured the same file for the same reason — the
300-line cap. main extracted the dev-server URL derivation into
`vite.dev-url.ts`; this branch extracted the module-asset loading into
`module-assets.ts`. Kept both: the config now imports `viteDevServer` and
`loadModuleAssets`. Taking either side alone would have dropped the other's
extraction, and taking main's inline asset block would also have dropped this
branch's components/ support (GH #258).
**Stale rationale.** main's #267 fixed cwd-independent .env discovery
(`find_env_file` walks up) and anchors relative sqlite URLs to the project
root — which is the alternative this branch explicitly declined for #262 as
too broad. It is the better fix, and it lands first. What that invalidates is
not the code here but its justification: `_alembic_argv`, both `env.py`
copies, and the bootstrap test all asserted that Settings resolves `.env`
against the cwd and that bootstrapping from host/ writes host/host/app.db.
Neither is true of the merged tree. Rewritten to say what is now true — the
bootstrap runs where `make migrate` and the app run so relative paths in the
scaffold mean one thing, with #262 defended in depth upstream.
The commit-before-response middleware survived the merge into
`_phase_helpers.py` unchanged, still innermost, now alongside #267's
csp_registry parameter.
Verified on the merged tree: pytest 2016 passed / 2 skipped; vitest 48 passed;
ruff format + ruff check + ty check clean; 300-line cap clean; tsc and
`biome ci .` clean.
Claude-Session: https://claude.ai/code/session_015fCFfMiqGce8unVpVezJG7
Sign up for freeto 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

@antosubash