Skip to content

fix(devcontainer): harden worktree startup with devrouter#5169

Draft
rschlaefli wants to merge 11 commits into
v3from
codex/worktree-lifecycle-hardening
Draft

fix(devcontainer): harden worktree startup with devrouter#5169
rschlaefli wants to merge 11 commits into
v3from
codex/worktree-lifecycle-hardening

Conversation

@rschlaefli

@rschlaefli rschlaefli commented Jul 13, 2026

Copy link
Copy Markdown
Member

What This Fixes

Linked worktrees could reuse a stale or ambiguous dev process, start against the wrong Git metadata, or stay registered while their routes were unhealthy. The previous fix made that lifecycle safe inside Klicker, but it also left generic process supervision and its regression suite duplicated in this repository.

This branch makes worktree startup reliable through one canonical command:

devrouter workspace ensure .

Generic process ownership now comes from the released @devrouter/cli 0.0.30. Klicker keeps only its app-specific environment setup and Turbo task list.

How It Works

  • .devrouter.yml pins Devrouter 0.0.30 and defines the routed apps and PostgreSQL endpoint for each persisted workspace identity.
  • The Devrouter Compose overlay separates WORKSPACE from DEVROUTER_WORKSPACE, requires the linked worktree's Git common directory, mounts it at the same absolute path, and uses workspace-specific aliases and origins.
  • The devcontainer image downloads the exact @devrouter/cli@0.0.30 package and extracts only devrouter-process. It does not install the full CLI or its dependency tree in the app image.
  • post-start.sh prepares Klicker's origins and delegates lifecycle ownership to devrouter-process ensure. The app command is the root pnpm run dev:container script, where the Klicker-specific app and worker filters remain visible.
  • The local generic supervisor and its 288-line regression harness were removed. Their fail-closed ownership, bounded shutdown, concurrency, malformed-state, and unhealthy-runtime coverage now live in Devrouter (devrouter#20). Route recovery was introduced in devrouter#19.
  • LTI exposes a canonical Turbo dev task, and local initialization assigns each database to its application role.
  • Agent guidance, the environment doctor, devcontainer docs, engineering wiki, changelog, and dated plan describe the same startup path.

Branch Coverage

  • Base: v3
  • Head: 68bd6a467
  • Reviewed: 11 commits; 17 files; 512 insertions; 113 deletions
  • Covered: worktree and Git identity, process ownership and recovery, route recovery, service startup, local database ownership, Devrouter 0.0.30 integration, and documentation

Review Focus

  • Confirm the boundary is simple: Devrouter owns generic process lifecycle, while Klicker supplies only the command, match string, log path, and app-specific setup.
  • Confirm the exact-version package extraction is reproducible and does not add a runtime dependency tree to the devcontainer.
  • Confirm the Compose overlay fails closed when Devrouter identity or Git metadata is missing and preserves the linked worktree inside the container.

Verification

Current head:

  • Pre-commit check:all passed: TypeScript, lint, formatting, syncpack, agent-instruction validation, and Prisma sync.
  • Pre-push pnpm run build passed 21/21 production build tasks.
  • Frozen pnpm install completed without changing the lockfile.
  • Bash syntax, ShellCheck, merged Compose validation, Prettier, and git diff --check passed. The wiki validator is not available in this repository, so affected Markdown was formatted and checked directly.
  • Opengrep ran 93 rules over the changed code and configuration files with zero findings.
  • A clean image build installed the published 0.0.30 helper through the Dockerfile's exact extraction path. Devrouter's complete Linux lifecycle regression then passed against /usr/local/bin/devrouter-process in that image.
  • A cold published workspace ensure recreated the devcontainer and started one filtered Turbo group. HTTP results were API 404, Auth 200, PWA 200, Manage 200, Control 200, OLAT health 200, Response API 200, LTI 401, and Chat 404; PostgreSQL was TCP-ready. The 404 and 401 responses are expected readiness results for those roots.
  • Two warm ensures preserved app container 486bce82e9cb and owned process state 770 770 893406388-101. The container resolved the exact final Git head and /workspaces/klicker-uzh worktree.
  • agent-browser completed the delegated lecturer login through the worktree-specific Auth route, returned to the same worktree-specific Manage route, and rendered the seeded library at https://manage.klicker.codex-worktree-lifecycle-hardeni.localhost/.
  • Final strict maintainability and security reviews found no blocking issue.

Warnings and unrelated observations:

  • The first sandboxed production build could not resolve fonts.googleapis.com; the network-enabled rerun and the pre-push build both passed 21/21.
  • Existing browser development warnings include an Auth nested-button hydration error, initial unauthenticated GraphQL responses, a Next HMR message warning, and image warnings. They did not block login or rendering, and this branch does not change those UI paths.
  • The existing Hatchet heartbeat logger TypeError remains unrelated and non-blocking for startup, routing, Git identity, and process reuse.
  • All substantive GitHub checks pass on head 68bd6a467, including all eight Playwright shards, GraphQL, TypeScript, build, format, lint, syncpack, CodeQL, GitGuardian, and Greptile.
  • The claude-review action failed twice in its review runner after one turn with subtype: success, is_error: true, zero cost, and no review comments. This is a review-service failure rather than a code finding.

Security / Privacy

  • The image fetches an exact immutable package version and installs only the reviewed helper file.
  • The helper receives constant arguments, owns only its recorded process group, and fails closed around foreign or malformed process state.
  • No secrets, credentials, or private data are included.

Blocking Before Merge

  • Resolve or waive the failing claude-review service check; all substantive CI checks pass
  • Maintainer review

The PR remains a draft.

Follow-Up After Merge

  • Pull the worktree-safe startup improvements into the relevant long-lived branches.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b29ad2bc-7adb-4775-aaa5-a61cffae5444

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 13, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

Safe to merge once CI passes; all changes are scoped to the devcontainer and developer tooling with no production code paths affected.

The two issues flagged in earlier review rounds are both resolved: the DEVROUTER_GIT_COMMON_DIR Compose mount now uses :? syntax for an actionable error, and the local dev-process.sh supervisor (with its missing-fingerprint edge case) has been removed entirely and replaced by the vetted packaged helper from the pinned @devrouter/cli@0.0.30 tarball. Process ownership is explicit and fails closed; the dev:container script plus LTI canonical dev task give Turbo a clean single-pass filter list; database ownership assignments in init.sql are correct given the roles defined earlier in the same file. Documentation is internally consistent with the executable startup path.

No files require special attention.

Important Files Changed

Filename Overview
.devcontainer/Dockerfile Adds util-linux (flock/setsid), tar, and a targeted npm pack + tar extraction of only devrouter-process from @devrouter/cli@0.0.30; strip-components and output path are correct.
.devcontainer/post-start.sh Replaces the inline setsid/pgrep double-start guard with a single devrouter-process ensure call; all origin and workspace env setup is retained above it, which is exactly the Klicker-specific boundary.
.devcontainer/docker-compose.devrouter.yml Adds DEVROUTER_WORKSPACE env propagation and the DEVROUTER_GIT_COMMON_DIR bind-mount using :? error syntax; both additions address the two prior review threads.
.devrouter.yml Bumps the pinned devrouter version from 0.0.21 to 0.0.30 and updates inline comments to reflect the new workspace ensure canonical path; no schema changes.
package.json Adds dev:container script with the exact Turbo filter list (11 packages + --concurrency 30), making the full in-container app command a single pnpm script that devrouter-process can fingerprint.
apps/lti/package.json Adds a canonical dev script aliasing dev:build + dev:run in parallel so Turbo can run the LTI package once without duplicate EADDRINUSE failures; dev:lti becomes a compatibility alias.
util/init.sql Assigns correct owners to klicker-prod-lti, klicker-qa, and klicker-qa-lti databases; all three roles are already created earlier in the same file, so the ALTER statements are valid.
.agents/skills/devrouter/SKILL.md New agent skill file documenting the complete devrouter config schema, workspace isolation model, env-var injection, commands, and validation workflow; content is consistent with the changes made to Compose and .devrouter.yml.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Host
    participant DevPod
    participant Container
    participant devrouter_process as devrouter-process
    participant Turbo

    Host->>Host: devrouter workspace ensure .
    Host->>Host: "Resolve workspace identity & overlay"
    Host->>DevPod: Start/attach exact DevPod (workspace id)
    DevPod->>Container: post-create.sh (once): install, build, seed
    DevPod->>Container: post-start.sh (every start)
    Container->>Container: Re-source devcontainer.env
    Container->>Container: Set WORKSPACE-scoped origins
    Container->>devrouter_process: devrouter-process ensure --name klicker-dev --match turbo run dev --log /tmp/dev.log -- pnpm run dev:container
    devrouter_process->>devrouter_process: flock state file
    alt Process already owned and fingerprint matches
        devrouter_process-->>Container: Idempotent reuse — return 0
    else Stale owned group
        devrouter_process->>devrouter_process: Bounded TERM/KILL of owned PGID
        devrouter_process->>Turbo: setsid pnpm run dev:container
    else No prior state
        devrouter_process->>Turbo: setsid pnpm run dev:container
    end
    devrouter_process->>devrouter_process: Record PID/PGID/fingerprint to state file
    Turbo->>Container: Start all routed apps + 2 Hatchet workers
    Host->>Host: devrouter workspace ensure — prove aliases, HTTP routes, Git mount
    Host-->>Host: 10 routes verified (one recreate budget if HTTP unhealthy)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Host
    participant DevPod
    participant Container
    participant devrouter_process as devrouter-process
    participant Turbo

    Host->>Host: devrouter workspace ensure .
    Host->>Host: "Resolve workspace identity & overlay"
    Host->>DevPod: Start/attach exact DevPod (workspace id)
    DevPod->>Container: post-create.sh (once): install, build, seed
    DevPod->>Container: post-start.sh (every start)
    Container->>Container: Re-source devcontainer.env
    Container->>Container: Set WORKSPACE-scoped origins
    Container->>devrouter_process: devrouter-process ensure --name klicker-dev --match turbo run dev --log /tmp/dev.log -- pnpm run dev:container
    devrouter_process->>devrouter_process: flock state file
    alt Process already owned and fingerprint matches
        devrouter_process-->>Container: Idempotent reuse — return 0
    else Stale owned group
        devrouter_process->>devrouter_process: Bounded TERM/KILL of owned PGID
        devrouter_process->>Turbo: setsid pnpm run dev:container
    else No prior state
        devrouter_process->>Turbo: setsid pnpm run dev:container
    end
    devrouter_process->>devrouter_process: Record PID/PGID/fingerprint to state file
    Turbo->>Container: Start all routed apps + 2 Hatchet workers
    Host->>Host: devrouter workspace ensure — prove aliases, HTTP routes, Git mount
    Host-->>Host: 10 routes verified (one recreate budget if HTTP unhealthy)
Loading

Reviews (4): Last reviewed commit: "refactor(devcontainer): delegate process..." | Re-trigger Greptile

Comment thread .devcontainer/dev-process.sh Outdated
Comment thread .devcontainer/docker-compose.devrouter.yml Outdated
@rschlaefli rschlaefli changed the title fix(devcontainer): harden linked-worktree lifecycle fix(devcontainer): harden worktree startup with devrouter Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant