Skip to content

refactor(router): gate init container first-run setup on explicit marker file - #66

Closed
mrsimpson wants to merge 1 commit into
mainfrom
fix/init-script-explicit-first-run-marker
Closed

refactor(router): gate init container first-run setup on explicit marker file#66
mrsimpson wants to merge 1 commit into
mainfrom
fix/init-script-explicit-first-run-marker

Conversation

@mrsimpson

Copy link
Copy Markdown
Owner

Follow-up to #65.

Summary

  • Init containers run on every pod start, not once per PVC. Today the script infers "is this the first run?" from filesystem side-effects ([ ! -d /home/opencode/.config/opencode ], [ ! -d /workspace/.git ]).
  • Those checks are per-phase, so a partial failure (config seed succeeds, git clone fails with ENOSPC) leaves the PVC in a half-initialized state — one phase sees "done", the other sees "needs init", and no retry heals it.
  • Replace with an explicit marker at /home/opencode/.opencode-init.v1, written last under set -e. Any failure before the marker leaves FIRST_RUN=1 on the next start and the whole first-run block re-runs.
  • Versioned suffix (.v1) gives a clean migration path: bump to .v2 to force re-init on existing PVCs when seeded defaults change in a breaking way.

Drive-by fix

The no-repoUrl path (git init + git commit --allow-empty) used to run unguarded on every pod start, adding a fresh empty commit per restart. Now gated on FIRST_RUN.

What stays "every-start"

Considered alternatives

  • Per-phase markers (.opencode-init.config.v1, .opencode-init.git.v1): more bookkeeping, marginal benefit — set -e already aborts on the first failure so phases never partially complete in a single run.
  • PVC annotation + downward API: source of truth on the control plane, but the router would have to stamp the annotation onto each new pod spec — real complexity for the same end behavior.
  • Separate Job for first-run setup: cleanest architectural fix but a larger rework. Worth doing later if init-time setup grows.

Test plan

  • bun test src/pod-manager.test.ts — 69 pass (2 new tests covering marker placement and no-repoUrl guard)
  • bun run typecheck — clean
  • Manual: start a fresh session pod, confirm /home/opencode/.opencode-init.v1 exists after init; restart the pod, confirm init container completes quickly without re-seeding .config/opencode

🤖 Generated with Claude Code

…ker file
Replaces implicit "is this the first run" heuristics
(`[ ! -d /home/opencode/.config/opencode ]`, `[ ! -d /workspace/.git ]`)
with an explicit, versioned marker at /home/opencode/.opencode-init.v1.
Why: the implicit checks are per-phase and inferred from side-effects.
If a prior init partially completed (e.g. config seed succeeded, git
clone failed with ENOSPC), one phase saw "already initialized" while
the other saw "needs init" — the PVC could get stuck in a half-state
that no retry would heal.
The marker is written LAST under `set -e`, so any failure before that
point leaves FIRST_RUN=1 on the next pod start and the whole first-run
block re-runs cleanly. Bumping the version suffix (v1 → v2) forces a
re-init on existing PVCs, giving a clean migration path when the
seeded defaults change in a breaking way.
Drive-by: the no-repoUrl path is now also guarded. Previously
`git commit --allow-empty` would add a new empty commit on every pod
restart because it ran unguarded.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@mrsimpson

Copy link
Copy Markdown
OwnerAuthor

Closing — reviewing this with fresh eyes uncovered a migration bug and a weak value proposition.

Migration bug: every existing session pod would crashloop on its next restart after this shipped. With no marker present, FIRST_RUN=1 fires, and:

  1. cp -r /etc/opencode-defaults/. /home/opencode/.config/opencode/ silently clobbers user-modified config files.
  2. git clone "$repoUrl" /workspace fails with fatal: destination path '/workspace' already exists and is not an empty directory. → init exits non-zero → crashloop.

Weak value proposition: the marker's main pitch was failure-atomicity. But each init phase is already individually idempotent (mkdir -p, cp -r, the existing [ ! -d ] guards), and the marker doesn't unlock any meaningful "force re-init on version bump" semantics that the every-start phases don't already cover. Bringing back the implicit checks as layered guards to fix the migration would end up at a strictly more complex version of the original code.

#65 already solved the user-visible crashloop. This refactor is mostly aesthetic — not worth the migration risk.

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

@mrsimpson