Skip to content

Commit 0941a7f

Browse files
committed
clone complete OMP agent state for OAuth
1 parent 3552b2d commit 0941a7f

8 files changed

Lines changed: 187 additions & 88 deletions

File tree

README.md

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -255,13 +255,12 @@ enable only the integrations that project needs.
255255
OMP is opt-in. When enabled, its selected binary, native modules, model catalog,
256256
and non-secret configuration are cloned into workspace-owned writable snapshots;
257257
container writes cannot modify the host copies. Set `requireCow: true` to require
258-
reflink support instead of allowing a private copy-once fallback. Set
259-
`agents.omp.import.oauthDB: true` only when the workspace may receive credentials.
260-
Cohotfs directly byte-copies `agent.db` and any existing `-wal`, `-shm`, or
261-
`-journal` sidecars into a private, writable snapshot; these files do not use
262-
reflinks even when `requireCow: true`. Host and workspace credential updates do
263-
not sync. Stop OMP or otherwise quiesce its database before workspace creation
264-
when a point-in-time-consistent copy is required.
258+
reflink support instead of allowing a private copy-once fallback. Setting
259+
`agents.omp.import.oauthDB: true` clones the complete OMP agent directory,
260+
including its credential, session, history, configuration, and cache state, into
261+
a private writable COW snapshot mounted at `PI_CODING_AGENT_DIR`. Host and
262+
workspace changes do not sync. Stop OMP before workspace creation when a
263+
point-in-time-consistent SQLite snapshot is required.
265264

266265
All host-side state stays under:
267266

integration/docker/end_to_end_test.go

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,18 +139,32 @@ func TestWorkspaceEndToEnd(t *testing.T) {
139139
}}
140140
record, _, source := harness.createWorkspaceWithOptions(t, "it-reserved-mask", "manual", true, config.ResourceSpec{}, options)
141141
record = harness.start(t, record)
142-
after, err := os.Lstat(filepath.Join(source, ".omp"))
142+
assertReservedMask := func() {
143+
after, err := os.Lstat(filepath.Join(source, ".omp"))
144+
if err != nil {
145+
t.Fatal(err)
146+
}
147+
if !os.SameFile(before, after) {
148+
t.Fatal("Docker replaced the host reserved directory")
149+
}
150+
assertFile(t, filepath.Join(source, ".omp", "host-sentinel"), "host-only\n")
151+
stdout, stderr := harness.ssh(t, record, nil, `test -d /workspace/.omp && test ! -e /workspace/.omp/host-sentinel && printf 'reserved-mask-ok\n'`)
152+
if string(stdout) != "reserved-mask-ok\n" || len(stderr) != 0 {
153+
t.Fatalf("reserved mask stdout=%q stderr=%q", stdout, stderr)
154+
}
155+
}
156+
assertReservedMask()
157+
record = harness.stop(t, record)
158+
afterStop, err := os.Lstat(filepath.Join(source, ".omp"))
143159
if err != nil {
144160
t.Fatal(err)
145161
}
146-
if !os.SameFile(before, after) {
147-
t.Fatal("Docker replaced the host reserved directory")
162+
if !os.SameFile(before, afterStop) {
163+
t.Fatal("stopping replaced the host reserved directory")
148164
}
149165
assertFile(t, filepath.Join(source, ".omp", "host-sentinel"), "host-only\n")
150-
stdout, stderr := harness.ssh(t, record, nil, `test -d /workspace/.omp && test ! -e /workspace/.omp/host-sentinel && printf 'reserved-mask-ok\n'`)
151-
if string(stdout) != "reserved-mask-ok\n" || len(stderr) != 0 {
152-
t.Fatalf("reserved mask stdout=%q stderr=%q", stdout, stderr)
153-
}
166+
record = harness.start(t, record)
167+
assertReservedMask()
154168
harness.remove(t, record)
155169
})
156170
t.Run("go-toolchain-with-omp-oauth", func(t *testing.T) {

internal/ompimport/plan_linux.go

Lines changed: 39 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
//go:build linux
22

3-
// Package ompimport compiles explicit host OMP file imports. Selected host
4-
// files, including the OAuth database and its existing sidecars, are copied
5-
// into workspace-owned writable snapshots.
3+
// Package ompimport compiles explicit host OMP imports into workspace-owned
4+
// writable snapshots.
65
package ompimport
76

87
import (
@@ -113,16 +112,22 @@ func Compile(root *hostroot.Root, workspaceID string, spec config.OMPAgentSpec,
113112
}
114113
plan.Mounts = append(plan.Mounts, runtime.Mount{Source: snapshot, Target: containerNative, Type: "bind", Propagation: "rprivate"})
115114
}
116-
if spec.Import.OAuthDB && sources.Agent == "" {
117-
return Plan{}, fmt.Errorf("OMP OAuth database source is unavailable")
118-
}
119-
if (spec.Import.Models || spec.Import.Config || spec.Import.OAuthDB) && sources.Agent != "" {
115+
if spec.Import.OAuthDB {
116+
if sources.Agent == "" {
117+
return Plan{}, fmt.Errorf("OMP agent directory source is unavailable")
118+
}
119+
snapshot, err := prepareDirectorySnapshot(root, workspaceID, "agent", sources.Agent, spec.Import.RequireCOW)
120+
if err != nil {
121+
return Plan{}, err
122+
}
123+
plan.Mounts = append(plan.Mounts, runtime.Mount{Source: snapshot, Target: containerAgent, Type: "bind", Propagation: "rprivate"})
124+
} else if (spec.Import.Models || spec.Import.Config) && sources.Agent != "" {
120125
selected, err := selectedAgentFiles(spec.Import, sources.Agent)
121126
if err != nil {
122127
return Plan{}, err
123128
}
124-
if len(selected) != 0 || spec.Import.OAuthDB {
125-
snapshot, err := prepareAgentSnapshot(root, workspaceID, sources.Agent, selected, spec.Import.RequireCOW)
129+
if len(selected) != 0 {
130+
snapshot, err := prepareSelectedSnapshot(root, workspaceID, "agent", sources.Agent, selected, spec.Import.RequireCOW)
126131
if err != nil {
127132
return Plan{}, err
128133
}
@@ -141,19 +146,6 @@ func selectedAgentFiles(spec config.OMPImportSpec, root string) ([]string, error
141146
candidates = append(candidates, "models.yml", "models.yaml")
142147
}
143148
selected := make([]string, 0, len(candidates))
144-
if spec.OAuthDB {
145-
for _, name := range []string{"agent.db", "agent.db-wal", "agent.db-shm", "agent.db-journal"} {
146-
path := filepath.Join(root, name)
147-
err := requireRegular(path, false)
148-
if name != "agent.db" && errors.Is(err, os.ErrNotExist) {
149-
continue
150-
}
151-
if err != nil {
152-
return nil, fmt.Errorf("validate OMP OAuth database file %s: %w", name, err)
153-
}
154-
selected = append(selected, name)
155-
}
156-
}
157149
for _, name := range candidates {
158150
path := filepath.Join(root, name)
159151
err := requireRegular(path, false)
@@ -196,38 +188,6 @@ func prepareSelectedSnapshot(root *hostroot.Root, workspaceID, name, source stri
196188
})
197189
}
198190

199-
func prepareAgentSnapshot(root *hostroot.Root, workspaceID, source string, selected []string, requireCOW bool) (string, error) {
200-
fingerprint, err := selectedFingerprint(source, selected)
201-
if err != nil {
202-
return "", err
203-
}
204-
return prepareSnapshot(root, workspaceID, "agent", fingerprint, func(staging string) error {
205-
for _, relative := range selected {
206-
sourcePath := filepath.Join(source, relative)
207-
destinationPath := filepath.Join(staging, relative)
208-
var err error
209-
if isOAuthDatabaseFile(relative) {
210-
err = copyRegularFile(sourcePath, destinationPath, false)
211-
} else {
212-
err = cloneRegularFile(sourcePath, destinationPath, false, requireCOW)
213-
}
214-
if err != nil {
215-
return err
216-
}
217-
}
218-
return nil
219-
})
220-
}
221-
222-
func isOAuthDatabaseFile(name string) bool {
223-
switch name {
224-
case "agent.db", "agent.db-wal", "agent.db-shm", "agent.db-journal":
225-
return true
226-
default:
227-
return false
228-
}
229-
}
230-
231191
func prepareDirectorySnapshot(root *hostroot.Root, workspaceID, name, source string, requireCOW bool) (string, error) {
232192
fingerprint, err := directoryFingerprint(source)
233193
if err != nil {
@@ -318,10 +278,6 @@ func cloneRegularFile(source, destination string, executable, requireCOW bool) e
318278
return writeRegularFile(source, destination, executable, true, requireCOW)
319279
}
320280

321-
func copyRegularFile(source, destination string, executable bool) error {
322-
return writeRegularFile(source, destination, executable, false, false)
323-
}
324-
325281
func writeRegularFile(source, destination string, executable, tryReflink, requireCOW bool) error {
326282
sourceFD, err := unix.Open(source, unix.O_RDONLY|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0)
327283
if err != nil {
@@ -399,13 +355,16 @@ func selectedFingerprint(root string, names []string) (string, error) {
399355
sort.Strings(names)
400356
for _, name := range names {
401357
path := filepath.Join(root, name)
402-
entry, err := os.Stat(path)
358+
entry, err := os.Lstat(path)
403359
if err != nil {
404360
return "", err
405361
}
406362
if _, err := fmt.Fprintf(hash, "%s\x00%d\x00%d\x00%d\x00", name, entry.Mode(), entry.Size(), entry.ModTime().UnixNano()); err != nil {
407363
return "", err
408364
}
365+
if err := fingerprintRegularFile(hash, path); err != nil {
366+
return "", err
367+
}
409368
}
410369
return hex.EncodeToString(hash.Sum(nil)[:16]), nil
411370
}
@@ -426,9 +385,30 @@ func fingerprintEntry(hash interface{ Write([]byte) (int, error) }, path, relati
426385
_, err = fmt.Fprintf(hash, "%s\x00", target)
427386
return err
428387
}
388+
if info.Mode().IsRegular() {
389+
return fingerprintRegularFile(hash, path)
390+
}
429391
return nil
430392
}
431393

394+
func fingerprintRegularFile(hash io.Writer, path string) error {
395+
fd, err := unix.Open(path, unix.O_RDONLY|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0)
396+
if err != nil {
397+
return err
398+
}
399+
file := os.NewFile(uintptr(fd), path)
400+
defer file.Close()
401+
var stat unix.Stat_t
402+
if err := unix.Fstat(fd, &stat); err != nil {
403+
return err
404+
}
405+
if stat.Mode&unix.S_IFMT != unix.S_IFREG {
406+
return fmt.Errorf("unsafe OMP file %s", path)
407+
}
408+
_, err = io.Copy(hash, file)
409+
return err
410+
}
411+
432412
func requireRegular(path string, executable bool) error {
433413
info, err := os.Lstat(path)
434414
if err != nil {

internal/ompimport/plan_linux_test.go

Lines changed: 68 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,21 @@ func TestCompileBuildsWritableReflinkMountsForSelectedOMPState(t *testing.T) {
6868
}
6969
}
7070

71-
func TestCompileCopiesOAuthDatabaseFilesDespiteRequireCOW(t *testing.T) {
71+
func TestCompileClonesCompleteAgentDirectoryWhenOAuthEnabled(t *testing.T) {
7272
root := openTestRoot(t)
7373
defer root.Close()
7474
sources := ompFixture(t)
75+
nested := filepath.Join(sources.Agent, "sessions", "current")
76+
if err := os.MkdirAll(nested, 0o700); err != nil {
77+
t.Fatal(err)
78+
}
79+
if err := os.WriteFile(filepath.Join(nested, "session.json"), []byte("session fixture"), 0o600); err != nil {
80+
t.Fatal(err)
81+
}
7582
spec := config.OMPAgentSpec{
7683
Enabled: true,
7784
Import: config.OMPImportSpec{
78-
Enabled: true, OAuthDB: true, RequireCOW: true,
85+
Enabled: true, OAuthDB: true, RequireCOW: false,
7986
},
8087
}
8188

@@ -84,7 +91,10 @@ func TestCompileCopiesOAuthDatabaseFilesDespiteRequireCOW(t *testing.T) {
8491
t.Fatal(err)
8592
}
8693
agent := requiredMount(t, plan, containerAgent).Source
87-
for _, name := range []string{"agent.db", "agent.db-wal", "agent.db-shm", "agent.db-journal"} {
94+
for _, name := range []string{
95+
"agent.db", "agent.db-wal", "agent.db-shm", "agent.db-journal",
96+
"config.yml", "models.yml", "history.db", filepath.Join("sessions", "current", "session.json"),
97+
} {
8898
sourcePath := filepath.Join(sources.Agent, name)
8999
snapshotPath := filepath.Join(agent, name)
90100
source, err := os.ReadFile(sourcePath)
@@ -96,7 +106,7 @@ func TestCompileCopiesOAuthDatabaseFilesDespiteRequireCOW(t *testing.T) {
96106
t.Fatal(err)
97107
}
98108
if string(snapshot) != string(source) {
99-
t.Fatalf("OAuth database file %s changed during copy", name)
109+
t.Fatalf("OMP agent file %s changed during clone", name)
100110
}
101111
sourceInfo, err := os.Stat(sourcePath)
102112
if err != nil {
@@ -107,7 +117,7 @@ func TestCompileCopiesOAuthDatabaseFilesDespiteRequireCOW(t *testing.T) {
107117
t.Fatal(err)
108118
}
109119
if os.SameFile(sourceInfo, snapshotInfo) {
110-
t.Fatalf("OAuth database file %s reuses the host inode", name)
120+
t.Fatalf("OMP agent file %s reuses the host inode", name)
111121
}
112122
}
113123
if err := os.WriteFile(filepath.Join(agent, "agent.db"), []byte("workspace-update"), 0o600); err != nil {
@@ -208,6 +218,59 @@ func TestCompileChangesSnapshotWhenHostStateChanges(t *testing.T) {
208218
}
209219
}
210220

221+
func TestCompileInvalidatesFullAgentSnapshotWhenContentChangesWithoutMetadata(t *testing.T) {
222+
root := openTestRoot(t)
223+
defer root.Close()
224+
sources := ompFixture(t)
225+
spec := config.OMPAgentSpec{
226+
Enabled: true,
227+
Import: config.OMPImportSpec{Enabled: true, OAuthDB: true},
228+
}
229+
first, err := Compile(root, testWorkspaceID, spec, sources)
230+
if err != nil {
231+
t.Fatal(err)
232+
}
233+
historyPath := filepath.Join(sources.Agent, "history.db")
234+
info, err := os.Stat(historyPath)
235+
if err != nil {
236+
t.Fatal(err)
237+
}
238+
changed, err := os.ReadFile(historyPath)
239+
if err != nil {
240+
t.Fatal(err)
241+
}
242+
changed[0] ^= 0xff
243+
if err := os.WriteFile(historyPath, changed, info.Mode().Perm()); err != nil {
244+
t.Fatal(err)
245+
}
246+
if err := os.Chtimes(historyPath, info.ModTime(), info.ModTime()); err != nil {
247+
t.Fatal(err)
248+
}
249+
changedInfo, err := os.Stat(historyPath)
250+
if err != nil {
251+
t.Fatal(err)
252+
}
253+
if changedInfo.Size() != info.Size() || !changedInfo.ModTime().Equal(info.ModTime()) {
254+
t.Fatalf("test mutation changed metadata: before=%#v after=%#v", info, changedInfo)
255+
}
256+
second, err := Compile(root, testWorkspaceID, spec, sources)
257+
if err != nil {
258+
t.Fatal(err)
259+
}
260+
firstAgent := requiredMount(t, first, containerAgent).Source
261+
secondAgent := requiredMount(t, second, containerAgent).Source
262+
if firstAgent == secondAgent {
263+
t.Fatal("content change with stable size and mtime reused the previous full agent snapshot")
264+
}
265+
snapshot, err := os.ReadFile(filepath.Join(secondAgent, "history.db"))
266+
if err != nil {
267+
t.Fatal(err)
268+
}
269+
if string(snapshot) != string(changed) {
270+
t.Fatal("new full agent snapshot does not contain changed content")
271+
}
272+
}
273+
211274
func TestDiscoverUsesExplicitAgentDirectoryAndPrivateNatives(t *testing.T) {
212275
home := t.TempDir()
213276
bin := filepath.Join(t.TempDir(), "bin")

internal/workspace/docker.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,15 @@ func (s *DockerService) startLocked(ctx context.Context, record state.Workspace)
705705
}
706706
releasePins, err := pinReservedWorkspaceMasks(plan)
707707
if err != nil {
708-
return record, err
708+
if !status.Running {
709+
return record, err
710+
}
711+
cleanupCtx := context.WithoutCancel(ctx)
712+
stopErr := s.backend.Stop(cleanupCtx, record.RuntimeRef, 10*time.Second)
713+
leaseErr := s.releaseActiveIntegrationLeases(cleanupCtx, &record)
714+
transitionErr := record.Transition(state.StatusError, s.now())
715+
saveErr := s.store.SaveWorkspace(record)
716+
return record, errors.Join(err, stopErr, leaseErr, transitionErr, saveErr)
709717
}
710718
defer func() { releasePins() }()
711719
if err := s.acquireIntegrationLeases(ctx, &record, plan); err != nil {

0 commit comments

Comments
 (0)