fix: make an uncertain invocation diagnosable and a failed prepare recoverable

Review of the Builder-over-gRPC change found three defects in the new
"only a certain outcome writes a result" design and several smaller ones.

An uncertain outcome (timeout, interruption, invalid payload, invalid
return, import failure) deliberately writes no result document, which
makes stderr the only carrier of the reason. The harness emitted that
line from inside bounded_logs, so it competed with the handler's own
max_log_bytes budget and lost: a handler writing 4000 bytes to stderr
before returning a wrong-typed output left exit 65, no result, and no
reason at all. The diagnostic is now written after the handler's bound
is released, which is where the completion write used to happen.

The harness also never cleared the result path. Verified: a timed-out
attempt leaves an earlier attempt's SUCCEEDED document in place, and
core's Complete ranks a valid result above the launcher's own deadline
termination, matching it to the run by invocation_id alone. A launcher
reusing the path therefore reads a stale success as this run's proven
result, or at best misreports a crash as INVALID_OUTPUT. The harness
now removes the document before reading the request and refuses to
start if it cannot: a document at that path is always this run's. The
test fixture was pre-clearing the path and hiding this, so it no longer
does. write_completion also renamed a NamedTemporaryFile into place,
narrowing the result to 0600; it is now explicitly 0644, since which
accounts may read it is the facility's choice, not the harness's.

RunnableBuildInputs cleared the prepared snapshot before doing anything
fallible, and prepare refuses a non-empty destination. Retrying into
the caller's own output directory therefore failed with codes.Unknown
*and* discarded the good snapshot, leaving a prepared tree on disk that
Package then refused. It now rejects an occupied destination by name
with AlreadyExists before touching state, and installs the new snapshot
only once the replacement is complete. Unexpected failures across the
Builder are wrapped as codes.Internal instead of reaching callers as
codes.Unknown, and an empty artifact list is reported rather than
panicking on artifacts[0].

outputDirectory had copied Confine's symlink-resolution walk verbatim
and already drifted from it; both now share generate.Resolve/Within.
pkg/agent had no tests, so the containment guard that keeps a build
from copying its own output is now covered directly. The deleted
contract_test.go was the only thing comparing this repo's Go constants
against the Python literals; restored against core's own symbols, so a
core rename fails the build instead of exiting 69 in the fleet.

The generated harness bytes and the configuration digest both changed
while the manifest still said 0.0.1, so the same declaration produced
different package bytes under one agent version. Bumped to 0.0.2.

Every behavior change has a test that fails without it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: antoinetoussaint-byte

agent.codefly.yaml

@@ -1,4 +1,4 @@
 publisher: codefly.dev
 kind: codefly:runnable
 name: python
-version: 0.0.1
+version: 0.0.2

builder_grpc_test.go

@@ -28,7 +28,7 @@ func TestBuilderPackagesThePreparedSnapshotOverGRPC(t *testing.T) {
 	root := t.TempDir()
 	t.Setenv(resources.CodeflyHomeEnv, filepath.Join(root, "home"))
 	t.Setenv(manager.AgentSourceEnv, "local")
-	agent, err := resources.ParseAgent(ctx, resources.RunnableAgent, "codefly.dev/python:0.0.1")
+	agent, err := resources.ParseAgent(ctx, resources.RunnableAgent, "codefly.dev/python:0.0.2")
 	require.NoError(t, err)
 	binary, err := agent.Path(ctx)
 	require.NoError(t, err)
@@ -83,6 +83,13 @@ func TestBuilderPackagesThePreparedSnapshotOverGRPC(t *testing.T) {
 	prepared, err := client.RunnableBuildInputs(ctx, &builderv0.RunnableBuildInputsRequest{OutputDirectory: filepath.Join(root, "prepared")})
 	require.NoError(t, err)
 	require.Equal(t, builderv0.RunnableBuildInputsStatus_SUCCESS, prepared.GetState().GetState())
+
+	// Preparing again into the caller's populated directory is refused by name
+	// rather than from inside preparation, and the snapshot already prepared
+	// survives the refusal: the Package below still emits it.
+	_, err = client.RunnableBuildInputs(ctx, &builderv0.RunnableBuildInputsRequest{OutputDirectory: filepath.Join(root, "prepared")})
+	require.Equal(t, codes.AlreadyExists, status.Code(err))
+
 	write(t, filepath.Join(r.Dir(), "handler.py"), "def handle(context, input):\n    return {\"count\": 999}\n")
 	_, err = client.Package(ctx, &builderv0.PackageRequest{Targets: []*builderv0.PackageTarget{{Os: "other", Architecture: runtime.GOARCH}}, OutputDirectory: filepath.Join(root, "unsupported")})
 	require.Equal(t, codes.Unimplemented, status.Code(err))
@@ -106,7 +113,8 @@ func TestBuilderPackagesThePreparedSnapshotOverGRPC(t *testing.T) {
 	unpack(t, response.GetArtifacts()[0].GetPath(), installed)
 	result := invoke(t, installed, pkg.GetArtifacts()[0].GetCommand(), pkg, map[string]any{"text": "one two three"})
 	require.Equal(t, 0, result.exit, result.stderr)
-	require.Equal(t, 3, result.count(t), "the package must execute the prepared handler")
+	require.Equal(t, 3, result.count(t),
+		"the package must execute the prepared handler, so the refused retry discarded no snapshot")
 
 	write(t, filepath.Join(root, "prepared", "runnable", "handler.py"), "def handle(context, input): return {\"count\": 888}\n")
 	_, err = client.Package(ctx, &builderv0.PackageRequest{OutputDirectory: filepath.Join(root, "tampered")})

docs/milestone_3_builder_grpc.md

@@ -23,7 +23,9 @@ published by this work. Core #474 must settle before coordinated fleet release.
 - [gRPC regression](../builder_grpc_test.go): builds and starts the real agent,
   verifies Builder capability, creates and packages over gRPC, edits live source,
   relocates and invokes the prepared archive. It also checks invalid target,
-  source/symlink overlap, snapshot tampering and failed-Load state clearing.
+  source/symlink overlap, snapshot tampering, failed-Load state clearing, and
+  that preparing again into an occupied output directory is refused without
+  discarding the snapshot already prepared.
 - [Native qualification](../qualification_test.go): core encodes the invocation
   and validates/classifies the real Python result; locked third-party dependencies
   and the bundled interpreter survive removal of the preparation environment.
@@ -41,7 +43,7 @@ GOWORK=off go test -race ./...
 uv run --with pytest --python 3.12 pytest -q pkg/harness/tests
 ```
 
-The native Go and gRPC tests pass on macOS/arm64. The harness has 57 passing real
+The native Go and gRPC tests pass on macOS/arm64. The harness has 61 passing real
 process/schema cases. The CLI companion integration creates through the actual
 CLI, packages through this agent, removes source/prepared files, and proves:
 

docs/protocol.md

@@ -62,7 +62,14 @@ This writes `status: FAILED` with `error: {code, message}` and no output. Use it
 only when the operation knows its effect's disposition. An unexpected exception,
 invalid payload, invalid return, import failure, timeout or interruption writes
 no result. Core therefore cannot mistake an unknown external effect for a known
-failure. Exit codes are diagnostics, not portable completion outcomes:
+failure.
+
+Because those outcomes write nothing, the harness removes any document at the
+result path before it reads the request, and refuses to start if it cannot. A
+document at that path is always this invocation's, so a launcher reusing the
+path cannot read an earlier attempt's result as this one's. The result is
+written 0644: which accounts may read it is the facility's choice. Exit codes
+are diagnostics, not portable completion outcomes:
 
 | Exit | Diagnostic |
 | --- | --- |
@@ -90,7 +97,10 @@ Core #474 describes a clock-offset budget based on `deadline - issued_at`; the
 cross-host clock policy needs reconciliation before Kubernetes qualification.
 No distributed deadline or cancellation guarantee is claimed here.
 
-`stdout` and `stderr` carry diagnostics only. The generated harness uses the
+`stdout` and `stderr` carry diagnostics only. The harness's own terminal
+diagnostic is written after the handler's bound is released, so a handler that
+exhausts the budget cannot erase the single record of why an uncertain
+invocation ended. The generated harness uses the
 release's `max_log_bytes` (4 MiB by default), bounding Python, native-library and
 inherited subprocess writes. It may append one truncation marker. The launcher
 must independently enforce its exact transport bound and record truncation.

pkg/agent/agent.go

@@ -36,6 +36,10 @@ func (a *Agent) GetAgentInformation(_ context.Context, _ *agentv0.AgentInformati
 	info := services.Advertisement{
 		CapabilityOnly: true,
 		Backends: runners.BackendSupport{
+			// Deliberately not core's runners/python.HasUVRuntime, which is
+			// identical: importing that package pulls a TOML dependency in for
+			// service runtime selection this agent never does. What is
+			// advertised here is exactly what prepare.Prepare requires.
 			Local:  func() bool { _, err := exec.LookPath("uv"); return err == nil },
 			Docker: false,
 		},

pkg/agent/builder.go

@@ -24,8 +24,14 @@ import (
 	"github.com/codefly-dev/runnable-python/pkg/prepare"
 )
 
-// Builder owns one loaded declaration and its prepared snapshot. Calls are
-// serialized so packaging cannot accidentally use another load's build state.
+// Builder owns one loaded declaration and its prepared snapshot.
+//
+// That state is per-process, not per-caller: the lock serializes calls but
+// cannot tell two callers apart, and Load replaces the state it finds. One
+// connection therefore drives one Runnable at a time — interleaving Load and
+// Package for two Runnables over the same agent packages whichever was loaded
+// last. A caller that builds Runnables concurrently needs one agent process
+// each, which is what manager.Load already gives it.
 type Builder struct {
 	builderv0.UnimplementedBuilderServer
 	mu       sync.Mutex
@@ -60,18 +66,18 @@ func (b *Builder) Load(ctx context.Context, request *builderv0.LoadRequest) (*bu
 	}
 	actual, err := workspace.RunnableLocationOf(r)
 	if err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.Internal, "locate the Runnable in its workspace: %v", err)
 	}
 	actualWire, err := actual.Proto()
 	if err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.Internal, "encode the Runnable location: %v", err)
 	}
 	if !proto.Equal(actualWire.GetIdentity(), id) {
 		return nil, status.Error(codes.InvalidArgument, "Runnable release identity does not match the workspace declaration")
 	}
 	actualDir, err := filepath.EvalSymlinks(r.Dir())
 	if err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.Internal, "resolve the declared Runnable directory: %v", err)
 	}
 	requestedDir, err := filepath.EvalSymlinks(resources.RunnableLocationFromProto(location).Dir())
 	if err != nil || actualDir != requestedDir {
@@ -98,18 +104,18 @@ func (b *Builder) Create(ctx context.Context, _ *builderv0.CreateRequest) (*buil
 	}
 	r := b.runnable
 	if err := generate.Scaffold(r, r.Dir()); err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.Internal, "scaffold the Runnable: %v", err)
 	}
 	for _, input := range []string{generate.ProjectFile, generate.LockFile} {
 		if !slices.Contains(r.Entrypoint.Inputs, input) {
 			r.Entrypoint.Inputs = append(r.Entrypoint.Inputs, input)
 		}
 	}
 	if err := r.Save(ctx); err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.Internal, "save the declaration: %v", err)
 	}
 	if err := generate.GenerateForRelease(r, r.Dir(), b.identity); err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.Internal, "generate the harness: %v", err)
 	}
 	return &builderv0.CreateResponse{State: &builderv0.CreateStatus{State: builderv0.CreateStatus_CREATED}}, nil
 }
@@ -120,7 +126,6 @@ func (b *Builder) RunnableBuildInputs(ctx context.Context, request *builderv0.Ru
 	if b.runnable == nil {
 		return nil, status.Error(codes.FailedPrecondition, "load a Runnable before building")
 	}
-	b.prepared, b.build = nil, nil
 	r := b.runnable
 	if len(r.LibraryDependencies) != 0 {
 		return nil, status.Error(codes.Unimplemented, "internal library dependency preparation is not implemented")
@@ -134,17 +139,26 @@ func (b *Builder) RunnableBuildInputs(ctx context.Context, request *builderv0.Ru
 	if err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "%v", err)
 	}
-	if err := generate.GenerateForRelease(r, r.Dir(), b.identity); err != nil {
+	// Preparation populates the destination and refuses a directory that is
+	// already populated, so an occupied one is rejected here rather than
+	// surfacing from inside preparation as an untyped error.
+	if err := requireEmpty(root); err != nil {
 		return nil, err
 	}
+	if err := generate.GenerateForRelease(r, r.Dir(), b.identity); err != nil {
+		return nil, status.Errorf(codes.Internal, "generate the harness: %v", err)
+	}
 	prepared, err := prepare.Prepare(ctx, r, r.Dir(), root)
 	if err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.Internal, "prepare the Runnable: %v", err)
 	}
 	build, err := pack.Build(r, filepath.Join(prepared.Root, prepare.SourceDirectory), prepared.Toolchain)
 	if err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.Internal, "measure the build inputs: %v", err)
 	}
+	// Only now, with a complete replacement in hand. A failed call leaves the
+	// previous snapshot loaded: it still describes a whole tree on disk, and
+	// Package re-measures it before using it, so it stays packageable.
 	b.prepared, b.build = prepared, build
 	return &builderv0.RunnableBuildInputsResponse{
 		State: &builderv0.RunnableBuildInputsStatus{State: builderv0.RunnableBuildInputsStatus_SUCCESS},
@@ -189,18 +203,18 @@ func (b *Builder) Package(_ context.Context, request *builderv0.PackageRequest)
 	}
 	currentBuild, err := pack.Build(b.runnable, filepath.Join(b.prepared.Root, prepare.SourceDirectory), b.prepared.Toolchain)
 	if err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.Internal, "re-measure the prepared snapshot: %v", err)
 	}
 	if !proto.Equal(currentBuild, b.build) {
 		return nil, status.Error(codes.FailedPrecondition, "prepared build inputs changed before packaging")
 	}
 	evidence, err := pack.Native(b.runnable, b.agent, b.prepared, archive)
 	if err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.Internal, "emit the native archive: %v", err)
 	}
 	build := &basev0.RunnableBuild{}
 	if err := protojson.Unmarshal(evidence.Build, build); err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.Internal, "decode the emitted build: %v", err)
 	}
 	if !proto.Equal(build, b.build) {
 		return nil, status.Error(codes.FailedPrecondition, "prepared build inputs changed before packaging")
@@ -209,7 +223,10 @@ func (b *Builder) Package(_ context.Context, request *builderv0.PackageRequest)
 	// metadata travel in the shared response, never in a language-specific CLI file.
 	var artifacts []*basev0.RunnableArtifact
 	if err := decodeArtifacts(evidence.Artifacts, &artifacts); err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.Internal, "decode the emitted artifacts: %v", err)
+	}
+	if len(artifacts) != 1 {
+		return nil, status.Errorf(codes.Internal, "native packaging emitted %d artifacts, expected exactly one", len(artifacts))
 	}
 	artifact := artifacts[0]
 	return &builderv0.PackageResponse{
@@ -237,43 +254,41 @@ func decodeArtifacts(data []byte, artifacts *[]*basev0.RunnableArtifact) error {
 
 // outputDirectory never allows preparing inside the source, or writing through
 // a symlink into it. This prevents a build from recursively copying its output.
+//
+// generate.Resolve does the resolution so this guard and generate.Confine
+// cannot drift apart: nothing is created until the resolved path is accepted,
+// so a rejected output leaves no directory inside the source or the snapshot.
 func outputDirectory(directory, source string) (string, error) {
 	if !filepath.IsAbs(directory) {
 		return "", fmt.Errorf("output_directory must be absolute")
 	}
-	// Resolve the nearest existing ancestor before creating anything. Rejected
-	// outputs must not leave directories inside the source or prepared snapshot.
-	ancestor := filepath.Clean(directory)
-	var missing []string
-	var root string
-	var err error
-	for {
-		root, err = filepath.EvalSymlinks(ancestor)
-		if err == nil {
-			break
-		}
-		if !os.IsNotExist(err) {
-			return "", err
-		}
-		if _, statErr := os.Lstat(ancestor); statErr == nil {
-			return "", fmt.Errorf("output contains a dangling symlink")
-		}
-		missing = append(missing, filepath.Base(ancestor))
-		parent := filepath.Dir(ancestor)
-		if parent == ancestor {
-			return "", err
-		}
-		ancestor = parent
-	}
-	for i := len(missing) - 1; i >= 0; i-- {
-		root = filepath.Join(root, missing[i])
+	root, err := generate.Resolve(directory)
+	if err != nil {
+		return "", err
 	}
 	source, err = filepath.EvalSymlinks(source)
 	if err != nil {
 		return "", err
 	}
-	if root == source || strings.HasPrefix(root, strings.TrimSuffix(source, string(filepath.Separator))+string(filepath.Separator)) || strings.HasPrefix(source, strings.TrimSuffix(root, string(filepath.Separator))+string(filepath.Separator)) {
+	if root == source || generate.Within(root, source) || generate.Within(source, root) {
 		return "", fmt.Errorf("output_directory must be separate from the Runnable source")
 	}
 	return root, nil
 }
+
+// requireEmpty refuses a destination that already holds something. Preparation
+// populates the whole directory, so overwriting one is how a caller loses an
+// earlier build, and the caller owns this path.
+func requireEmpty(directory string) error {
+	entries, err := os.ReadDir(directory)
+	if os.IsNotExist(err) {
+		return nil
+	}
+	if err != nil {
+		return status.Errorf(codes.InvalidArgument, "inspect output_directory: %v", err)
+	}
+	if len(entries) != 0 {
+		return status.Errorf(codes.AlreadyExists, "output_directory %s is not empty", directory)
+	}
+	return nil
+}