Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
79 changes: 79 additions & 0 deletions pkg/devcontainer/config/feature.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,52 @@
package config

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

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

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)
}

func decodeObjectKeys(value json.RawMessage) ([]string, error) {
dec := json.NewDecoder(bytes.NewReader(value))
if _, err := dec.Token(); err != nil {
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 +118,45 @@ type FeatureConfig struct {

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

dependsOnOrder []string `json:"-"`
}

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
}

func (c *FeatureConfig) DependsOnKeys() []string {
if c.capturedOrderMatchesDeps() {
return c.dependsOnOrder
}
keys := make([]string, 0, len(c.DependsOn))
for k := range c.DependsOn {
keys = append(keys, k)
}
sort.Strings(keys)
return keys
}

func (c *FeatureConfig) capturedOrderMatchesDeps() bool {
if len(c.dependsOnOrder) != len(c.DependsOn) {
return false
}
for _, k := range c.dependsOnOrder {
if _, ok := c.DependsOn[k]; !ok {
return false
}
}
return true
}

type DependsOnField map[string]any
Expand Down
71 changes: 71 additions & 0 deletions pkg/devcontainer/config/feature_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
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) {
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) {
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_DependsOnKeysFallsBackWhenKeyMutated(t *testing.T) {
data := []byte(
`{"id": "example", "dependsOn": {"ghcr.io/x/zeta:1": {}, "ghcr.io/x/alpha:1": {}}}`,
)

var cfg FeatureConfig
if err := json.Unmarshal(data, &cfg); err != nil {
t.Fatalf("unmarshal: %v", err)
}
delete(cfg.DependsOn, depAlpha)
cfg.DependsOn[depMid] = map[string]any{}

want := []string{depMid, 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
25 changes: 25 additions & 0 deletions pkg/devcontainer/feature/lockfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,31 @@ 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)
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