Skip to content

fix(runs): preserve oneof fields when decoding revision spec #621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
trtyr wants to merge 1 commit into
base: main
Choose a base branch
from
+28 −2
Open

fix(runs): preserve oneof fields when decoding revision spec #621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions pkg/runs/preparation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package runs

import (
"context"
"encoding/json"
"fmt"
"os"
"path/filepath"
Expand All @@ -15,6 +14,7 @@ import (
"agent-compose/pkg/projects"
"agent-compose/pkg/storage/sandboxstore"
agentcomposev2 "agent-compose/proto/agentcompose/v2"
"google.golang.org/protobuf/encoding/protojson"
)

type PreparationStore interface {
Expand Down Expand Up @@ -151,7 +151,8 @@ func DecodeRevisionSpec(raw string) (*agentcomposev2.ProjectSpec, error) {
if err != nil {
return nil, fmt.Errorf("decode project revision spec: %w", err)
}
if err := json.Unmarshal(normalizedData, &spec); err != nil {
opts := protojson.UnmarshalOptions{DiscardUnknown: true}

@monkeyscan monkeyscan Bot Aug 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

protojson 解码切换缺少对既有存储 spec 兼容性与完整往返的回归覆盖

将 encoding/json.Unmarshal 替换为 protojson.Unmarshal 后，所有已存储的 project revision spec（revision.SpecJSON）的解码语义都发生了变化。encoding/json 按 protobuf 生成结构体的字段标签（camelCase + Go 字段结构）解析，而 protojson 遵循 proto3 JSON 规范：well-known 类型必须使用其规范表示（如 google.protobuf.Timestamp 为 RFC3339 字符串而非生成的 Go 结构体 {"seconds":...,"nanos":...}、Any 使用 @type），并且对同一对象的重复键直接报错而非 last-wins。若存在任何以非 protojson 规范序列化的历史 spec（例如旧写入路径使用 encoding/json.Marshal 产生的结构字段表示），切换后 DecodeRevisionSpec 将解码失败，进而导致 PrepareProjectRun 与 GetProject 中断；而 DiscardUnknown 又会使未知/无法识别的字段被静默丢弃。新增测试仅覆盖 driver 的 docker oneof 这一条最小路径（不含 workspaces/closed-sets 等经过 normalizeRevisionClosedSetsJSON 与 restoreCanonicalRevisionWorkspaces 处理的字段），既未验证 GetProject→PatchProject 的完整往返（解码后重新 protojson.Marshal 再被 PatchProject 接受），也未覆盖与既有存储数据格式的兼容性，回归风险面未被充分覆盖。

Problem code:

Changed code at pkg/runs/preparation.go:154-155

Recommendation:
补充覆盖完整往返与兼容性的测试：1) 构造一个包含 driver oneof、workspace、closed-sets 等字段的 protojson 序列化 spec，经 DecodeRevisionSpec 解码后重新 protojson.Marshal 再解码，断言 oneof 与其余字段在往返中保持（等价于 GetProject→PatchProject 场景）；2) 覆盖 well-known 类型（若 ProjectSpec 含 Timestamp/Any 等）的 protojson 规范表示解码，以及非规范结构字段表示的报错/丢弃行为符合预期，防止解码器切换对既有存储数据造成静默数据丢失或运行中断。

if err := opts.Unmarshal(normalizedData, &spec); err != nil {
return nil, fmt.Errorf("decode project revision spec: %w", err)
}
if err := restoreCanonicalRevisionWorkspaces(data, &spec); err != nil {
Expand Down
25 changes: 25 additions & 0 deletions pkg/runs/preparation_driver_oneof_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package runs

import (
"testing"

agentcomposev2 "agent-compose/proto/agentcompose/v2"
)

// DecodeRevisionSpec must preserve oneof fields (e.g. driver runtime config).
// Regression test for the GetProject → PatchProject round-trip where a driver
// decoded via encoding/json lost its config oneof, so PatchProject rejected the
// returned spec with "driver requires exactly one runtime config".
func TestDecodeRevisionSpecPreservesDriverRuntimeConfig(t *testing.T) {
spec, err := DecodeRevisionSpec(`{"name":"demo","agents":[{"name":"worker","driver":{"name":"docker","docker":{}}}]}`)
if err != nil {
t.Fatalf("DecodeRevisionSpec returned error: %v", err)
}
driver := spec.GetAgents()[0].GetDriver()
if driver.GetName() != "docker" {
t.Fatalf("driver name = %q, want docker", driver.GetName())
}
if _, ok := driver.GetConfig().(*agentcomposev2.DriverSpec_Docker); !ok {
t.Fatalf("driver config = %T, want *agentcomposev2.DriverSpec_Docker", driver.GetConfig())
}
}
Loading