build/devenv: serialize raw phased output map, version-aware load#1140
Merged
Conversation
This was referenced May 29, 2026
winder
force-pushed
the
will/devenv-phased-no-cfg
branch
from
4304da6 to
e9dc852
Compare
This was referenced May 29, 2026
makramkd
previously approved these changes
Jun 2, 2026
Comment on lines
+101
to
+103
| // - version 0 / absent — the legacy monolith output, a strict Cfg dump. | ||
| // - version 1 — the phased output, a raw component-output map; the required | ||
| // components are decoded out of it and the endpoint maps are derived. |
Collaborator
There was a problem hiding this comment.
question: can these be made constants defined somewhere in the devenv package?
Collaborator
Author
There was a problem hiding this comment.
I don't think it's useful to treat a version number like a feature flag. This is more of a backwards compatibility gate. If we end up needing a constant at some point we can add it later.
NewPhasedEnvironment now serializes the raw accumulated component hand-built PhasedOutput, and re-emits the schema version as a public key. LoadOutput[Cfg] switches on that version: absent/0 keeps the strict legacy Cfg decode, while version 1 leniently decodes the phased map and derives the aggregator/indexer endpoint maps — so all existing readers and NewLibFromCCVEnv are unchanged. Components now own their public output keys: protocol_contracts emits cldf and environment_topology, while shared TLS certs become a private "_"-key. jd stays public, with JDInfrastructure.OffchainClient marked toml:"-" so the live client is skipped but the JD output and node IDs persist for future job proposals. env.toml drops its version key so monolith output stays at 0.
winder
force-pushed
the
will/devenv-phased-no-cfg
branch
from
e9dc852 to
aa2bb20
Compare
|
Code coverage report:
|
makramkd
approved these changes
Jun 2, 2026
winder
commented
Jun 2, 2026
Comment on lines
+122
to
+127
| switch probe.Version { | ||
| case 0: | ||
| c, err = loadLegacyCfg(data) | ||
| case 1: | ||
| c, err = loadPhasedCfg(data) | ||
| default: |
Collaborator
Author
There was a problem hiding this comment.
I see why you might want a constant, but this (hopefully) goes away eventually. If we keep legacy around for an extended duration this probably changes to "version == 0 { legacy } else { phased }"
Contributor
There was a problem hiding this comment.
Pull request overview
This PR changes the phased devenv output contract to serialize the raw component output map (with runtime-only _-prefixed keys stripped) and updates LoadOutput[Cfg] to be version-aware so downstream readers can load either legacy (monolith) or phased output through the same API.
Changes:
NewPhasedEnvironmentnow returns the accumulated output map and persists a stripped, raw-map output file for phased mode.LoadOutput[Cfg]now dispatches decoding based on a top-levelversionmarker (legacy strict decode vs phased lenient decode + derived endpoint maps).- Component output keys are reclassified into public vs
_-private (e.g.,environment_topology,_shared_tls_certs), and JD infra serialization is made TOML-safe.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| changelog/2026-05-29_phased_raw_output_versioned_load.md | Documents the phased raw-output + versioned loader contract and breaking changes. |
| build/devenv/phased_loader_test.go | Adds unit tests around phased decoding and output stripping behavior. |
| build/devenv/jobs/jd.go | Makes JDInfrastructure serializable by tagging fields and excluding the live gRPC client. |
| build/devenv/environment_phased.go | Switches phased env creation to persist the raw output map (minus _ keys) and return it. |
| build/devenv/env.toml | Drops version from legacy env config so legacy output routes to version 0/absent behavior. |
| build/devenv/config.go | Implements version-aware LoadOutput plus phased/legacy decode helpers. |
| build/devenv/components/protocol_contracts/component.go | Publishes cldf and environment_topology as public outputs instead of _-prefixed keys. |
| build/devenv/components/indexer/component.go | Updates indexer to consume _shared_tls_certs runtime-only key. |
| build/devenv/components/executor/component.go | Updates executor to consume environment_topology public output key. |
| build/devenv/components/committeeccv/component.go | Updates committeeccv outputs to use _shared_tls_certs and consumes environment_topology. |
| build/devenv/cli/ccv.go | Adapts CLI env constructor handling to ignore return values consistently. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+121
to
+129
| var c *Cfg | ||
| switch probe.Version { | ||
| case 0: | ||
| c, err = loadLegacyCfg(data) | ||
| case 1: | ||
| c, err = loadPhasedCfg(data) | ||
| default: | ||
| return nil, fmt.Errorf("unsupported output version %d; supported version is 1", probe.Version) | ||
| } |
Comment on lines
+30
to
+35
| // Capture the schema version before the runtime consumes and deletes the | ||
| // "version" key from rawConfig, so it can be re-emitted to the output file. | ||
| var version int | ||
| if v, ok := rawConfig["version"].(int64); ok { | ||
| version = int(v) | ||
| } |
Comment on lines
+79
to
+103
| // TestStorePhasedOutputStripsPrivateKeys verifies "_"-prefixed runtime keys are | ||
| // dropped from the serialized output and version is re-emitted as public. | ||
| func TestStorePhasedOutputStripsPrivateKeys(t *testing.T) { | ||
| out := map[string]any{ | ||
| "version": 1, | ||
| "blockchains": []*blockchain.Input{{Type: "anvil"}}, | ||
| "_env": "runtime-only", | ||
| "_shared_tls_certs": "runtime-only", | ||
| "environment_topology": map[string]any{"x": 1}, | ||
| } | ||
|
|
||
| public := make(map[string]any, len(out)) | ||
| for k, v := range out { | ||
| if k[0] == '_' { | ||
| continue | ||
| } | ||
| public[k] = v | ||
| } | ||
|
|
||
| _, hasEnv := public["_env"] | ||
| _, hasTLS := public["_shared_tls_certs"] | ||
| assert.False(t, hasEnv, "_env should be stripped") | ||
| assert.False(t, hasTLS, "_shared_tls_certs should be stripped") | ||
| assert.Contains(t, public, "blockchains") | ||
| assert.Contains(t, public, "environment_topology") |
huangzhen1997
approved these changes
Jun 2, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
4 tasks
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
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.
PR Chain
There are multiple open pull requests that feed into each other:
Description
The phased devenv (--env-mode phased) previously reconstructed a transitional *Cfg from the component runtime's output map via hand-written sync blocks. This PR removes that bridge: NewPhasedEnvironment now serializes the raw accumulated output map directly, and LoadOutput[Cfg] switches on a version marker to decode either format. Adding a new serialized output no longer requires touching a central mapping — a component just publishes a public key.
What changed
Why
Compatibility / risk
Testing
Checklist
changelogdirectory)just lint fix- no new lint errorsjust generate- mocks and protobufs are up to date