Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions pkg/devcontainer/config/feature.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,56 @@
package config

import (
"bytes"
"encoding/json"
"fmt"
"sort"

"github.com/devsy-org/devsy/pkg/types"
)

// objectKeyOrder returns the keys of the named top-level object field in the
// order they appear in the JSON document. It returns nil when the field is
// absent or is not a JSON object.
func objectKeyOrder(data []byte, field string) ([]string, error) {
var raw map[string]json.RawMessage
if err := json.Unmarshal(data, &raw); err != nil {
return nil, err
}
value, ok := raw[field]
if !ok {
return nil, nil
}

var obj map[string]json.RawMessage
if err := json.Unmarshal(value, &obj); err != nil {
return nil, nil //nolint:nilerr // non-object values carry no key order
}
return decodeObjectKeys(value)
}

// decodeObjectKeys reads the keys of a JSON object in document order.
func decodeObjectKeys(value json.RawMessage) ([]string, error) {
dec := json.NewDecoder(bytes.NewReader(value))
if _, err := dec.Token(); err != nil { // consume opening '{'
return nil, err
}

var keys []string
for dec.More() {
keyTok, err := dec.Token()
if err != nil {
return nil, err
}
keys = append(keys, keyTok.(string))
var skip json.RawMessage
if err := dec.Decode(&skip); err != nil {
return nil, err
}
}
return keys, nil
}

type FeatureSet struct {
ConfigID string
Version string
Expand Down Expand Up @@ -78,6 +122,43 @@ type FeatureConfig struct {

// Origin is the path where the feature was loaded from
Origin string `json:"-"`

// dependsOnOrder preserves the declaration order of DependsOn keys as they
// appear in devcontainer-feature.json. The reference devcontainer CLI emits
// lockfile dependsOn arrays in this order (Object.keys), so we retain it to
// produce byte-identical lockfiles.
dependsOnOrder []string `json:"-"`
}

// UnmarshalJSON decodes a FeatureConfig while capturing the declaration order
// of the dependsOn object's keys, which a plain map cannot preserve.
func (c *FeatureConfig) UnmarshalJSON(data []byte) error {
type alias FeatureConfig
if err := json.Unmarshal(data, (*alias)(c)); err != nil {
return err
}
order, err := objectKeyOrder(data, "dependsOn")
if err != nil {
return err
}
c.dependsOnOrder = order
return nil
}

// DependsOnKeys returns the dependency feature identifiers in their original
// declaration order, matching the reference devcontainer CLI's lockfile output.
// It falls back to sorted keys when order was not captured (e.g. a
// programmatically constructed config).
func (c *FeatureConfig) DependsOnKeys() []string {
if len(c.dependsOnOrder) == len(c.DependsOn) {
return c.dependsOnOrder
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}
keys := make([]string, 0, len(c.DependsOn))
for k := range c.DependsOn {
keys = append(keys, k)
}
sort.Strings(keys)
return keys
}

type DependsOnField map[string]any
Expand Down
57 changes: 57 additions & 0 deletions pkg/devcontainer/config/feature_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package config

import (
"encoding/json"
"reflect"
"testing"
)

const (
depZeta = "ghcr.io/x/zeta:1"
depAlpha = "ghcr.io/x/alpha:1"
depMid = "ghcr.io/x/mid:1"
)

func TestFeatureConfig_DependsOnKeysPreservesDeclarationOrder(t *testing.T) {
// Keys are intentionally not alphabetical to prove declaration order is
// preserved rather than sorted, matching the reference devcontainer CLI.
data := []byte(`{
"id": "example",
"dependsOn": {
"ghcr.io/x/zeta:1": {},
"ghcr.io/x/alpha:1": {},
"ghcr.io/x/mid:1": {}
}
}`)

var cfg FeatureConfig
if err := json.Unmarshal(data, &cfg); err != nil {
t.Fatalf("unmarshal: %v", err)
}

want := []string{depZeta, depAlpha, depMid}
if got := cfg.DependsOnKeys(); !reflect.DeepEqual(got, want) {
t.Errorf("DependsOnKeys() = %v, want %v", got, want)
}
}

func TestFeatureConfig_DependsOnKeysFallsBackToSorted(t *testing.T) {
// A programmatically constructed config has no captured order; keys must be
// returned deterministically (sorted).
cfg := FeatureConfig{DependsOn: DependsOnField{
depZeta: map[string]any{},
depAlpha: map[string]any{},
}}

want := []string{depAlpha, depZeta}
if got := cfg.DependsOnKeys(); !reflect.DeepEqual(got, want) {
t.Errorf("DependsOnKeys() = %v, want %v", got, want)
}
}

func TestFeatureConfig_DependsOnKeysEmpty(t *testing.T) {
var cfg FeatureConfig
if got := cfg.DependsOnKeys(); len(got) != 0 {
t.Errorf("DependsOnKeys() = %v, want empty", got)
}
}
2 changes: 1 addition & 1 deletion pkg/devcontainer/feature/extend.go
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ func (p *featureProcessor) recordLockEntry(
Integrity: res.integrity,
}
if len(cfg.DependsOn) > 0 {
entry.DependsOn = map[string]any(cfg.DependsOn)
entry.DependsOn = cfg.DependsOnKeys()
}
p.lock.record(featureID, entry)
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/devcontainer/feature/lockfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ import (
// LockedFeature is a single pinned entry in a devcontainer-lock.json file. It
// mirrors the structure produced by the reference devcontainer CLI.
type LockedFeature struct {
Version string `json:"version,omitempty"`
Resolved string `json:"resolved,omitempty"`
Integrity string `json:"integrity,omitempty"`
DependsOn map[string]any `json:"dependsOn,omitempty"`
Version string `json:"version,omitempty"`
Resolved string `json:"resolved,omitempty"`
Integrity string `json:"integrity,omitempty"`
DependsOn []string `json:"dependsOn,omitempty"`
Comment on lines +19 to +22

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Locate lockfile readers and JSON decoding paths involving Lockfile types.
ast-grep outline pkg/devcontainer/feature/lockfile.go --items all
ast-grep run --lang go --pattern 'json.Unmarshal($DATA, $TARGET)' pkg
rg -nP --type=go -C3 '\b(?:Lockfile|LockedFeature)\b|json\.NewDecoder\(' pkg

Repository: devsy-org/devsy

Length of output: 35212


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== lockfile.go =="
sed -n '1,260p' pkg/devcontainer/feature/lockfile.go

echo "== call sites for newLockfileState / ReadLockfile / lockfileState =="
rg -n -C 4 'newLockfileState\(|ReadLockfile\(|\.loaded|checkFrozenPrecondition|lockfileMode|needsWrite|lockfileNeedsWrite' pkg/devcontainer pkg -g '*.go' | head -n 200

echo "== inspect devcontainer feature/extend relevant =="
sed -n '480,545p' pkg/devcontainer/feature/extend.go

Repository: devsy-org/devsy

Length of output: 21450


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Go JSON Unmarshal legacy object as []string behavior =="
go version 2>/dev/null || true
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT

cat > "$tmpdir/unmarshal_probe.go" <<'GO'
package main

import (
	"encoding/json"
	"fmt"
)

type LockedFeature struct {
	Version   string   `json:"version,omitempty"`
	Resolved  string   `json:"resolved,omitempty"`
	Integrity string   `json:"integrity,omitempty"`
	DependsOn []string `json:"dependsOn,omitempty"`
}

type Lockfile struct {
	Features map[string]LockedFeature `json:"features"`
}

func main() {
	legacy := []byte(`{
  "features": {
    "ghcr.io/a/f:1": {"resolved": "sha256:00", "dependsOn": "nope"}
  }
}`)
	var lf Lockfile
	err := json.Unmarshal(legacy, &lf)
	fmt.Printf("legacy-object-string err=%v lf=%#v\n", err, lf)

	legacyMap := []byte(`{
  "features": {
    "ghcr.io/a/f:1": {"resolved": "sha256:00", "dependsOn": {"dep": true}}
  }
}`)
	err = json.Unmarshal(legacyMap, &lf)
	fmt.Printf("legacy-map err=%v lf=%#v\n", err, lf)

	current := []byte(`{
  "features": {
    "ghcr.io/a/f:1": {"resolved": "sha256:00", "dependsOn": ["dep"]}
  }
}`)
	err = json.Unmarshal(current, &lf)
	fmt.Printf("current-array err=%v lf=%#v\n", err, lf)
}
GO

go run "$tmpdir/unmarshal_probe.go"

echo "== inspect config DependsOn type for feature config keys =="
sed -n '1,60p' pkg/devcontainer/config/feature.go
rg -n -C 3 'DependsOn[A-Za-z0-9_]*|DependsOn' pkg/devcontainer/config pkg/devcontainer/feature/lockfile.go pkg/devcontainer/feature/extend.go

Repository: devsy-org/devsy

Length of output: 11881


Normalize legacy dependsOn on lockfile read.

ReadLockfile unmarshals persisted files into LockedFeature, whose DependsOn is now []string; object-shaped dependsOn entries fail to parse during fetch resolution (via newLockfileState). Decode lockfile entries as flexible JSON first/normatively, then normalize to the array shape used for pinning and writing.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/devcontainer/feature/lockfile.go` around lines 19 - 22, Update
ReadLockfile to decode persisted feature entries through a flexible JSON
representation before constructing LockedFeature. Normalize legacy object-shaped
dependsOn values into the []string form, while preserving existing array-shaped
values, so newLockfileState receives the canonical representation used for
pinning and writing.

}

// Lockfile mirrors the devcontainer-lock.json structure: a map of feature
Expand Down
27 changes: 27 additions & 0 deletions pkg/devcontainer/feature/lockfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,33 @@ func TestWriteLockfile_CreatesSortedStable(t *testing.T) {
}
}

func TestWriteLockfile_DependsOnIsArray(t *testing.T) {
path := filepath.Join(t.TempDir(), "devcontainer-lock.json")
lf := &Lockfile{Features: map[string]LockedFeature{
lockTestFeatureA: {
Version: lockTestVersion,
Resolved: lockTestResolvedA,
Integrity: lockTestShaA,
DependsOn: []string{"ghcr.io/b/feature:1"},
},
}}

if err := WriteLockfile(path, lf, false); err != nil {
t.Fatalf("WriteLockfile: %v", err)
}

data, err := os.ReadFile(filepath.Clean(path))
if err != nil {
t.Fatal(err)
}
content := string(data)
// The reference devcontainer CLI serializes dependsOn as an array of
// feature identifiers, not an object.
if !strings.Contains(content, "\"dependsOn\": [") {
t.Errorf("expected dependsOn as JSON array, got:\n%s", content)
}
}

func TestWriteLockfile_SkipsUnchanged(t *testing.T) {
path := filepath.Join(t.TempDir(), "devcontainer-lock.json")
lf := &Lockfile{Features: map[string]LockedFeature{
Expand Down
Loading