Skip to content

feat: rewrite v2 - #206

Open
Exeloo wants to merge 7 commits into
mainfrom
rewrite/swap-to-v2
Open

Exeloo wants to merge 7 commits into
mainfrom
rewrite/swap-to-v2

Conversation

@Exeloo

@Exeloo Exeloo commented Oct 4, 2026

Copy link
Copy Markdown
Member

What does this PR do?

Rewrites the NanoForge editor from scratch (rewrite/v2). It keeps what the previous editor did (edit a NanoForge game in the browser, hosted or through nf editor) on a new base: everything visible is a plugin, the game's code is the only source of truth, and every change can be undone. The full list of user-facing changes is in CHANGELOG-v2.md; the plan, phase by phase, is in docs/rewrite-plan.md.

Resolves #174
Resolves #156
Resolves #154
Resolves #128
Resolves #127
Resolves #126
Resolves #125
Resolves #123
Resolves #97

Architecture

  • Monorepo (pnpm + turbo): apps/editor (SvelteKit static SPA + Bun server), packages/* (kernel, sdk, settings, history, layout, ui, code, runtime, protocol, rpc, project, server-core, meta, registry), plugins/* (12 built-in plugins), tooling/* (vite plugin for building plugins, example plugin).
  • Plugins only import @nanoforge-dev/editor-sdk (enforced by ESLint). The SDK and @nanoforge-dev/editor-vite-plugin are the public API for third-party plugins (guide in docs/docs/plugins).
  • Code-first ECS: no more .nanoforge/*.save.json. Entities, components and systems are read from and written to the app's main.ts through a TypeScript code worker (ts-morph stays in the worker, out of the page bundle).
  • Engine bridge (protocol v1): live mode, profiler, network and world inspectors, source-mapped console lines.
  • Packages and plugins (ADR 0003/0005): nanoforge.manifest.json, @scope/name, packages installed read-only in nf_modules, plugins per user or per project. Shared libraries between client and server (ADR 0004).
  • API hand-off: docs/api/ documents the registry and settings-sync contracts the API does not offer yet; contract tests run against stand-ins (API_FEATURES gates the calls).
  • Source layout (ADR 0006), aligned on the engine and the CLI: feature folders with index.ts barrels, kebab-case files named after their main export, kind suffixes (.type.ts, .enum.ts, .exception.ts, .const.ts), tests in test/**/*.spec.ts mirroring src/.

Built-in plugins

code-editor, file-manager, viewport, history-panel, settings-ui, ecs, console, command-palette, inspectors, git, packages, scene.

Repository

  • Node 26, pnpm 12; every workspace has the engine's README, LICENSE, .prettierignore, labels and issue-form entry; published workspaces (apps/editor, packages/sdk, tooling/vite-plugin) have their changelog and release config.
  • New e2e workflow; the old editor's files are removed.

Not in this PR

  • No release: versions are unchanged, nothing is tagged or published.
  • Depends on unpushed branches of other repos: engine feat/core-editor-bridge (and feat/scene for scenes), loader feat/server-editor-ipc, CLI feat/editor-open-project (nf editor, nf create plugin).
  • The registry and settings sync wait for the API (see docs/api/).

How do you test this PR?

pnpm install
pnpm typecheck
pnpm lint
CHROME=/usr/bin/google-chrome-stable pnpm test
pnpm turbo build --filter=@nanoforge-dev/editor...

# E2E, against the production build
cd apps/editor
CHROME=/usr/bin/google-chrome-stable \
NANOFORGE_ENGINE=<engine checkout on feat/core-editor-bridge, built> \
NANOFORGE_CLI=<cli checkout on feat/editor-open-project, built> \
npx playwright test --reporter=line

- Typecheck: 42/42 tasks. Unit and Vitest browser tests: 39/39 tasks. Lint: 29/29. SDK public API snapshot unchanged.
- E2E: 48 passed, 0 failed. The 26 tests that play the game are skipped without NANOFORGE_ENGINE pointing at a built engine. Run them with it to cover play, live mode, the inspectors and scenes.
- Manually: pnpm example breakout (with NANOFORGE_ENGINE=../engine-scene), or nf editor <project> from the CLI branch, then play, edit entities in the Scene screen and in live mode, commit from the Git panel, undo across panels.

@Exeloo
Exeloo requested a review from Tchips46 as a code owner October 4, 2026 23:07
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Oct 4, 2026

@Tchips46 Tchips46 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Security audit

Comment thread packages/server-core/src/env/load-env.ts
Comment thread packages/server-core/src/server/create-editor-server.ts Outdated
Comment thread packages/server-core/src/server/create-editor-server.ts
Comment thread packages/server-core/src/server/create-editor-server.ts
Comment thread packages/server-core/src/fs/fs-service.ts
Comment thread packages/server-core/src/server/create-editor-server.ts
Comment thread packages/server-core/src/project/type-files.ts
Comment thread packages/server-core/src/session/session-store.ts Outdated
Comment thread packages/server-core/src/project/project-registry.ts

@Tchips46 Tchips46 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Bug reviews Claude found

Comment thread apps/editor/src/routes/(editor)/load/+page.svelte Outdated
Comment thread apps/editor/src/lib/plugin/follow-project-plugins.ts
Comment thread packages/server-core/src/http/static-files.ts
Comment thread packages/server-core/src/git/git-service.ts Outdated
Comment thread packages/server-core/src/git/git-service.ts Outdated
Comment thread packages/history/src/command/composite-command.ts
Comment thread packages/code/src/engine/code-engine.ts Outdated
Comment thread packages/registry/src/project/project-packages.ts
Comment thread packages/server-core/src/project/formatter.ts
Comment thread apps/editor/package.json

@Tchips46 Tchips46 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Bug reviews Claude found

(child) => (child.redo ?? child.do).call(child),
),
redo: () => runAll(children, (child) => (child.redo ?? child.do).call(child)),
async isValid() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Every child is checked against the current text, but each child expects the text right after itself: two children editing the same file are always invalid, so the undo is refused and the history wiped. Check only the last child per file, or validate in reverse while undoing.

(command) => (command.redo ?? command.do).call(command),
),
redo: () => runAll([previous, next], (command) => (command.redo ?? command.do).call(command)),
async isValid() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same with mergeCommands: previous never matches once next ran, so a merged document edit can't be undone.

undo: async () => {
for (const step of [...done].reverse()) await step.undo();
},
isValid: async () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same bug: renaming a scene whose file moves edits main twice, so undoing that rename now wipes the Scenes history (it worked before).

Comment thread Dockerfile
ARG PUBLIC_PM_URL

# The server reads its mode at runtime (packages/server-core/src/env/load-env.ts)
ENV PUBLIC_MODE=${PUBLIC_MODE}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Without the build arg, PUBLIC_MODE becomes "" and the env schema refuses it: a plain docker build no longer starts. Give it a default.

* A repository's own folder is never reached through the file API: writing hooks or config there
* runs code on the next git command, and its config may hold credentials.
*/
const isGitPath = (path: string) =>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Checked by name only: a committed g -> .git symlink still allows writing g/hooks/…. Check the real path from jail.resolve too.

}
for (const [baseUrl, plugin] of located) {
if (!urlPath.startsWith(baseUrl)) continue;
const rel = decodeURIComponent(urlPath.slice(baseUrl.length));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

decodeURIComponent is still outside the try: /plugins/x/%E0 gives a 500.

);
let response: Response;
try {
response = await (this._options.fetch ?? fetch)(url.toString(), {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Redirects are followed, so a same-origin URL that redirects elsewhere still sends our headers. Use redirect: 'manual', or drop the headers after a redirect.

const run = this._cli.run(['build', '--editor', '--directory', app.root || '.'], {
cwd: this._project.root,
env: { FORCE_COLOR: '0', NO_COLOR: '1' },
signal: AbortSignal.any([this._abort.signal, AbortSignal.timeout(this._buildTimeoutMs)]),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The abort only kills nf: what it starts keeps running. Spawn it detached and kill the process group, like the game server.

});
}
for (const path of io.files()) {
if (!CODE.test(path) || !specifier(library.name).test(await io.read(path))) continue;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The pre-filter reads the disk, so an open file with unsaved edits that imports the library is skipped. Read it through DocumentService. The regex fallback is still there without a worker.

const file: HistoryCommand = {
label: `${move ? 'Move' : 'Copy'} ${from}`,
do: async () => {
if (move) await fs.rename(from, to);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rename then write: if the write fails, the file stays moved. Write first, or rename back on failure.

- Implement `readPackageContents` for loading and validating package manifests and sub-manifests.
- Add validation logic for path constraints, nesting depth, and maximum manifests.
- Introduce test suite for verifying manifest handling, edge cases, and error reporting.
- Update related schemas, registry client, and API contract.
@MartinFillon

Copy link
Copy Markdown
Contributor

can you please not run e2e tests on ci to not run out of actions minutes please

@Exeloo

Exeloo commented Oct 18, 2026

Copy link
Copy Markdown
Member Author

can you please not run e2e tests on ci to not run out of actions minutes please

The repo is in public, so it doesn't use our action minutes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

3 participants