Plan for refactoring to single go binary - #51
Closed
grubmeshi wants to merge 32 commits into
Closed
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…kers Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-binary/ Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ent coverage, D12-D14, phase 2b) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…me-origin pin Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…DE contract in D9 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…metric names frozen in D12 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…stry, atomic consolidation) and D12 (no listener in single-run) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ncy) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…phase-6 umbrella plan; D9 SPRING_PROFILES_ACTIVE alias, acceptance-gap fallback Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…alCallError) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
grubmeshi
commented
Jul 9, 2026
grubmeshi
commented
Jul 9, 2026
grubmeshi
commented
Jul 9, 2026
grubmeshi
commented
Jul 9, 2026
grubmeshi
commented
Jul 9, 2026
grubmeshi
commented
Jul 9, 2026
grubmeshi
commented
Jul 9, 2026
grubmeshi
commented
Jul 9, 2026
grubmeshi
commented
Jul 9, 2026
grubmeshi
commented
Jul 9, 2026
grubmeshi
commented
Jul 9, 2026
grubmeshi
commented
Jul 9, 2026
Grill round 2 rulings (all marked "RULED (grill r2)"):
- R12 exit non-zero scoped to pre-mutation failures only (plan 02, plan 05 §16.3)
- data-race B6/B10 fixed in phase 2, -race on (D13 exception, plan 02 §5.5)
- phase-2 exit adds polling + single-run runtime smokes (plan 02 §6)
- go-meshapi-client CI leg approved for phase 0 (plan 00)
- config: defaults < shared base YAML < per-impl YAML < env deep-merge (D7, plan 03/04)
- env compat: literal spellings only, fail-fast on unconsumed legacy-prefix vars (D7, umbrella)
- graceful shutdown: cancel sync-polling runs, terminal ABORTED (fallback FAILED, never
SUCCEEDED), configurable 120s grace; ABORTED added to report.ExecutionStatus (D9, plan 03/05/06C/06D)
- D5 fail-fast reporting uses process credentials (plan 05)
- single-run exit-code tightening fixed in phase 6, old behavior pinned (umbrella, 06A-D)
- composite value formatting -> compact JSON, flagged byte change (umbrella, 06B/06C)
- GitHub App JWT: stdlib RS256, no golang-jwt dep (umbrella, 06D)
- dev key kept verbatim in shared base config, phase-7 removal item (umbrella, 06B, 07)
- L14 controller decrypt-failure FAILED report approved for phase 7 (07)
- slog-native for plans 03/04/05; phase-7 logging migration reduced to tf/tfrun (D15, 03/04/05/07)
PR#51 review reflections (marked "PR#51"):
- P4 pointer rule scoped to composite types; P7/D16 unit-tests-sparingly; P8 cohesive types
- D6 test infra -> httptest server mock + bare-repo testdata (plan 01 keeps fake transport in phase 1)
- D7 typed config struct + ${VAR} YAML interpolation; existing RUNNER_*/TF_* spellings kept (TF_ passed to tofu)
- plan 02 git.LocalRepo + tf Init/Plan/Apply/Destroy domain steer
- plan 05 capacity guard is k8s-independent; RunHandler purity (manual = inputs->outputs, no HTTP)
- retry/backoff confirmed in scope; client unification deferred
- mux obsolescence a downstream goal; removal tracked as separate meshfed-release item
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… follow-up)
Verified two assumptions against the actual repos, findings folded into the plans:
- ABORTED (meshfed-release): the runner-facing PATCH .../status/source/{sourceId}
endpoint accepts an inbound ABORTED and persists it terminal via the coordinator.
Accepted transition is IN_PROGRESS->ABORTED (exactly the graceful-shutdown case);
PENDING is the only runner-forbidden status; an already-aborted run returns
409 {runAborted:true} (treat as success). FAILED fallback stays valid. Removed the
"assumption" hedge in D9 / plan 03 enum / plan 05 H7 and pinned the two constraints.
- Smoke coverage (meshstack-smoke-test): tf/manual/github confirmed covered (tf in the
smoke-test repo; github_workflows + manual e2e modules live in meshstack-hub and are
discovered/run by the harness); gitlab and azure-devops confirmed NOT covered.
Corrected the repo name (meshstack-smoke-test, singular) and the hub split in the
umbrella deletion gate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…port
Resolved the deferred RunHandler-purity design question via a grill interview,
grounded by verifying runner reporting shapes (Kotlin sources) and meshfed step-merge
semantics. Threaded the decision across all plans (marker "Grill r3 (RunHandler purity)"):
- Handler purity boundary: a handler MAY read the meshapi client's DTOs and consume its
use-case/domain API; purity = it never assembles its own HTTP transport/auth. The
reporter is injected as a use-case-level port (D4, plan 05 §4.2).
- ONE unified Reporter interface consumed by all five runners:
type Reporter interface { Register(RunStatus) error; Report(RunStatus) (abort bool, err error) }
The four event-driven ports (manual/gitlab/azdevops/github) discard the abort return
and run no Observer; tf alone drives the Progress+Observer 10s ticker and honors abort.
- Report(RunStatus) sends only the changed/new steps present; the meshfed endpoint upserts
steps by id (verified: BlockRunSourceUpdateService merges by id, never replaces). Step
messages stay cumulative-replace (full per-step text; backend overwrites by assignment,
not append). The lean SourceUpdateDTO/StepUpdateDTO remain the wire body.
- tf switches from full-snapshot to changed-steps-only sends in PHASE 3 — a deliberate,
flagged wire change, backend-result-identical, so acceptance stays green while tf's
phase-1 HTTP transcript pins are updated. Phase 2 keeps tf's full-snapshot send.
- Replaced plan 06A's bespoke report.SourceReporter{Register,Update} with the unified
Reporter, and reconciled all 06B/06C/06D references (heaviest dedup consumer: azdevops)
to it; impl-secret decryption stays handler-owned (manual none), external clients+Clock
via per-runner constructors, Execute(ctx, ClaimedRun) unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…unner superset (grill r4) Challenged and reversed the single-argv[0]-binary design (D2/D8). Grounded in the dependency reality: run-controller links k8s client-go and NOT go-git/tofu; tf-block-runner links go-git+terraform-exec and NOT k8s — disjoint heavy trees, so one fat binary shipped each into images that never use it (bloat + SBOM/CVE noise on public images). New shape (marked "Grill r4"): - One Go module ./runner, shared internal/* packages (unchanged — the refactor's value). - Fit per-persona binaries cmd/<persona>/main.go (cmd/tf + the four runner personas), each linking only its deps; direct image entrypoints, no argv[0]/symlink multiplexing. - cmd/bbrunner = the superset AND the controller (amended: controller ≡ superset): links all handlers + both dispatchers and auto-detects the in-cluster k8s API (rest.InClusterConfig()/KUBERNETES_SERVICE_HOST) → KubernetesJobDispatcher in-cluster, InProcessDispatcher otherwise; RUNNER_DISPATCHER overrides. Shipped AS the run-controller image (one adaptive image; saves a whole persona). No separate cmd/controller. - Sequencing: phase 4 creates cmd/tf + cmd/bbrunner (controller, k8s dispatch); phase 5 adds InProcessDispatcher + auto-detect; handlers accrete through phase 6. - Threaded across D1/D2/D8/D11, §1, phase-4/§7, and plans 02-07 (cmd/ layout, Dockerfile matrix, CI/release, dispatcher story, phase-6 template, cleanup). Terminology: the concept is now "single Go module" (title + prose); branch names (refactor/single-go-binary/*) and the PLAN_DETAIL_04_single_binary.md filename kept as-is (open PR references them). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…spec Sweep across all plans removing the plan-creation history (grill-round tags, PR#51 refinement labels, "RULED"/"VERIFIED"/"self-grilled" framing, "this replaced/amended/ superseded an earlier design" narration, and resolved reviewer-may-veto/open-question scaffolding). Every technical decision, guideline (P1-P8, D1-D16), behavior pin, interface, contract, STOP marker, and assumptions-table is preserved as plain current spec; redundant restatements collapsed to precise cross-references. Net ~157 lines removed (622 ins / 779 del across 13 files); process tags now 0. Also reconciled two internal inconsistencies surfaced during the sweep: - plan 02: removed a stray "-race turns on in phase 2" clause that contradicted plan 02 §5.5 pt3 / 2b R1 / plan 03 A5+A7 (races fixed structurally in phase 2; -race gate flips in phase 2b). Substantive decision unchanged. - umbrella §5.5/§5.6/A7/rollback: rewrote leftover single-runner.Dockerfile+target-stage base prose to the per-persona containers/<persona>/Dockerfile model already stated in 06A/06B (it had contradicted the sub-plans after the r4 pivot). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cess 3, k8s 10) Module layout (Change 1): the single consolidated Go module now lives at the repo root, not a ./runner/ subdir. Module path becomes github.com/meshcloud/building-block-runner (packages internal/*, binaries cmd/*). The Go workspace is removed entirely at phase 4 — go.work + go.work.sum are deleted outright in the consolidation step; there is no workspace end-state and no intermediate `use ./runner`. Threaded through git-mv destinations, Dockerfile (COPY go.mod go.sum, root build context, ldflags .../internal/build), CI (go-version-file: go.mod, test leg at root over ./...), Taskfile (no dir: runner/), depguard prefixes, coverage thresholds/exclusions, and the cross-repo local-dev-stack SKILL. Added the note that the root module coexists with the still-Kotlin/Gradle runner dirs (separate build systems, deleted phase 6/7). Default concurrency (sanctioned behavior changes): - Standalone in-process maxConcurrentRuns default 1 -> 3 (Change 2): the tf and all phase-6 in-process personas run up to 3 concurrent runs by default; =1 restores today's exact serial cadence. Makes the H1-H8 concurrency paths default-on. - k8s controller maxConcurrentJobs default 20 -> 10 (Change 3): sole exception to the run-controller persona's bit-identical guarantee. Consistency sweep: reconciled the module READMEs to a single root README.md; propagated default-3 into all phase-6 plans (they still cited "default 1 (plan 05)") incl. the azdevops sync-poll blocking argument; fixed plan 05 A1 which listed the four runner mains as phase-4 deliverables (they arrive in phase 6). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Docker model (task a): phase 4 now creates two standalone per-app Dockerfiles (containers/run-controller/Dockerfile builds ./cmd/bbrunner, containers/tf-block-runner/Dockerfile builds ./cmd/tf) — no shared containers/runner.Dockerfile, no --target, direct ENTRYPOINT — uniform with the per-app model phases 6/7 already assume. Updated STOP-D, §4.4, §4.5, §5 step 5, §7.2, §11; reframed the 06C/06D step-image lines off their leftover `docker build --target …` residue. Ref-integrity sweep (5 partitioned audits): fixed 9 dangling/mis-resolving cross-references — plan 02 "§4.1 item 3"→"§3.1 item 3"; plan 04 "(§5.4)"→"(§6)" and "flag §10.6"→"flag §10.8"; plan 05 "depguard §5.3"→"depguard §5"; umbrella A8 "Plans 00/02/04 §7.1"→"plan 00 §5.4, plan 02 §7, plan 04 §7.1"; and dropped the stale open-question Q2/Q3/Q4 pointers left by the de-historicize sweep (plans 01/02), keeping their valid companion refs. The bare umbrella-§ shorthand in the 06 sub-plans is confirmed established house style (present in untouched siblings), left as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… smokes + opt-in controller kind e2e Closes the gap where the high-level plan mandated a reusable httptest meshfed-API mock but no detail plan built it, and where "does the runner actually work" and the controller's k8s dispatch were never exercised end-to-end. (a) Shared mock server: new plan 03 §5.7 defines `meshapitest` — a net/http/httptest meshfed-API mock server (claim/register/PATCH/artifact, real HTTP, seedable runs + captured requests), phase-3 deliverable at go-meshapi-client/meshapitest → phase-4 internal/meshapitest. High-level D6 and plan 01's deferral now point at it; it is the single owner. Reused by phases 4–7. (b) Mock-backed container smoke: plan 04 step 5 and every phase-6 §8 image step now docker-run the built image against a meshapitest server (host networking, RUNNER_API_URL→mock) and assert a full claim→execute→report cycle — not just healthz. Rule stated once in umbrella §5.6, inherited by all four sub-plans; per-persona integration tests use meshapitest for the meshStack side (external-service httptest fakes unchanged). (c) Opt-in controller kind e2e (plan 07 §5.4): a //go:build e2e test that stands up a kind cluster, deploys run-controller, points it at a meshapitest coordinator, seeds a claimed run, and asserts a real k8s Job is created (dispatched tf image runs single-run). Runs under task test:e2e:controller, never a PR gate; closes the plan 03 §12.3 / plan 05 §13 controller-e2e gap. meshfed-release has no turnkey minikube (docker-compose local-dev); harness-shape pattern = replicator fabric8 KubernetesTestData. Plan 05 §7 concurrency suite also moves onto meshapitest. Refs threaded consistently (plan 03 §5.7, plan 07 §5.4 qualified everywhere; umbrella's own §5.7 Acceptance left intact). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…client's Logger seam
LOG_LEVEL (debug|info|warn|error, default info) sets the process slog handler level for
every persona. config.LogLevel helper ships in plan 03 §5.3; main wires the handler level
in plan 04 §4 / plan 07 §8.1. Plan 04 scope note updated (two new env vars now:
MANAGEMENT_PORT + LOG_LEVEL).
DEBUG HTTP wire logging (plan 03 §5.2.6): at LOG_LEVEL=debug the shared meshapi client
logs full request/response headers AND bodies including sensitive values (runToken bearer,
claim payloads, decrypted inputs, PATCH messages) — unredacted by design (opt-in
diagnostic, off by default; §8 records the trade-off). Artifact-download bodies are
excepted (metadata only) so DEBUG can't blow up on a 128MiB stream.
Reuse from terraform-provider-meshstack (D3, §7): the provider already implements this in
client/internal/{logging.go,http_client.go} — a pluggable Logger interface
(Debug/Info/Warn(ctx,msg,args...), noopLogger default) + request/response Debug logging
with sorted headers and pretty-JSON body via lazy fmt.Stringer wrappers. We copy the
interface + loggedHeaders/loggedBody/bytesToPrettyJson verbatim into meshapi so the future
shared meshstack-go-sdk merges with NO logging-seam delta. The meshapi client takes this
Logger interface (not *slog.Logger); main injects a meshapi.SlogLogger adapter, keeping the
app slog-native while the SDK-bound client stays portable.
Two conflicts flagged and resolved:
- The provider REDACTS Authorization ([REDACTED], logging.go:47-50); we drop that redaction
per the no-obfuscation requirement (one-line policy diff, recorded in §3.5/§7/§8).
- The provider is JSON-only (no streaming download); our DownloadArtifact stream is kept
out of the body logger (our addition).
Recorded in §3.5 (adopt list), §7 (alignment: logging merge cost now "none"), and the §5
slog-native note (meshapi's Logger seam is the carve-out from *slog.Logger).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… logging exception Reverses the earlier deviation: instead of dropping the provider's Authorization-header redaction, we keep meshapi's copied loggedHeaders verbatim — Authorization stays [REDACTED] as the single sanctioned exception to the "don't obfuscate secrets in logs" policy. Request/response bodies remain fully unredacted at LOG_LEVEL=debug (a runToken in a claim body is still shown); only the Authorization header value is masked. Net effect: zero deviation from the provider code, so the §7 SDK-merge logging cost is genuinely nil. Updated plan 03 §5.2.6/§3.5/§7/§8 and the wording in plan 04 §4, plan 07 §8.1, high-level. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PLAN_IMPL_GUIDE.md (orchestration docs only — no planned gate is changed) plus .claude/workflows/implement-refactor.js: a full-autonomous, per-phase Claude Code workflow that verifies each phase's assumptions, fans out parallel sub-agents partitioned to disjoint files (haiku/sonnet), then reconciles with an opus consolidation agent into one green branch + draft PR. Model tiering keeps ~73% of ~66 agents off opus; opus is reserved for the 9 consolidators, the run-log, and the 8 hard implementer slots. The run never halts: it resolves assumption failures, conflicts and the phase-6 rebase-stack itself, recording only genuinely uncertain calls to PLAN_IMPL_RUN_LOG.md for human review. Agents never edit outside this repo; cross-repo changes are handed off via CROSS_REPO_TODO.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…edger reading) Human-facing reading of the decision ledger from the phases 0→7 autonomous run (wf_8826f73f-1bb). Leads with a "Review this first" list: 5 BLOCKER items (the deferred phase-2/3 DDD debt carried across five phases; phase-3 steps 6–11 + missing NewMetricsCollector(reg) seam; phase-5 tf-persona concurrency cutover not delivered; phase-7 L14 decrypt-FAILED wire change without STOP-D sign-off; the infeasible `log` depguard + unenforced D11 layering), 5 judgment calls, the cross-plan inconsistencies agents could not fix on the plan branch, a per-phase summary with PR links, and an honest "what merely made it fit" section. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The recurring failure across the run was consolidators shipping green-but-INCOMPLETE PRs — reconciling only the slices the fan-out produced and deferring the phase's remaining plan steps to the uncertainty log instead of finishing them (4 phases missed their own exit criteria this way). Instruct every phase's consolidator to IMPLEMENT the missing steps so the stated exit criteria actually hold before opening the PR, keeping the tree green (task test / task lint / -race / coverage), and to record an uncertainty only when it genuinely cannot be completed (needs live meshStack/cluster/Gradle or a frozen contract) with a precise reason. Sets exitCriteriaMet=true only when the criteria truly hold. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is WIP and is never going to be merged. Only there to review the planned refactoring, which is huge!