Skip to content

refactor(open-core): skip private submodules in recursive init, public ENTERPRISE.md, edition in /api/version (#1443) - #1448

Merged
dolho merged 2 commits into
devfrom
feature/1443-enterprise-seam-dx
Jul 6, 2026
Merged

dolho merged 2 commits into
devfrom
feature/1443-enterprise-seam-dx

Conversation

@vybe

@vybe vybe commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Frictionless OSS clone: both private submodules (.claude, src/backend/enterprise) now carry update = none in .gitmodules — a fresh public clone + git submodule update --init --recursive (and git clone --recurse-submodules) completes without credentials; git skips them with exit 0. Verified end-to-end in clean-room clones.
  • Public seam doc: new docs/ENTERPRISE.md documents the generic open-core mechanism only (mount, HTTPS-PAT override, rebuild, verify) — passes the enterprise-docs-guard grep (run locally).
  • Edition surface: GET /api/version now returns edition: "oss" | "enterprise" plus enterprise_features, both derived from entitlement_service.list_entitled_features() — the same source as feature-flags, so the two surfaces can't diverge. Reflects live registration state, not submodule-on-disk. Drive-by: the hardcoded runtimes list was stale (missing codex, Codex harness MVP — pluggable agentic execution engine alongside Claude Code #1187).
  • Deploy pipeline hardened: under update = none a skipped submodule exits 0, so deploy-dev.yml's old exit-code success check would have silently degraded the dev deploy to OSS-only with a green log. It now persists the durable update = checkout local override before init and judges success by the populated marker file.
  • Contributor docs: CLAUDE.md .claude setup rewritten to the verified config-first flow (a one-shot --init --checkout copies none into local config, so later plain updates silently skip again — the config line must come first), plus an external-contributor path via the public abilities dev-methodology plugin. README gains a one-line pointer.

⚠️ For entitled operators (existing clones)

Existing clones have no protective local update key for the enterprise submodule (unlike .claude, whose update = checkout was copied at init). After this lands, plain git submodule update starts skipping it — run once:

git config submodule.src/backend/enterprise.update checkout

(Documented in docs/ENTERPRISE.md; trinity-ops-public runbooks should pick this up too.)

Changes

  • .gitmodules, .github/workflows/deploy-dev.yml, .github/workflows/build-without-submodule.yml
  • src/backend/main.py (_build_version_payload + get_version — builder stays stdlib-pure for the exec-slice test harness; handler imports entitlement_service function-locally so _set_for_testing stubs work)
  • docs/ENTERPRISE.md (new), README.md, CLAUDE.md
  • docs/memory/requirements/infrastructure.md (§35.2), docs/memory/architecture.md, docs/memory/learnings.md
  • tests/unit/test_926_version_endpoint.py (+3 tests, call sites updated), tests/registry.json

Test Plan

Fixes #1443

🤖 Generated with Claude Code

…c ENTERPRISE.md, edition in /api/version (#1443)

- .gitmodules: update = none on both private submodules (.claude,
  src/backend/enterprise) — a fresh OSS clone + `git submodule update
  --init --recursive` completes without credentials (verified end-to-end,
  incl. `git clone --recurse-submodules`)
- deploy-dev.yml: persist the local update=checkout override before init
  and judge success by the populated marker file — a skipped submodule
  exits 0, so the old exit-code check would have silently degraded the
  dev deploy to OSS-only with a green log
- docs/ENTERPRISE.md (new): generic open-core seam install/verify doc —
  config-first opt-in (a one-shot --init --checkout copies `none` into
  local config and later plain updates silently skip again), HTTPS-PAT
  URL override, rebuild, verification via boot line / feature-flags /
  edition; existing-clone migration note (enterprise clones carry no
  protective local update key — .claude clones do)
- GET /api/version: edition "oss"|"enterprise" + enterprise_features,
  both from entitlement_service.list_entitled_features() (same source as
  feature-flags — surfaces cannot diverge); function-local import so
  _set_for_testing stubs aren't bypassed; builder stays stdlib-pure with
  the values threaded as params; runtimes list un-staled (+codex, #1187)
- CLAUDE.md: rewritten .claude one-time setup (config-first, 3 commands)
  + external-contributor path via the public abilities dev-methodology
  plugin; README: pointer note (submodules optional, no credentials)
- build-without-submodule.yml: assert /api/version edition=oss (only
  end-to-end check of the handler wiring)
- tests: edition/enterprise_features passthrough, runtimes list, static
  guard for the function-local import; registry entry; requirements
  §35.2 + architecture.md updated; learnings entry on update=none
  semantics

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vybe
vybe requested a review from AndriiPasternak31 as a code owner July 4, 2026 17:12
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

…d), stale-submodule drift warning, prod-overlay comments

Adversarial /review findings on PR #1448:

- F1 (high): ENTERPRISE.md step 3 was factually wrong — the backend
  Dockerfile COPY allowlist excludes enterprise/, so a rebuild never bakes
  the submodule in; enterprise reaches the container ONLY via bind-mount.
  Rewritten: dev stack = restart (src/backend is already mounted at /app);
  prod stack = add the docker-compose.prod.enterprise.yml overlay to every
  invocation. Following the old text on prod dead-ended at "submodule not
  present".
- F2 (medium): deploy-dev.yml — a failed update on a previously-mounted VM
  leaves the OLD checkout, so the populated-marker check printed a green
  "initialized at <sha>" for a stale tree. Now compares the checked-out
  SHA against the recorded gitlink and emits a ::warning:: on drift.
- F3 (medium): docker-compose.prod.enterprise.yml comments told operators
  to run submodule commands that now silently no-op under update=none
  (verified: --remote also skips). Comments now require the one-time local
  update=checkout override and point at docs/ENTERPRISE.md.
- F4 (low): docs/learnings overstated the trap as specific to
  --init --checkout — ANY init path (plain --init, clone
  --recurse-submodules) copies update=none into local config; corrected in
  ENTERPRISE.md, CLAUDE.md, learnings.md.
- F5 (low): passthrough test now includes a deliberately mismatched
  edition/features pair so a builder that re-derives edition fails.

Cross-repo follow-up filed: trinity-enterprise#88 (ENTERPRISE_LOCAL_DEV.md
documents the same now-skipping commands; lives in the private submodule).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vybe

vybe commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

/review (adversarial, second pass) — 2 findings fixed in 55fb35b6, 3 more resolved as docs/comment corrections; no criticals remain.

All git mechanics were re-verified empirically by an independent reviewer (10 throwaway-repo experiments — config-key syntax, init-copy semantics, pre-existing-clone asymmetry, --remote skip, clone --recurse-submodules, recurse×none interaction): every documented command works as written.

Fixed on the branch:

  • F1 (high): ENTERPRISE.md step 3 said "rebuild the backend" — factually wrong (the image's COPY allowlist excludes enterprise/; the code arrives via bind-mount only). Rewritten with the dev-restart vs prod-overlay split. Following the old text on a prod stack dead-ended at "submodule not present".
  • F2 (medium): deploy-dev.yml could print a green initialized at <sha> for a stale tree (marker file survives a failed update on a previously-mounted VM). Now warns on checked-out-SHA vs recorded-gitlink drift.
  • F3 (medium): docker-compose.prod.enterprise.yml comments documented update procedures (--init --recursive, --remote) that silently no-op under update = none; now require the one-time local override.
  • F4/F5 (low): docs wording corrected (ANY init path copies none into local config, not just --init --checkout); passthrough test now pins the property with a mismatched edition/features pair.

Follow-ups filed: trinity-enterprise#88 (private ENTERPRISE_LOCAL_DEV.md documents the same now-skipping commands), #1450-class comment-disclosure issue (main.py seam comment; pre-existing, out of scope here), #1447 (pre-existing stale test, unrelated).

@dolho

dolho commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Review — LGTM ✅ (minor nits, none blocking)

Read the full diff + verified against the tree. Clean, well-tested, all 18 CI checks green (incl. backend boots without enterprise submodule asserting edition=oss end-to-end). The three empirically-verified git behaviors in learnings.md (skip exits 0, --init copies the strategy local, asymmetric protection of existing clones) are exactly the traps a naive rollout hits — good that each is handled in code, not just documented.

Verified during review:

  • _build_version_payload has a single caller (the handler) — both sides updated, no stale callsite. ✅
  • entitlement_service.list_entitled_features() exists; edition derives from its non-emptiness → provably can't diverge from enterprise_features. ✅
  • Function-local import matches the routers/settings.py pattern and is pinned by the new static guard test. ✅
  • Runtimes ["claude-code","gemini-cli","codex"] match KNOWN_RUNTIMES in runtime_adapter.py exactly. ✅
  • No CI workflow depends on .claude auto-init, so flipping it to update = none is safe. ✅

Nits (follow-up, not blocking):

  1. runtimes list is still hardcoded — this PR's drive-by fixes a staleness (codex missing) that recurs by construction; the next runtime will re-stale it. I get why it isn't derived from KNOWN_RUNTIMES (that lives in the agent-server tree and the builder is deliberately stdlib-pure for the exec-slice harness). Suggest a one-line comment next to the literal pointing at runtime_adapter.KNOWN_RUNTIMES as the source of truth, so the next person greps their way to updating both.

  2. F2 drift warning is the right instinct — comparing the recorded gitlink SHA vs the checked-out SHA catches "populated but stale after a failed update." It only ::warning::s (non-fatal), which is consistent with enterprise-being-optional, but it does mean a stale enterprise tree can still ship under a green deploy. That's an acceptable tradeoff given the policy; just flagging that the warning is the only signal — worth making sure someone actually watches deploy-dev warnings.

  3. Existing-operator migration (the git config submodule.src/backend/enterprise.update checkout one-liner) is handled well and documented in three places. Confirm trinity-ops-public runbooks get the same note, as the PR body suggests — that's the one surface outside this repo that silently changes behavior.

Nice work on the docs — ENTERPRISE.md is thorough, guard-compliant, and the HTTPS-PAT path avoids ever committing a token. Approving from my side once the runtimes comment (nit 1) is added or explicitly waved off.

@dolho
dolho merged commit 54bac2a into dev Jul 6, 2026
20 checks passed
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