Skip to content

Commit 166f3a5

Browse files
fix: address the agent's source tree when the gateway does source work
The gateway assumed its work directory was the tree the agent calls its source. A service declaration moves that tree with source-dir, and now that the gateway resolves symbol patches and import inventories itself, the two sides addressed different files under the same relative path. Standardising on the service directory, as the previous commit did, only moved the seam. It made symbol patches self-consistent but left prepared apply-edit previewing through the agent while writing beside it, and the preview identity check could not catch that: it compares the preview against the gateway's own read, so once both sides read the same tree it passes by construction. With a byte-identical file at each root it passed while the edit landed on the wrong one, reporting success and consuming a single-use permit. The engine decides where an agent roots, so it now answers that question. AgentSourceRoot mirrors both halves of the decision: source-dir applies only when a real service is bound, since an ephemeral source workspace resolves its attachment back to the service directory, and only from the declaration beside the agent's own work directory. The gateway resolves it once and routes project reads and writes, both symbol-patch paths, the prepared write and the import inventory through it, while VCS, terminal and code-unit paths stay on the service directory where they belong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent a678e0b commit 166f3a5

4 files changed

Lines changed: 226 additions & 18 deletions

File tree

‎pkg/engine/target.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,3 +238,55 @@ func parentResourceDir(start, configurationName string) (string, error) {
238238
current = parent
239239
}
240240
}
241+
242+
// AgentSourceRoot reports the tree an agent bound to root will treat as its
243+
// source. Callers that perform source work themselves — the Gateway resolves
244+
// symbol patches and import inventories in-process — must address the same
245+
// tree the agent does, or they preview one file and write another of the same
246+
// name.
247+
//
248+
// It mirrors the two decisions that place an agent's root. First, a service is
249+
// only bound from its real declaration when a workspace and a service
250+
// configuration exist above root; otherwise resolveServiceDescriptor prepares
251+
// an ephemeral source workspace whose attachment resolves back to root, and any
252+
// source-dir beside root is not the one the agent reads. Second, the agent
253+
// reads the declaration beside its own work directory only, so a declaration
254+
// further up the tree does not move it (see core agents/services.
255+
// ResolveSourceLocation).
256+
func AgentSourceRoot(ctx context.Context, root string) string {
257+
absolute, err := filepath.Abs(root)
258+
if err != nil {
259+
return root
260+
}
261+
absolute = filepath.Clean(absolute)
262+
if _, workspaceErr := parentResourceDir(absolute, resources.WorkspaceConfigurationName); workspaceErr != nil {
263+
return absolute
264+
}
265+
if _, serviceErr := parentResourceDir(absolute, resources.ServiceConfigurationName); serviceErr != nil {
266+
return absolute
267+
}
268+
if info, statErr := os.Stat(filepath.Join(absolute, resources.ServiceConfigurationName)); statErr != nil || info.IsDir() {
269+
return absolute
270+
}
271+
declaration, err := resources.LoadServiceFromDir(ctx, absolute)
272+
if err != nil || declaration == nil {
273+
return absolute
274+
}
275+
configured, _ := declaration.Spec["source-dir"].(string)
276+
configured = strings.TrimSpace(configured)
277+
if configured == "" {
278+
return absolute
279+
}
280+
configured = filepath.FromSlash(configured)
281+
// Core refuses a source-dir that leaves the service directory, so a
282+
// rejected value leaves the agent at its work directory rather than
283+
// somewhere outside it.
284+
if !filepath.IsLocal(configured) {
285+
return absolute
286+
}
287+
location := filepath.Clean(filepath.Join(absolute, configured))
288+
if physical, evalErr := filepath.EvalSymlinks(location); evalErr == nil {
289+
return physical
290+
}
291+
return location
292+
}

‎pkg/gateway/engine_source_semantics_test.go‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package gateway
22

33
import (
4+
"crypto/ed25519"
5+
"crypto/rand"
46
"crypto/sha256"
57
"encoding/hex"
68
"os"
@@ -10,11 +12,13 @@ import (
1012
"testing"
1113
"time"
1214

15+
"github.com/codefly-dev/cli/pkg/engine"
1316
"github.com/codefly-dev/cli/pkg/internal/protocoltest"
1417
basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0"
1518
executionv1 "github.com/codefly-dev/core/generated/go/codefly/execution/v1"
1619
codev0 "github.com/codefly-dev/core/generated/go/codefly/services/code/v0"
1720
gatewayv1 "github.com/codefly-dev/core/generated/go/mind/gateway/v1"
21+
"github.com/google/uuid"
1822
)
1923

2024
// newSourceSemanticsGateway binds a gateway to one peer-backed service so the
@@ -351,3 +355,125 @@ func TestGatewaySemanticIndexDegradesOnMalformedSource(t *testing.T) {
351355
t.Fatalf("degraded index dropped the parseable file's symbols: %+v", index.GetSymbols())
352356
}
353357
}
358+
359+
// newSourceDirGateway builds a gateway whose service declares source-dir, with
360+
// a byte-identical shadow of the target file at the service root. The shadow is
361+
// what makes the bug silent: hash-and-size checks cannot tell the two apart, so
362+
// only the path that is actually written distinguishes right from wrong.
363+
func newSourceDirGateway(t *testing.T, relative, content string) (*Server, string) {
364+
t.Helper()
365+
root := t.TempDir()
366+
writeCodeUnitFixture(t, root, "workspace.codefly.yaml", "name: shop\nlayout: flat\n")
367+
writeCodeUnitFixture(t, root, "service.codefly.yaml", "name: app\nversion: 0.0.1\nspec:\n source-dir: src\n")
368+
writeCodeUnitFixture(t, root, "mind.yaml", "service: app\nplugin: generic-go\n")
369+
writeCodeUnitFixture(t, root, filepath.Join("src", relative), content)
370+
writeCodeUnitFixture(t, root, relative, content)
371+
372+
server, err := NewServer(Config{WorkDir: root})
373+
if err != nil {
374+
t.Fatal(err)
375+
}
376+
t.Cleanup(func() { _ = server.Close() })
377+
return server, root
378+
}
379+
380+
// The gateway resolves symbol patches itself, so it must address the tree the
381+
// agent calls its source. With source-dir set, patching the service-root copy
382+
// leaves the compiled source untouched while reporting success.
383+
func TestSymbolPatchTargetsTheAgentSourceRootNotTheServiceRoot(t *testing.T) {
384+
declaration := "func Value() int { return 1 }"
385+
body := "package service\n\n" + declaration + "\n"
386+
server, root := newSourceDirGateway(t, filepath.Join("pkg", "service.go"), body)
387+
388+
response, err := server.ApplySymbolPatch(t.Context(), &gatewayv1.ApplySymbolPatchRequest{
389+
Service: "app", File: "pkg/service.go", QualifiedName: "service.Value",
390+
ExpectedDeclarationSha256: contentSHA256([]byte(declaration)),
391+
NewSource: "func Value() int { return 2 }",
392+
})
393+
if err != nil || !response.GetSuccess() {
394+
t.Fatalf("apply symbol patch: response=%+v err=%v", response, err)
395+
}
396+
397+
patched, err := os.ReadFile(filepath.Join(root, "src", "pkg", "service.go"))
398+
if err != nil || !strings.Contains(string(patched), "return 2") {
399+
t.Fatalf("agent source tree was not patched: content=%q err=%v", patched, err)
400+
}
401+
shadow, err := os.ReadFile(filepath.Join(root, "pkg", "service.go"))
402+
if err != nil {
403+
t.Fatal(err)
404+
}
405+
if string(shadow) != body {
406+
t.Fatalf("the service-root shadow was patched instead of the agent's source: %q", shadow)
407+
}
408+
}
409+
410+
// The same root has to govern prepared mutations end to end: preview, the
411+
// authoritative re-read, and the write. A prepared symbol patch must move the
412+
// agent's source and leave the identical shadow alone.
413+
func TestPreparedMutationTargetsTheAgentSourceRoot(t *testing.T) {
414+
declaration := "func Value() int { return 1 }"
415+
body := "package service\n\n" + declaration + "\n"
416+
server, root := newSourceDirGateway(t, filepath.Join("pkg", "service.go"), body)
417+
418+
publicKey, privateKey, err := ed25519.GenerateKey(rand.Reader)
419+
if err != nil {
420+
t.Fatal(err)
421+
}
422+
if _, err := server.ConfigureMutationAuthority(t.Context(), &gatewayv1.ConfigureMutationAuthorityRequest{
423+
AuthorityId: "coordinator-key-v1", WorkspaceId: "workspace-" + uuid.NewString(), Ed25519PublicKey: publicKey,
424+
}); err != nil {
425+
t.Fatal(err)
426+
}
427+
428+
preparedResponse, err := server.PrepareMutation(t.Context(), &gatewayv1.PrepareMutationRequest{
429+
Service: "app", WorkspaceVersion: "workspace-source-dir-v1",
430+
Mutation: &gatewayv1.PrepareMutationRequest_SymbolPatch{SymbolPatch: &gatewayv1.PrepareSymbolPatchMutation{
431+
File: "pkg/service.go", SymbolId: "symbol-service-value", QualifiedName: "service.Value",
432+
ExpectedDeclarationSha256: contentSHA256([]byte(declaration)),
433+
NewSource: "func Value() int { return 2 }", FixMode: basev0.FixMode_FIX_MODE_NONE,
434+
}},
435+
})
436+
if err != nil || !preparedResponse.GetSuccess() {
437+
t.Fatalf("prepare symbol mutation: response=%+v err=%v", preparedResponse, err)
438+
}
439+
prepared := preparedResponse.GetPrepared()
440+
permit := signPreparedMutationPermit(t, privateKey, prepared, time.Now().UTC().Add(-time.Second), time.Minute)
441+
applied, err := server.ApplyPreparedMutation(t.Context(), &gatewayv1.ApplyPreparedMutationRequest{
442+
Service: "app", PreparationId: prepared.GetPreparationId(),
443+
MutationDigest: prepared.GetMutationDigest(), MutationPermit: permit,
444+
})
445+
if err != nil || !applied.GetSuccess() {
446+
t.Fatalf("apply prepared mutation: response=%+v err=%v", applied, err)
447+
}
448+
449+
patched, err := os.ReadFile(filepath.Join(root, "src", "pkg", "service.go"))
450+
if err != nil || !strings.Contains(string(patched), "return 2") {
451+
t.Fatalf("agent source tree was not written: content=%q err=%v", patched, err)
452+
}
453+
shadow, err := os.ReadFile(filepath.Join(root, "pkg", "service.go"))
454+
if err != nil {
455+
t.Fatal(err)
456+
}
457+
if string(shadow) != body {
458+
t.Fatalf("prepared bytes landed on the service-root shadow: %q", shadow)
459+
}
460+
}
461+
462+
// An ephemeral source workspace is bound when no workspace configuration
463+
// encloses the root, and its attachment resolves back to that root — so a
464+
// source-dir beside it is not the declaration the agent reads, and honouring it
465+
// would move the gateway off the agent's tree instead of onto it.
466+
func TestAgentSourceRootIgnoresSourceDirWithoutAnEnclosingWorkspace(t *testing.T) {
467+
root := t.TempDir()
468+
writeCodeUnitFixture(t, root, "service.codefly.yaml", "name: app\nversion: 0.0.1\nspec:\n source-dir: src\n")
469+
writeCodeUnitFixture(t, root, filepath.Join("src", "keep.go"), "package keep\n")
470+
471+
resolved := engine.AgentSourceRoot(t.Context(), root)
472+
physical, err := filepath.EvalSymlinks(root)
473+
if err != nil {
474+
t.Fatal(err)
475+
}
476+
if resolved != physical && resolved != root {
477+
t.Fatalf("source root = %q, want the service root %q when no workspace encloses it", resolved, root)
478+
}
479+
}

‎pkg/gateway/prepared_mutation.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -305,11 +305,10 @@ func (s *Server) ApplyPreparedMutation(ctx context.Context, req *gatewayv1.Apply
305305
return applyPreparedFailure(fmt.Sprintf("prepared bytes for %q are unavailable or corrupted", file.GetPath())), nil
306306
}
307307
// The prepared bytes were previewed, hashed and re-verified against the
308-
// tree under serviceRoot(), so they must be written to that same tree.
309-
// Routing this write through the agent instead would target whatever
310-
// root the agent resolved (a declared source-dir moves it), writing
311-
// bytes derived from one file over a different one.
312-
response, err := s.rootedSourceExecute(ctx, s.serviceRoot(), &codev0.CodeRequest{Operation: &codev0.CodeRequest_WriteFile{WriteFile: &codev0.WriteFileRequest{
308+
// agent's source tree, so they are written to that same tree. Writing
309+
// anywhere else puts bytes derived from one file on top of another of
310+
// the same name.
311+
response, err := s.rootedSourceExecute(ctx, s.sourceRoot(), &codev0.CodeRequest{Operation: &codev0.CodeRequest_WriteFile{WriteFile: &codev0.WriteFileRequest{
313312
Path: file.GetPath(), Content: string(after),
314313
}}})
315314
if err != nil || response.GetWriteFile() == nil || !response.GetWriteFile().GetSuccess() {

‎pkg/gateway/server.go‎

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,12 @@ type Server struct {
171171

172172
executionRecorder ExecutionRecorder
173173
executionDispatcher ExecutionDispatcher
174+
175+
// resolvedSourceRoot is the agent's source tree, resolved once at
176+
// construction. A declaration edited afterwards can only make it stale, and
177+
// a stale root fails the prepared-mutation preview check loudly rather than
178+
// writing somewhere unverified.
179+
resolvedSourceRoot string
174180
}
175181

176182
// serviceExecution is the transport-independent behavior consumed by the
@@ -260,6 +266,7 @@ func NewServer(cfg Config) (*Server, error) {
260266
terminals: newTerminalManager(),
261267
executionRecorder: cfg.ExecutionRecorder,
262268
executionDispatcher: cfg.ExecutionDispatcher,
269+
resolvedSourceRoot: engine.AgentSourceRoot(context.Background(), cfg.WorkDir),
263270
}
264271

265272
path := filepath.Join(cfg.WorkDir, "mind.yaml")
@@ -510,11 +517,25 @@ func pluginToAgentName(plugin string) string {
510517
return plugin + ":latest"
511518
}
512519

513-
// serviceRoot returns the absolute path to the service source tree.
520+
// serviceRoot returns the absolute path to the service directory. It is the
521+
// basis for VCS, terminal and code-unit paths, which are addressed relative to
522+
// the checkout rather than to whatever subtree holds the source.
514523
func (s *Server) serviceRoot() string {
515524
return s.cfg.WorkDir
516525
}
517526

527+
// sourceRoot returns the tree the service's agent resolves as its source root.
528+
// A service declaration moves it below the service directory with `source-dir`,
529+
// and the gateway now performs parser-derived work itself, so both sides must
530+
// agree on which tree they read and write. Disagreeing means previewing an edit
531+
// against one file and writing it to another of the same name.
532+
func (s *Server) sourceRoot() string {
533+
if s.resolvedSourceRoot != "" {
534+
return s.resolvedSourceRoot
535+
}
536+
return s.cfg.WorkDir
537+
}
538+
518539
// controlScope returns a control-plane handle scoped to this gateway's service,
519540
// rooted at the service source tree. The Gateway is a thin adapter: generic
520541
// operations (git today) delegate to the one control plane rather than
@@ -528,8 +549,10 @@ func (s *Server) controlScope() control.ServiceScope {
528549
return control.ServiceScopeAt(name, s.serviceRoot())
529550
}
530551

552+
// fileOps reads and writes project bytes, so it is rooted where the agent's
553+
// source lives rather than at the service directory.
531554
func (s *Server) fileOps() codecore.FileOperation {
532-
return codecore.NewFileOps(codecore.LocalVFS{}, s.serviceRoot())
555+
return codecore.NewFileOps(codecore.LocalVFS{}, s.sourceRoot())
533556
}
534557

535558
func (s *Server) validateService(service string) error {
@@ -1169,7 +1192,7 @@ func (s *Server) ApplySymbolPatch(ctx context.Context, req *gatewayv1.ApplySymbo
11691192
// needs a parser — DeclarationSpans and ValidateSyntax — which is exactly why
11701193
// it belongs in the single binary that carries the analyzer.
11711194
func (s *Server) executeSymbolPatch(ctx context.Context, req *gatewayv1.ApplySymbolPatchRequest, rel string) (*codev0.CodeResponse, error) {
1172-
return s.rootedSourceExecute(ctx, s.serviceRoot(), &codev0.CodeRequest{Operation: &codev0.CodeRequest_ApplySymbolPatch{ApplySymbolPatch: &codev0.ApplySymbolPatchRequest{
1195+
return s.rootedSourceExecute(ctx, s.sourceRoot(), &codev0.CodeRequest{Operation: &codev0.CodeRequest_ApplySymbolPatch{ApplySymbolPatch: &codev0.ApplySymbolPatchRequest{
11731196
File: rel, QualifiedName: req.GetQualifiedName(), ExpectedDeclarationSha256: req.GetExpectedDeclarationSha256(),
11741197
NewSource: req.GetNewSource(), FixMode: req.GetFixMode(), DryRun: req.GetDryRun(),
11751198
}}})
@@ -1195,7 +1218,7 @@ func gatewaySymbolPatchResponse(raw *codev0.CodeResponse) *gatewayv1.ApplySymbol
11951218

11961219
func (s *Server) applySymbolPatchWithReceipt(ctx context.Context, req *gatewayv1.ApplySymbolPatchRequest, rel, beforeSHA256 string, attempt *executionrecorder.Attempt) (*gatewayv1.ApplySymbolPatchResponse, error) {
11971220
effectStarted := time.Now()
1198-
raw, err := s.rootedSourceExecute(ctx, s.serviceRoot(), &codev0.CodeRequest{Operation: &codev0.CodeRequest_ApplySymbolPatch{ApplySymbolPatch: &codev0.ApplySymbolPatchRequest{
1221+
raw, err := s.rootedSourceExecute(ctx, s.sourceRoot(), &codev0.CodeRequest{Operation: &codev0.CodeRequest_ApplySymbolPatch{ApplySymbolPatch: &codev0.ApplySymbolPatchRequest{
11991222
File: rel, QualifiedName: req.GetQualifiedName(), ExpectedDeclarationSha256: req.GetExpectedDeclarationSha256(),
12001223
NewSource: req.GetNewSource(), FixMode: req.GetFixMode(), DryRun: false,
12011224
}}})
@@ -1505,7 +1528,7 @@ func (s *Server) GetProjectInfo(ctx context.Context, req *gatewayv1.GetProjectIn
15051528
inventory := pi.GetSourceFiles()
15061529
inventoryFailure := failures.Clone(resp.GetFailure())
15071530
if pi.GetSourceFilesOmitted() {
1508-
root := s.serviceRoot()
1531+
root := s.sourceRoot()
15091532
if inspected != nil {
15101533
root = inspected.root
15111534
}
@@ -1551,10 +1574,7 @@ func (s *Server) GetProjectInfo(ctx context.Context, req *gatewayv1.GetProjectIn
15511574
// agent declined. An empty inventory without that flag is authoritative — the
15521575
// unit genuinely has no source files — so only a declined one is refilled here.
15531576
func (s *Server) takeSourceImports(ctx context.Context, root, language string) ([]*codev0.SourceFileInfo, error) {
1554-
if s.host == nil {
1555-
return nil, fmt.Errorf("workspace source behavior is unavailable")
1556-
}
1557-
source, err := s.host.SourceAt(root)
1577+
source, err := s.boundSource(root)
15581578
if err != nil {
15591579
return nil, err
15601580
}
@@ -1566,6 +1586,11 @@ func (s *Server) takeSourceImports(ctx context.Context, root, language string) (
15661586
// the engine, which carries the source-semantics analyzer. Routing it to the
15671587
// owning agent instead would make every language agent link the tree-sitter
15681588
// CGO stack to answer an operation none of them implements.
1589+
//
1590+
// A source_agents entry for the unit therefore selects nothing here: the
1591+
// projection is language-neutral and the analyzer is the same one every agent
1592+
// would have installed. The entry still selects the agent for operations an
1593+
// agent actually owns.
15691594
func (s *Server) GetSemanticIndex(ctx context.Context, req *gatewayv1.GetSemanticIndexRequest) (*gatewayv1.GetSemanticIndexResponse, error) {
15701595
requestedService := ""
15711596
if req != nil {
@@ -1606,17 +1631,23 @@ func (s *Server) GetSemanticIndex(ctx context.Context, req *gatewayv1.GetSemanti
16061631
// exact root, so a parser-derived answer describes that tree and no other. The
16071632
// host-wide behavior of sourceExecute can sit above the root in question.
16081633
func (s *Server) rootedSourceExecute(ctx context.Context, root string, request *codev0.CodeRequest) (*codev0.CodeResponse, error) {
1609-
if s.host == nil {
1610-
return nil, fmt.Errorf("workspace source behavior is unavailable")
1611-
}
1612-
source, err := s.host.SourceAt(root)
1634+
source, err := s.boundSource(root)
16131635
if err != nil {
16141636
return nil, err
16151637
}
16161638
defer func() { _ = source.Close() }()
16171639
return source.ExecuteCode(ctx, request)
16181640
}
16191641

1642+
// boundSource binds engine source behavior to an exact root. The caller owns
1643+
// the result and closes it.
1644+
func (s *Server) boundSource(root string) (*engine.Source, error) {
1645+
if s.host == nil {
1646+
return nil, fmt.Errorf("workspace source behavior is unavailable")
1647+
}
1648+
return s.host.SourceAt(root)
1649+
}
1650+
16201651
// GetSourceManifest invokes the language-neutral rooted Code behavior so every
16211652
// workspace has the same body-free artifact inventory regardless of which
16221653
// language agents are installed. The Gateway forwards typed identities only;

0 commit comments

Comments
 (0)