Skip to content

feat(operator): manifest delivery reconciler (PR-51 of #48) - #51

Merged
flg77 merged 9 commits into
mainfrom
feat/op-pr50-manifest-delivery
May 9, 2026
Merged

feat(operator): manifest delivery reconciler (PR-51 of #48)#51
flg77 merged 9 commits into
mainfrom
feat/op-pr50-manifest-delivery

Conversation

@flg77

@flg77 flg77 commented May 9, 2026

Copy link
Copy Markdown
Owner

Summary

Implements PR-51 of openspec change merged in #48: corpus-scoped ConfigMaps that deliver the operator-baked roles/, skills/, mcps/ trees into agent pods. After this PR, an agent pod created by the operator has the same in-pod filesystem layout that compose-mode pods get from ../../roles:/app/roles:ro,z bind mounts — closing the parity gap with container/production/podman-compose.yml.

Note: this PR was originally planned as #50 in the openspec change, but #50 was taken by an unrelated docs PR (docs(index): cross-link to acc-podman-desktop extension). Renumbered to #51/#52/#53 in commit f485b3d.

What changed

Build-side prep

  • make sync-manifests mirrors ../{roles,skills,mcps} into operator/internal/reconcilers/manifests/data/ so //go:embed can reach them. Filters out __pycache__, *.pyc, *.pyo, .DS_Store to keep the round-trip flat-key contract safe. The data/ dir is gitignored. Wired as a prereq for generate, build, run, test, test-unit, docker-build.

New reconciler

  • internal/reconcilers/manifests/delivery.goManifestDeliveryReconciler implementing the existing SubReconciler interface. //go:embed all:data/{roles,skills,mcps}. Reads each tree via fs.WalkDir, flattens path separators (/__) to satisfy Kubernetes ConfigMap key constraints, upserts three corpus-namespace ConfigMaps named {corpus}-acc-{roles,skills,mcps} via util.Upsert. Sets corpus.Status.ManifestDeliveryReady=true on success.

    Public helpers: FlattenPath, UnflattenKey, ConfigMapName, Suffixes, plus RolesMountPath / SkillsMountPath / MCPsMountPath consts so agent_deployment.go reuses the exact naming and path rules.

  • Slotted into the chain at position 2 (after PrerequisiteReconciler, before UpgradeReconciler) — the CMs must exist before any agent Deployment is built.

Volume/env injection in agent pods

  • buildManifestDelivery helper in agent_deployment.go reads each ConfigMap from the API client and projects keys into items[]: [{key, path: UnflattenKey(key)}], so the in-pod filesystem sees the original slash-paths instead of the flattened storage keys.
  • Appends three VolumeMounts (/etc/acc/{roles,skills,mcps}, read-only), three Volumes with explicit items[] projection, and three env vars (ACC_ROLES_ROOT, ACC_SKILLS_ROOT, ACC_MCPS_ROOT).
  • Returns empty slices when spec.manifestDelivery == "none" (opt-out) or when a CM is not yet present (graceful first-cycle handling — manifest reconciler runs earlier in the chain so this is rare).

TUI parity

  • acc_tui_deployment.yaml gains ACC_ROLES_ROOT / ACC_SKILLS_ROOT env vars and acc-roles / acc-skills ConfigMap mounts at /etc/acc/{roles,skills} (read-only, optional: true to tolerate first-apply race). Closes the parity gap with container/production/podman-compose.yml:464.

Verification

Check Result
make manifests generate exit 0, no diff
go vet ./... clean
go build ./... clean
go test ./test/unit/... (12 manifest tests) all PASS
go test ./internal/reconcilers/manifests/... (4 white-box tests) all PASS
kubectl apply --dry-run=client on TUI sample accepted

Commit graph (8 commits)

  • 8649a98 lock claim
  • f485b3d openspec renumber (drift)
  • baa60f0 Makefile sync-manifests target + .gitignore
  • 72a8650 ManifestDeliveryReconciler
  • 93783ee wire into reconciler chain
  • a6ac7c1 agent_deployment volume + env injection
  • adf1f90 TUI sample parity
  • 6ba5b0f unit tests
  • ed4def0 lock board: PR-51 tasks complete

🤖 Generated with Claude Code

flg77 and others added 9 commits May 9, 2026 22:16
Lock acquisition for the manifest delivery reconciler PR. Single-file
edit pushed immediately so the other Claude instance (10.199.12.8)
sees the claim on next git pull.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Slot #50 went to an unrelated docs PR
(docs(index): cross-link to acc-podman-desktop extension), so the
manifest delivery PR landed as #51, the MCP reconciler will be #52,
and the demo samples PR will be #53.

Updates tasks.md headings, design.md cross-refs, and proposal.md
PR-slot summary. Local branch name (feat/op-pr50-manifest-delivery)
left as-is — already pushed and the PR is open.

This is the second drift encountered on this openspec change, both
absorbed cleanly by the renumber clause. No content changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The ManifestDeliveryReconciler ships next on this branch and uses
//go:embed over operator/internal/reconcilers/manifests/data/. embed
paths can't escape the Go module, so the trees have to be mirrored
into the operator subtree at build time.

Adds:
  make sync-manifests  — mirrors ../roles, ../skills, ../mcps into
                         internal/reconcilers/manifests/data/ and
                         strips __pycache__, *.pyc, *.pyo, .DS_Store
                         (so flat-key round-trip k→k.replace("__","/")
                         in the reconciler is safe).

Wires sync-manifests as a prerequisite for: generate, build, run,
test, test-unit, docker-build. Existing developers running any of
those targets get the mirror automatically.

The mirror is gitignored — it's a build artefact, not source.

Verified: 183 files mirrored from the live trees (47 roles ×
~2-3 files + 14 skills + 5 mcps + supporting docs/data). No
__pycache__ leakage.

Refs openspec/changes/20260508-operator-feature-parity-d-e/tasks.md
PR-51 task 1 (build-side prep).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds operator/internal/reconcilers/manifests/delivery.go: a
SubReconciler that emits three corpus-namespace ConfigMaps holding
the operator-baked roles/, skills/, and mcps/ trees. Agent pods get
these mounted at /etc/acc/{roles,skills,mcps} via volume injection
landing in a follow-up commit on this branch.

Design choices:

  * embed.FS over data/roles, data/skills, data/mcps (mirrored at
    build time by `make sync-manifests`). Build-time embed avoids
    runtime fetches and lets the operator GC ConfigMap ownership
    cleanly via SetControllerReference.

  * ConfigMap data keys flatten "/" to "__" because Kubernetes
    rejects "/" in keys. Round-trip is safe: the sync target strips
    __pycache__ et al, and no real path contains "__". The
    agent_deployment volume's items[] reverses the flatten via
    UnflattenKey() so the in-pod filesystem sees the original tree
    shape.

  * Opt-out gate: spec.manifestDelivery == "none" skips the upsert
    and clears status.manifestDeliveryReady, leaving any
    pre-existing user-managed ConfigMaps alone.

  * Public helpers (FlattenPath, UnflattenKey, ConfigMapName,
    Suffixes, RolesMountPath / SkillsMountPath / MCPsMountPath
    consts) so agent_deployment can reference the same naming and
    path rules without duplicating logic.

Smoke-tested locally: walkTree returns 149 role files, 28 skill
files, 6 mcp files (matches `find ../{roles,skills,mcps} -type f`
after the sync filters), and the round-trip on
coding_agent_implementer/role.yaml is byte-stable.

Refs openspec/changes/20260508-operator-feature-parity-d-e/tasks.md
PR-51 task 2.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Slots the new manifest delivery reconciler immediately after
PrerequisiteReconciler and before UpgradeReconciler. The
acc-{roles,skills,mcps} ConfigMaps must exist before any agent
Deployment is built — slot 2 puts the upsert before the upgrade
pre-checks and before every infra reconciler that doesn't depend on
the manifest tree. Order matches the design.md table for PR-51 wire-in.

Verified: go build ./... and go vet ./... clean.

Refs openspec/changes/20260508-operator-feature-parity-d-e/tasks.md
PR-51 task 3.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires the corpus-scoped acc-roles, acc-skills, and acc-mcps ConfigMaps
emitted by ManifestDeliveryReconciler into every agent pod via:

  /etc/acc/roles   ← corpus.Name + "-acc-roles"   ConfigMap
  /etc/acc/skills  ← corpus.Name + "-acc-skills"  ConfigMap
  /etc/acc/mcps    ← corpus.Name + "-acc-mcps"    ConfigMap

  ACC_ROLES_ROOT=/etc/acc/roles
  ACC_SKILLS_ROOT=/etc/acc/skills
  ACC_MCPS_ROOT=/etc/acc/mcps

The Volume sources use an explicit items[] projection list — keys are
read from the live ConfigMap and projected via
manifests.UnflattenKey(key) so the in-pod filesystem sees the original
slash-separated paths instead of the flattened "key__like__this"
ConfigMap data keys.

Behavioural choices:

  * spec.manifestDelivery == "none" returns empty slices — no mounts,
    no env vars. Lets users bake the trees into a custom agent image
    if they prefer.

  * If a ConfigMap is not yet present at agent_deployment time, that
    tree's mount is skipped (not errored). The manifest reconciler
    runs earlier in the parent chain so this only happens during the
    first reconcile pass; the next cycle picks it up cleanly.

  * Reads CM data from the API client, not from any cached structure,
    so adding a new role at runtime (regenerate + redeploy operator)
    propagates into the items[] list and triggers a Deployment update
    via the Upsert MergeFrom patch.

go build ./... and go vet ./... clean.

Refs openspec/changes/20260508-operator-feature-parity-d-e/tasks.md
PR-51 task 4.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the parity gap with container/production/podman-compose.yml:464
(TUI service, ACC_ROLES_ROOT=/app/roles): the operator-managed TUI
Deployment now reads the same operator-baked roles/, skills/ trees
that agent pods do, so the Ecosystem screen has data to render in
operator-mode just as it does in compose-mode.

Adds:
  env vars  ACC_ROLES_ROOT=/etc/acc/roles, ACC_SKILLS_ROOT=/etc/acc/skills
  mounts    /etc/acc/roles  ← {corpus}-acc-roles  ConfigMap (read-only)
            /etc/acc/skills ← {corpus}-acc-skills ConfigMap (read-only)

The ConfigMap volumes are mounted with optional: true so first-apply
races against the manifest reconciler don't ImagePullBack the TUI;
the next reconcile cycle resolves both objects cleanly.

Documented caveat in the file header: the operator flattens "/" → "__"
in CM keys (Kubernetes rejects "/"), so without an items[] projection
the TUI sees flat-keyed entries. Acceptable for the Ecosystem screen
which only enumerates role names; a TUI screen needing the nested
shape should call the role_loader's operator-mode helper that splits
"__" back to "/" — tracked separately.

Refs openspec/changes/20260508-operator-feature-parity-d-e/tasks.md
PR-51 task 5.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds two test files:

  internal/reconcilers/manifests/delivery_test.go  (white-box, 4 tests)
    - walkTree returns the right keys for a known persona spread
      (analyst legacy, coding_agent_implementer D3, research_planner E4)
    - round-trip safety: every flat key unflattens back to a valid path
      with no spurious "/" introductions, across all three trees
    - FlattenPath / UnflattenKey direct round-trip on six explicit cases
    - Suffixes() returns the exact strings agent_deployment relies on

  test/unit/manifest_delivery_test.go  (black-box, 4 tests)
    - End-to-end Reconcile against a fake controller-runtime client:
      asserts three corpus-scoped ConfigMaps are created with non-empty
      Data and the operator-managed manifest-tree label
    - ConfigMap keys never contain "/", and unflatten produces multi-
      segment paths (proving the projection contract holds)
    - spec.manifestDelivery="none" short-circuits the reconciler:
      no CMs created, status.ManifestDeliveryReady stays false
    - Idempotency: two Reconcile calls leave Data sizes equal

go mod tidy was needed (fake client pulled in
gopkg.in/evanphx/json-patch.v4 as an indirect dep — already present
in go.sum via transitive github.com/evanphx/json-patch).

Refs openspec/changes/20260508-operator-feature-parity-d-e/tasks.md
PR-51 task 6.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
All seven PR-51 tasks (1 build prep + 6 plan items) landed across
commits baa60f0, 72a8650, 93783ee, a6ac7c1, adf1f90, 6ba5b0f. Updates
the lock board to status=ready-for-review.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@flg77 flg77 changed the title feat(operator): manifest delivery reconciler (PR-50 of #48) feat(operator): manifest delivery reconciler (PR-51 of #48) May 9, 2026
@flg77
flg77 marked this pull request as ready for review May 9, 2026 20:40
@flg77
flg77 merged commit 57e4033 into main May 9, 2026
@flg77
flg77 deleted the feat/op-pr50-manifest-delivery branch May 9, 2026 20:43
flg77 added a commit that referenced this pull request May 31, 2026
…ible state (v0.3.40) (#6)

Closes follow-up #51.

PR-MEM2 shipped in v0.3.1 but `acc/agent.py::_reflection_loop` is
gated on `ACC_REFLECTION_INTERVAL_S > 0` and the baseline production
compose never set it. Reflection loop returned immediately; the entire
memory consolidation chain (episodes → reflection → memory_notes →
Redis hot-cache → PR-I retrieval) has been silently disabled on every
default deploy since v0.3.1. It only surfaced now because v0.3.37
unblocked the episode-persist producer side — episodes finally flow
(verified `episodes` rows = 5 on lighthouse) but memory_notes stays 0.

Three coupled changes:

1. `container/production/podman-compose.yml` — adds
   `ACC_REFLECTION_INTERVAL_S: ${ACC_REFLECTION_INTERVAL_S:-600}` to
   every agent service's `environment:` block (baseline + coding-split
   profile). 10-min default with operator override.

2. `acc/agent.py::_reflection_loop` — boot-time INFO log lines so the
   on/off state is operator-visible. Pre-v0.3.40 the loop was silent
   when disabled, hiding the fact that `memory_notes` was empty by
   configuration rather than failure. Now logs either:
     memory_reflection: disabled (ACC_REFLECTION_INTERVAL_S=0)
   or:
     memory_reflection: enabled interval=600s role=assistant agent_id=assistant-1

3. `tests/test_reflection_loop.py` — 4 new env-gating regression tests:
   - skipped when env unset (with disabled log line)
   - skipped when env="0" (with disabled log line)
   - enabled logs interval + role + agent_id
   - silent skip when cognitive_core is None (dormant worker contract
     preserved)

Downstream consumers unblocked:
- PR-I memory retrieval (queries memory_notes)
- ACC-6a/11 centroid drift (uses memory_notes as priors)
- SIP-P3 contextual prior (last_eval_reward + memory-informed)
- Dreamer Phase 2 (M1 dedup, M2 prune) — operates on memory_notes

Tests: 11 passing in tests/test_reflection_loop.py; 278 passing across
the broader memory/agent/reflection sweep.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
flg77 added a commit that referenced this pull request May 31, 2026
…rue (v0.3.41) (#7)

Continuation of followup #51.

v0.3.40 enabled the reflection LOOP (`ACC_REFLECTION_INTERVAL_S`
default 600 in baseline compose + INFO log on boot). But
`_run_reflection_once` is ALSO gated on the per-role
`memory_reflection` flag, and:

  - `RoleDefinitionConfig.memory_reflection: bool = False` (default)
  - Zero role.yaml files flip it on (`grep -rn memory_reflection roles/`
    returns nothing)

So even with the loop firing every 600s, no role ever actually
consolidated. Two-layer silent gate.

Verified live on lighthouse 2026-05-31 post-v0.3.40:
  ACC_REFLECTION_INTERVAL_S=60, boot log shows "memory_reflection:
  enabled interval=60s role=assistant"; sent a smoke task → episodes
  rose from 5 to 6 → waited 70s for the cycle → memory_notes stayed 0.
  Reflection loop firing, but `_run_reflection_once` early-returns.

Fix: flip the default from False to True. Roles that genuinely don't
want reflection (arbiter is a plausible candidate — pure cluster
control, doesn't reason on tasks) can opt out per role.yaml with
`memory_reflection: false`.

Cost: one extra LLM call per reflection_interval_s window per active
role. With the 600s default that's ~6 LLM calls/hour per agent — well
within the budget every existing role already consumes for tasks.

Test update: `test_role_memory_reflection_defaults_false` →
`test_role_memory_reflection_defaults_true` with opt-out assertion.

Full sweep: 2417 passed, 38 skipped, 0 failed.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
flg77 added a commit that referenced this pull request Jun 11, 2026
…ers #51) (#60)

* feat(webgui): WS-C2 — role authoring endpoints (proposal 020)

WebGUI parity with the TUI Ecosystem editor for role authoring. Reuses
acc.tui.role_writeback so the WebGUI and TUI share ONE validate +
atomic-write path.

acc/webgui/routes_roles.py:
  - GET  /api/roles/{id}/yaml + /md — read for the editor (viewer)
  - PUT  /api/roles/{id}/yaml — validate + atomic write; RoleValidationError
    → HTTP 400 carrying {message, errors} (operator)
  - PUT  /api/roles/{id}/md — free-form narrative write (operator)
  - POST /api/roles — create a new role dir + validated role.yaml +
    optional role.md; rolls back the dir on validation failure; 409 on
    duplicate (operator)
  - path-safe role_id (regex-gated, blocks traversal); ACC_ROLES_ROOT
    resolution consistent with CapabilityIndex / acc-tui.

Publish-to-catalog (WS-C3) is deferred — gated on the signing-identity
decision (proposal 020 open question: trigger Tekton pipeline vs mounted
token vs keypair).  The React Marketplace/Catalogs/Role-editor screens
(WS-C1) wrap these + the existing endpoints next.

tests/test_routes_role_authoring.py — 12 tests (read/404, edit
valid/invalid-400-with-errors/404, md write, create/409/rollback,
invalid-id rejection), mirroring the test_routes_roles.py fixtures.

Sweep: 3071 passed, 69 skipped.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(webgui): WS-C1 — React Marketplace + Catalogs + Role editor (proposal 020)

Completes the WebGUI side of proposal 020's "feature parity TUI↔WebGUI"
goal: browse the catalog, manage layered catalogs, and author/edit roles
from the web frontend (over the WS-C2 authoring backend, this branch's base).

Backend (acc/webgui/routes_roles.py) — two small read endpoints the
editor needs:
* GET /api/roles            — list authorable in-tree roles (picker source;
                              excludes _base + dirs without role.yaml; flags
                              role.md presence)
* GET /api/roles/{id}/md    — read role.md (empty text, not 404, when absent
                              so the editor can start a narrative)

Frontend (webgui/src/):
* api/client.ts  — put/delete/patch helpers + typed fns for marketplace,
                   catalogs CRUD, and role authoring (list/get/put/create)
* screens.tsx    — Marketplace (browse + stage install → Compliance queue),
                   Catalogs (add/remove/re-prioritise workspace catalogs),
                   Role editor (open existing role.yaml+role.md or create
                   new; server-side validation errors surface inline)
* App.tsx        — register the three screens in the nav
* styles.css     — monospace .code textarea for the role.yaml editor

Tests: tests/test_routes_role_listing.py (7) — listing excludes _base +
no-yaml dirs, flags has_md, empty when root absent; md read returns text /
empty / 400 on bad id.  React layer verified via `npm run build`
(tsc -b + vite) — green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.

1 participant