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
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# Codex adversarial shadow review — workforce #279

VERDICT: FINDINGS

## Review state

Final adversarial review completed against draft PR #280 head `3a1fc1eae2572f2c240ba20e4848318e581595d3`. The principal implementation is correctly wired and its normal, duplicate-version, pnpm, valid-workspace-symlink, transport, and runtime-log paths are proven. One fail-closed edge remains incorrect and prevents comprehensive satisfaction.

## Root cause established from the baseline

`packages/deploy/src/bundle.ts` asks esbuild to bundle the persona handler and externalizes only Node builtins plus `@agentworkforce/runtime` and `@agentworkforce/runtime/raw`. Consequently, third-party and transitive package source that reaches the output is frozen into `agent.bundle.mjs`. The staged `package.json`, however, is built independently from esbuild's resolved graph and contains only the separately installed external runtime version. Neither `BundleResult`, the generated runner, sandbox upload, cloud request, `runner.started`, nor the deployments-list JSON surface carries an inventory of the package code actually inlined into the artifact.

This creates two distinct facts that the baseline conflates or omits:

1. `dependencies.@agentworkforce/runtime` describes code intentionally external to `agent.bundle.mjs` and installed at execution time.
2. Bundled-package attribution must describe package-owned inputs that contributed bytes to the actual esbuild output, including transitives and possibly multiple versions of the same package name.

An author dependency range or the repository lockfile alone cannot prove either fact for an already-staged artifact. Restarting the runner cannot change inlined bytes, and publishing a newer upstream dependency cannot propagate without a new resolution and bundle.

## Risky assumptions to disprove

- **Requested/imported is not actually bundled.** Metafile inputs that are fully tree-shaken, external imports, or packages used only by the external runtime must not be reported as inlined code. Evidence should be tied to the output contribution (`bytesInOutput > 0` or an equally strong graph fact), not to `package.json` declarations.
- **Direct is not complete.** The manifest must include transitive package inputs that contribute code, not only the persona's direct dependencies or a hard-coded `@relayfile/*` allowlist.
- **One name does not imply one version.** Nested dependency graphs can bundle two versions of the same package. A name-to-single-version object risks false attribution or last-write-wins nondeterminism; the schema and logs must preserve distinct `(name, version)` identities.
- **Filesystem path is not operator evidence.** pnpm's content-addressed `.pnpm` layout, symlinked workspaces, nested `node_modules`, scoped packages, conditional exports, and realpath-vs-symlink paths must resolve to the package manifest that owns each bundled input. Absolute source paths, pnpm store paths, user names, temp roots, and symlink targets must never enter generated metadata or logs.
- **Nearest manifest is not automatically the owner.** An upward package.json search must stop at defensible package boundaries and verify a valid name/version; it must not accidentally attribute persona-local source to the workspace root, a parent application, or a neighboring package.
- **Workspace version can overclaim provenance.** A workspace package's `package.json` version may label unpublished or locally modified source. If workspace packages are inventoried, the field must remain explicitly a resolved package version, not a claim that the bytes equal the published npm artifact.
- **Missing metadata must not become a lie.** Package-owned bundled code whose manifest is absent, unreadable, invalid, or has no name/version needs a deterministic, tested policy (fail closed or explicit unresolved evidence). Silently omitting it while describing the list as complete is unsafe.
- **Object traversal order is not a contract.** Manifest and log ordering must be explicitly sorted and stable across identical builds, platforms, symlink layouts, and repeated runs.
- **An auxiliary file can go stale or disappear.** The stager deliberately leaves auxiliary files from prior runs untouched; sandbox upload currently names exactly four files; cloud upload serializes only runner, agent, and package JSON. Any sibling manifest must be overwritten, represented in `BundleResult`, uploaded in every mode, and protected from stale residue. Embedding metadata in the already-uploaded package JSON avoids some, but not all, stale-artifact risks.
- **Runtime logging must describe this artifact.** `runner.started` must receive immutable metadata from the staged artifact/runner, not re-resolve the current filesystem or read the author project at cold start. It must remain structured, deterministic, bounded, and free of paths, environment values, tokens, source-map contents, or other secrets.
- **Presence in a staged file is not fleet queryability.** `--bundle-out` makes local package JSON queryable, but deployed fleet queryability requires cloud persistence/response wiring or another supported JSON surface. If cloud changes are outside this repository/iteration, that limitation must be explicit rather than implied complete.
- **Unit mocks can erase the defect.** Tests that replace `BundleStager`, feed a hand-written package JSON, or assert only a mocked cloud POST do not prove the real esbuild graph. Required proof must stage a real bundle from a realistic installed/package layout, inspect the emitted JS and metadata together, exercise transitive and workspace/symlink cases, and show the runtime's actual `runner.started` record.

## Deterministic baseline evidence

- Review base and HEAD before implementation: `533b9669bcad1f06e31c8fa6644f7228e12fc510`; `git diff origin/main...HEAD` was empty.
- Issue #279 states that a deployed artifact inlines `@relayfile/relay-helpers` and `@relayfile/adapter-core`, while generated package JSON pins only `@agentworkforce/runtime`.
- Baseline `packages/deploy/src/bundle.ts` calls esbuild without `metafile: true`, writes only `agent.bundle.mjs`, `runner.mjs`, `persona.json`, and `package.json`, and resolves only the external runtime version.
- Baseline `packages/runtime/src/runner.ts` logs `runner.started` with persona, workspace, schedules, triggers, and integrations only.
- Baseline sandbox upload enumerates exactly those four staged files. Baseline cloud upload includes runner source, agent source, and parsed package JSON; deployments-list parsing exposes no bundle inventory.
- Lockfile evidence illustrates why graph attribution matters: the workspace contains both `@relayfile/adapter-core@0.5.1` and transitive `@relayfile/adapter-core@0.5.6`; a single version inferred from a direct declaration would be false for some bundle graphs.

## End-to-end wiring assessment

The main path is end-to-end wired:

1. esbuild emits a metafile and the stager selects only input entries with `bytesInOutput > 0` for `agent.bundle.mjs`.
2. Valid package owners become a deterministic array of distinct `(name, version)` pairs. Deduplication uses the full pair, so `@relayfile/adapter-core@0.5.1` and `@relayfile/adapter-core@0.5.6` coexist.
3. The generated `package.json` retains the exact external `@agentworkforce/runtime` pin and embeds the path-free `bundleManifest` as the sole persisted source of truth.
4. The generated runner reads that exact object from its adjacent package JSON and passes it to `startRunner`; `runner.started` emits it unchanged.
5. Sandbox transport uploads the package JSON and cloud transport places the parsed package JSON under `bundle.packageJson`. Local operators can query a staged artifact with `jq .bundleManifest <bundle-dir>/package.json`.

Fleet-wide `deployments list --json` queryability still depends on cloud persistence/response work and is explicitly not claimed. That limitation is acceptable under the iteration's “artifact or supported JSON surface” acceptance wording, but it remains an operational follow-up.

The missing-metadata workspace-symlink path breaks the otherwise correct build step: logical dependency identity is erased by `realpath()` before ownership is determined, allowing bundled dependency bytes to be silently omitted from the manifest.

## Findings

### WF279-SHADOW-001 — workspace symlink without its own package metadata is silently omitted

- Severity: HIGH
- File: `packages/deploy/src/bundle.ts` (`findPackageOwnership`, `findPackageOwnershipFromDirectory`, and the `entryOwner` exclusion in `buildBundleManifest`)
- Issue: Ownership discovery immediately realpaths each esbuild input. For a logical `node_modules/missing-workspace` symlink whose target is inside the author project and lacks `package.json`, the search walks from the real target into the author project's package manifest. That owner equals `entryOwner`, so the contributing dependency input is skipped. The bundle contains the dependency bytes while `bundleManifest.packages` is empty. A target beneath a different ancestor package can analogously be falsely attributed to that ancestor. This contradicts the implementation report's fail-closed claim and acceptance focus #5.
- Deterministic reproduction: Create an author project with `{name:"author-project",version:"1.0.0"}`, symlink `node_modules/missing-workspace` to `packages/missing-workspace`, omit a package manifest at the target, import its exported marker from the agent, and stage with the real built `bundleStager`. Observed output at pushed head: `{"stage":"SUCCEEDED","manifest":{"schemaVersion":1,"packages":[]},"bundleContainsMarker":true,"leaksPath":false}`.
- Exact fix: Preserve the logical esbuild input path alongside its realpath. Detect and retain the logical `node_modules` package boundary before dereferencing symlinks; require that dependency boundary/target to supply its own valid package name and version, and never walk above that package root into the consumer or an unrelated ancestor. Continue using real roots for deduplication/cache identity and keep all failure messages path-free.
- Required test: Add a real `bundleStager`/esbuild regression with an author-project package manifest and a `node_modules` workspace symlink to an in-project target that has code but no `package.json`. Assert staging rejects, the error contains a stable inferred dependency label but no temp/absolute path, and the same fixture succeeds with the correct `(name, version)` only after a valid target manifest is added. Add the analogous protection against attribution to a different valid ancestor package.

## Deterministic final evidence

- Exact review target: local `HEAD`, branch remote, and PR #280 `headRefOid` all resolved to `3a1fc1eae2572f2c240ba20e4848318e581595d3`; base was `533b9669bcad1f06e31c8fa6644f7228e12fc510`.
- Scope diff contains only the two iteration artifacts, README/changelogs, deploy bundle/tests/transport tests, and runtime runner/tests. The reviewer artifact is the only additional untracked file.
- `pnpm --filter @agentworkforce/deploy test`: 230 passed, 0 failed. These include real esbuild fixtures for output-only attribution, unused declaration exclusion, deterministic sorting, valid workspace and pnpm symlinks, versionless metadata rejection, and simultaneous `@relayfile/adapter-core@0.5.1` plus `@0.5.6` representation.
- `node --test packages/runtime/dist/runner.test.js`: 13 passed, 0 failed.
- `git diff --check`: passed.
- Real repository graph probe using `packages/delivery/src/index.ts`: emitted `@relayfile/adapter-core@0.5.6`, `@relayfile/adapter-linear@0.4.6`, `@relayfile/relay-helpers@0.4.7`, and `@relayfile/sdk@0.7.40`; retained runtime `4.1.26`; left no external `@relayfile/*` imports; serialized no working-directory path.
- Actual generated-runner probe: staged a real `shadow-probe@7.8.9` package, executed generated `runner.mjs`, observed exit 0, and confirmed byte-for-byte JSON parity between `package.json.bundleManifest` and the structured `runner.started.bundleManifest`; stdout/artifact metadata contained no temp root.
- Adversarial missing-metadata workspace-symlink probe reproduced WF279-SHADOW-001 exactly as documented above.
- PR #280 remained draft. CI `check` was in progress at review time. CodeRabbit's draft-skip notice is informational only and provides no substantive review evidence.

## Scope-matrix status

- `workforce` #279: draft PR #280 delivered at `3a1fc1e`; main wiring is complete, but WF279-SHADOW-001 requires repair and regression proof.
- `relayfile-adapters` #248: deferred dependency-only context; no changes authorized in this iteration.
- `cloud` #2678: deferred; no changes authorized in this iteration.

## Remaining risks

- Until WF279-SHADOW-001 is fixed, a workspace-linked dependency with absent metadata can ship bundled bytes with no corresponding manifest entry or can inherit an unrelated ancestor identity.
- The manifest records a workspace package's declared resolved version, not proof that locally modified bytes match the published artifact; README correctly states this limitation.
- Fleet-wide version queries remain unavailable until the cloud service persists and returns `bundle.packageJson.bundleManifest`; this workforce iteration provides artifact and startup-log queryability only.
- CI completion was not yet available at the review timestamp and must be rechecked before any readiness decision.
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
# Implementation: workforce #279

## Outcome

- `bundleStager` requests an esbuild metafile and derives the manifest from the
inputs that contributed bytes to `agent.bundle.mjs`, rather than from declared
dependencies.
- The generated `package.json` retains the exact `@agentworkforce/runtime` pin
and adds a versioned, deterministic `bundleManifest` containing only package
names and exact versions.
- Manifest entries are deduplicated by the full `(name, version)` pair and
sorted by name then version, so two installed versions of the same package
remain visible.
- Package ownership follows real paths, covering workspace and pnpm symlinks.
A bundled dependency with missing, malformed, or versionless package metadata
fails staging with a path-free error instead of producing an incomplete
manifest.
- The generated runner reads that same manifest from `package.json`, passes it
unchanged to `startRunner`, and the runtime emits it in structured
`runner.started` evidence.
- Existing cloud and sandbox transports already carry `package.json`; regression
assertions now prove the manifest survives both the hosted bundle request and
the sandbox file upload without a second source of truth.
- No secret, absolute path, pnpm store path, or other machine-specific locator is
serialized into the manifest.

## Changed files

- `README.md` — documents the operator-queryable artifact and runtime signal.
- `packages/deploy/CHANGELOG.md` — records the deploy artifact change.
- `packages/deploy/src/bundle.ts` — derives, writes, and wires the exact bundle
manifest while preserving the runtime pin.
- `packages/deploy/src/bundle.test.ts` — real esbuild/staging regressions for
actual inputs, deterministic ordering, duplicate package versions,
workspace/pnpm symlinks, absent metadata, runtime pin preservation, and path
redaction.
- `packages/deploy/src/modes/cloud.test.ts` — proves the hosted deployment JSON
carries the manifest unchanged.
- `packages/deploy/src/modes/sandbox-client.test.ts` — proves sandbox upload
carries the manifest unchanged.
- `packages/runtime/CHANGELOG.md` — records the startup evidence change.
- `packages/runtime/src/runner.ts` — adds the typed manifest input and emits it
on `runner.started`.
- `packages/runtime/src/runner.test.ts` — asserts exact structured startup
evidence.
- `.workflow-artifacts/release-propagation-279/iteration-1/preflight.md` — input
contract supplied for this iteration.
- `.workflow-artifacts/release-propagation-279/iteration-1/implementation.md` —
this implementation and verification record.

## Red-first evidence

After lockfile installation and prerequisite workspace builds, these commands
failed on the new regressions before implementation:

```bash
pnpm --filter @agentworkforce/runtime test
```

- Expected red: TypeScript rejected the new `bundleManifest` runner option.

```bash
pnpm --filter @agentworkforce/deploy test
```

- Expected red: the generated runner did not read/pass the manifest and the
staged `package.json` had no `bundleManifest`.

## Verification commands

```bash
pnpm install --frozen-lockfile
pnpm --filter @agentworkforce/events build
pnpm --filter @agentworkforce/persona-kit build
pnpm --filter @agentworkforce/deploy test
node --test packages/runtime/dist/runner.test.js
node --test packages/runtime/dist/clients/index.test.js packages/runtime/dist/cloud-defaults.test.js packages/runtime/dist/cloud-llm.test.js packages/runtime/dist/cron.test.js packages/runtime/dist/ctx.test.js packages/runtime/dist/define-agent.test.js packages/runtime/dist/harness-process.test.js packages/runtime/dist/proactive.test.js packages/runtime/dist/relay-mcp.test.js packages/runtime/dist/relay.test.js packages/runtime/dist/run-contracts.test.js packages/runtime/dist/runner.test.js packages/runtime/dist/shim.contract.test.js packages/runtime/dist/shim.test.js packages/runtime/dist/simulate/simulate.test.js packages/runtime/dist/to-agent-event.test.js packages/runtime/dist/trajectory.test.js
pnpm run build
pnpm run lint
pnpm run typecheck
git diff --check
```

Results:

- Deploy package suite: 230 passed, 0 failed.
- Runtime runner suite: 13 passed, 0 failed.
- All runtime suites executable on this host, excluding the unrelated
permission-boundary file: 120 passed, 0 failed.
- Workspace build: passed.
- Workspace lint: passed.
- Workspace typecheck, including examples: passed.
- Diff whitespace validation: passed.

The unfiltered runtime command was also run:

```bash
pnpm --filter @agentworkforce/runtime test
```

It compiled successfully and reported 121 passing tests plus 13 failures, all
in `local-preview.test.js`. Those tests explicitly require the patched Node
permission API at Node >=26.3.1; this worktree host is Node 25.8.1. No failing
test touched bundle manifest or runner behavior.

## Real installed graph probe

The built stager was also exercised against the repository's real
`@agentworkforce/delivery` source graph. It produced this path-free manifest,
confirming resolution through installed pnpm inputs rather than fixture
declarations:

```json
{"schemaVersion":1,"packages":[{"name":"@relayfile/adapter-core","version":"0.5.6"},{"name":"@relayfile/adapter-linear","version":"0.4.6"},{"name":"@relayfile/relay-helpers","version":"0.4.7"},{"name":"@relayfile/sdk","version":"0.7.40"}]}
```

## Safety

- No merge, publish, release, deployment, production mutation, or rollback was
performed.
- External writes are limited to the issue branch push and draft PR described
below after all local gates are green.

## Remaining limitation

This repository can guarantee the staged/package artifact, sandbox upload,
hosted deploy request, and runtime startup log. Fleet-wide querying through
`deployments list --json` additionally requires the cloud service to persist and
return `bundle.packageJson.bundleManifest`; that optional cross-repository API
work is not claimed by this iteration.

## Delivery

- Branch: `codex/issue-279-bundle-versions`
- Implementation commit: `790ee7b` (`feat(deploy): record bundled package versions`)
- Draft PR: https://github.com/AgentWorkforce/workforce/pull/280
Loading
Loading