Skip to content

feat(harness): compile focused agent briefs - #786

Open
ynadge wants to merge 10 commits into
conductor/e4-sap-3068-authoring-toolsfrom
conductor/e4-sap-3070-brief-compiler
Open

feat(harness): compile focused agent briefs#786
ynadge wants to merge 10 commits into
conductor/e4-sap-3068-authoring-toolsfrom
conductor/e4-sap-3070-brief-compiler

Conversation

@ynadge

@ynadge ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add the deterministic architecture/build-plan compiler with focused per-agent briefs, categorized fingerprints, bounded diagnostics, and prompt-safe bootstrap projections
  • add canonical targeted impact evaluation and atomic service/store persistence using separate effective and committable brief sets
  • wire proposal-backed production authoring while keeping confirmed-revision resolution explicitly fail closed
  • add stock-research golden coverage plus determinism, malformed-input, impact, lineage, rollback, and production integration tests

Review remediation

  • prove contract direction only between agent/subagent endpoints while allowing connected typed paths through third-agent-owned intermediary nodes; disconnected same-contract carriers and non-agent endpoints fail closed
  • compute impact reasons from the complete graph change set before bounding emitted evidence
  • make apply replay results identical above 128 assignments and verify the canonical digest of every persisted receipt impact
  • expose a curated, standalone-compiled package-root API whose current compiler records are v2, while retaining v1 aggregate read/migration compatibility internally for sequential stack releases
  • align production contract validation with compiler relay semantics: intermediary ownership is allowed only on a fully connected typed path whose terminals are agent/subagent actors
  • index complete graph-change sets once for targeted impact lookup, expose a coded compiler-boundary error for legacy results, and document owned non-agent terminal rejection

Verification

Exact head 2de609c6c0da7435ff35bf7d046f3b3a5aed3658, stacked on SAP-3068 head 9cf604dc26172310e393dd05fc9b8ee2c63f0243:

  • final compiler/impact/package-root focused suite: 23 tests passed
  • full harness unit suite: 216 files, 3,423 tests passed
  • full harness performance suite: 3 files, 10 tests passed
  • harness typecheck, including the standalone package-root consumer: passed
  • harness lint: passed
  • harness production build: passed
  • scoped Prettier check: passed
  • exact-base diff check: passed
  • independent exact-head delta review: approved with no blockers

Stack

Stacked on SAP-3068 and transitively SAP-3067.

Closes: SAP-3070
Depends on: #785

…tools' into conductor/e4-sap-3070-brief-compiler

# Conflicts:
#	packages/harness/src/server/index.ts
…tools' into conductor/e4-sap-3070-brief-compiler
…tools' into conductor/e4-sap-3070-brief-compiler
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Review — PR #786 feat(harness): compile focused agent briefs

1. Unbounded recursion: a cyclic dependsOn crashes every compile

packages/harness/src/core/builder-bootstrap-context.ts:67

const visit = (id: string): void => {
  const milestone = index.get(id as BuildMilestone["milestoneId"]);
  if (!milestone) return;
  selected.add(id);            // no `if (selected.has(id)) return` guard
  milestone.dependsOn.forEach(visit);
};

projectBuildPlanVersionSchema (shared/build-plan-codec.ts:267-277) rejects only
self-references and unknown milestone ids — a two-node cycle (m1.dependsOn=[m2],
m2.dependsOn=[m1]) is accepted. createBuilderBootstrapContext is called for every agent by
compileAgentBriefs, so a planner authoring two mutually dependent milestones via
build_plan_apply / build_plan_validate recurses until RangeError: Maximum call stack size exceeded. That is not a BuilderBootstrapLimitError, so the catch at
agent-brief-compiler.ts:1127 rethrows it and build-plan-service.ts:1662 has no mapping — the
tool call dies with an unhandled error instead of a bootstrap-limit-exceeded diagnostic. Same
crash for any consumer of the now-public createBuilderBootstrapContext. Add a visited guard, and
reject dependsOn cycles in the codec.

2. New public exports reference types no consumer can import

packages/harness/src/index.ts:36-79

The package exports only . (package.json:27-33), so deep imports of ./shared/build-plan.js
do not resolve. These newly exported types name types that are not re-exported:

  • CompileAgentBriefsRequestAgentMapGraph, ProjectBuildPlanVersion
  • CompileAgentBriefsResult / CompiledBriefCandidateAgentBriefVersionRecord
  • BuildPlanImpactResultAssignmentImpact, ImpactDigest
  • BuilderBootstrapContextFocusedAgentBriefProjection, PlanNodeSummary, BuildMilestoneSummary

A consumer cannot declare a variable to call compileAgentBriefs with, or write a function taking
an AssignmentImpact. This directly contradicts the comment on line 34: "include every
branded/member type needed to construct and consume the six records."
Likewise
AgentBriefCompilationError and BuilderBootstrapLimitError are thrown by exported functions but
not exported, so callers can only discriminate on error.name.

Also worth deciding before publish: compileAgentBriefs, createBuilderBootstrapContext and
serializeBuilderBootstrapContext are internal orchestration seams the server wires up itself.
If nothing outside this repo needs them, keeping them internal is cheaper than committing to them.

3. 🔒 Internal tracker IDs ship inside the npm tarball

JSDoc on exported declarations is preserved into dist/*.d.ts, which is in files:

  • builder-bootstrap-context.ts:183/** SAP-3074 may place this canonical, escaped payload inside trusted delimiters. */ on the exported serializeBuilderBootstrapContext: an internal ticket id plus unreleased roadmap intent.
  • agent-brief-compiler.ts:1212/** Production adapter for SAP-3068's authoring orchestration seam. */ on the exported DeterministicAgentBriefCompiler.

Rewrite to the behavior ("Serializes the bootstrap context as escaped, delimiter-safe JSON for a
trusted host prompt"). Related: build-plan-service.ts:123,130 still read "Fail-closed production
seam until SAP-3070 supplies the real compiler" — SAP-3070 is this PR, so the comment is now
false as well as internal.

4. Impact is embedded in the size-bounded, persisted receipt without truncation

build-plan-service.ts:1070-1080, 1743-1750

Diagnostics are explicitly slice(0, BUILD_PLAN_MAX_DIAGNOSTICS) before entering the receipt, but
impact is not truncated anywhere. impactSchema caps assignmentChanges at 256 with 9 reasons
each, and each reason's affectedNodeIds / affectedRelationshipIds / affectedContractIds are
uncapped unique(...) projections of the fingerprint refs — plus changedNodeIds covers the whole
graph on a first compile. On a large plan this crosses BUILD_PLAN_MAX_RESULT_BYTES (512 KB) and
bounded() turns an otherwise valid build_plan_apply into result_too_large, with no recovery
the planner can act on. Cap the id lists (or store a reduced impact in the receipt).

5. Brief↔plan version binding removed from the validator, unmentioned in the changeset

build-plan-contract-validator.ts:136-142 drops the brief.plan.version / brief.plan.semanticDigest
equality check, leaving only planId. This is required by the new effectiveBriefs set (which
deliberately reuses briefs carrying an older plan.version), but nothing replaces it: the validator
can no longer distinguish a brief the compiler intentionally preserved from one that was never
recompiled. It is a behavior change to a fail-closed boundary and deserves a line in
.changeset/quiet-planners-author.md, which currently mentions only additions.


Verdict: Request changes — fix the milestone-cycle recursion (#1) and the JSDoc leak (#3) before
merge; #2 is a publish-once API commitment worth settling now.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #786 (delta since 47d6dacd)

Earlier findings: #1 cycle recursion ✅ (visited guard + codec acyclic check), #3 SAP-id JSDoc ✅, #4 impact size ✅ (boundBuildPlanImpact), #5 changeset ✅. #2 fixed but over-corrected — see New 2.

New 1 — v1 brief compat scaffolding for a shape that was never published

shared/build-plan.ts:275,317-330, build-plan-canonicalization.ts:130, build-plan-codec.ts:513

The commit adding build-plan.ts (42233439) is not on main, so AgentBriefVersionRecord
schemaVersion 1 has never shipped in a tarball and no store can hold one. The push nonetheless
bumps to v2 and adds a permanent public union — LegacyAgentBriefVersionRecord,
LegacyBriefContractPort, LegacyDependencyFingerprint, PersistedAgentBriefVersionRecord,
legacyAgentBriefVersionRecordSchema, parsePersistedAgentBriefVersionRecord — threaded through
CompiledBriefCandidate.brief, BuildPlanImpactEvaluator.evaluate, and
BuildPlanningAggregateV1. A consumer reading brief.dependencyFingerprints[0].nodeIds must now
narrow on schemaVersion for a variant that cannot exist, and fingerprintReasons
(build-plan-impact-evaluator.ts:141-153) carries a dead branch that degrades stale reasons to a
bare digest compare. Emit v2 only and drop the union before this is published once.

New 2 — export type * makes the whole module public API

src/index.ts:38. The curated list was replaced with export type * from "./shared/build-plan.js",
so every type in that file — plus anything added to it later — is a permanent contract.
type-tests/public-build-plan-consumer.ts already enumerates the ~14 types that were actually
unreachable; re-export that set explicitly instead.

New 3 — connectedFlowEvidence turns a silent skip into a compile abort

agent-brief-compiler.ts:291,474-491. Where the old filter dropped one dependency, an unreachable
contract now pushes incompatible-contract-direction, which reaches
AgentBriefCompilationError and rejects the whole build_plan_apply. isAllowedNode admits only
provider/consumer-rooted nodes plus unrooted artifact/resource/connector, so a contract routed
through a third agent's node fails the entire compile. Only the disconnected-carrier shape is
tested; add the relay case and put this behavior change in the changeset.

Nit: graphChanges (build-plan-impact-evaluator.ts:127) truncates to 128 before
fingerprintReasons filters against it, so past 128 changed nodes a stale reason can report empty
affectedNodeIds and an assignment can read preserved rather than presentation-refreshed.

Verdict: Request changes — New 1 and New 2 are publish-once commitments; New 3 needs the test.

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Resolved the latest exact-head findings in 86ec3b4482d2546fbf3d4447fb623324f785a0f5:

  • apply replay now uses the same deterministic 128-assignment projection as the initial result
  • aggregate reads recompute and verify every persisted receipt impact digest
  • the package root now has a curated v2-only compiler API; sequential-release v1 persistence compatibility remains internal
  • connected flow proof accepts third-agent relay intermediaries but requires true agent/subagent endpoints, with disconnected-carrier and non-agent-endpoint regressions
  • impact reasons use the full graph change set before emitted evidence is bounded

Verification is green: 101 focused tests; 3,418 full unit tests; 10 performance tests; typecheck (including standalone root consumer), lint, production build, scoped Prettier, and exact-base diff check.

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

/review

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

/review

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Resolved the compiler/validator relay integration blocker in 98f9a89237069f04db045c198448548854e653c4.

The validator now allows third-agent-owned intermediary nodes only when the submitted typed evidence forms one complete directed path between producer and consumer agent/subagent terminals. It continues to reject disconnected same-contract carriers and owned non-agent terminals.

Coverage now passes the compiler's relay output through BuildPlanContractValidator and exercises the real production BuildPlanService commit path, alongside direct negative regressions.

Verification is green: 60 focused compiler/validator/service tests; 3,422 full unit tests; 10 performance tests; typecheck, lint, production build, scoped Prettier, and exact-base diff check.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #786 (delta since bf15a1af)

Earlier findings — all addressed. New 1 ✅ at the public boundary: CompiledBriefCandidate.brief
and CompileAgentBriefsRequest.previous.briefs are v2 AgentBriefVersionRecord, and no
Legacy*/Persisted* type is exported from src/index.ts — the union survives internally only,
which is a fair call for sequential-release persistence. New 2 ✅ — export type * replaced with a
curated list; DeterministicAgentBriefCompiler, CanonicalBuildPlanImpactEvaluator,
AgentBriefCompiler and AgentBriefCompileResult are no longer root exports. New 3 ✅ — relay path
now covered through the compiler, BuildPlanContractValidator and the real BuildPlanService, and
named in the changeset. Round-2 nit ✅ — graphChanges truncates after fingerprintReasons, with a
130-node regression test. No new confidentiality issues: the changeset addition is role-generic and
no SAP id remains in this PR's source.

New 1 — un-truncating changedNodeIds made reason filtering quadratic

build-plan-impact-evaluator.ts:169-179, 123-129

affected() filters with changed.includes(id) — a linear array scan. That was fine while
graphChanges capped each list at 128; the fix removed the cap, so changed is now the full graph
change set (the graph has no schema size cap). Cost is now O(fingerprintIds × changedIds) per
fingerprint kind per brief, run inside build_plan_apply: a 500-node revision across ~200 briefs is
~10^8 string compares where it used to be ~10^5. Build the three Sets once in graphChanges and
membership-test against those; the correctness fix is unaffected.

Nits

  • agent-brief-compiler.ts:1388 — public compileAgentBriefs throws a bare
    Error("current compiler input produced a legacy brief"); an untyped JS caller gets an error it
    cannot discriminate. Use AgentBriefCompilationError or document it.
  • The changeset covers relays and disconnected carriers but not the other new fail-closed shape:
    an agent-owned artifact/resource as a contract terminal now hard-fails the compile
    (agent-brief-compiler.test.ts:290).

Verdict: Approve — remaining items are non-blocking.

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Cleared the three approved follow-ups in 2de609c6c0da7435ff35bf7d046f3b3a5aed3658:

  • impact evaluation now constructs node, relationship, and contract change sets once and uses constant-time membership throughout per-brief evidence selection; the existing >128-node regression continues to cover the full unbounded reasoning set
  • the public legacy-result guard now throws AgentBriefCompilationError with discriminant legacy-brief-result, covered by a runtime boundary regression
  • the changeset now explicitly documents that owned artifacts, resources, and connectors cannot serve as producer/consumer terminals

Verification is green: 23 focused tests; 3,423 full unit tests; 10 performance tests; typecheck, lint, production build, scoped Prettier, and exact-base diff check.

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Independent delta review approved exact head 2de609c6c0da7435ff35bf7d046f3b3a5aed3658 with no blockers. The PR body now records the exact-head approval and final verification evidence. No code changed after the approved head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant