Skip to content

feat(harness): auto-map agents in empty planners - #791

Closed
ynadge wants to merge 5 commits into
mainfrom
fix/agent-map-auto-bootstrap
Closed

feat(harness): auto-map agents in empty planners#791
ynadge wants to merge 5 commits into
mainfrom
fix/agent-map-auto-bootstrap

Conversation

@ynadge

@ynadge ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Primary change type

  • Bug fix
  • Documentation
  • Feature
  • Tests
  • Dependency update
  • Maintenance or refactor

Problem and motivation

A project can already contain agents and explicit dependencies while its Agent Map workspace is still unstarted. Opening the shipped planning session currently leaves that map empty and waits for the user to know that they must request a map, so existing architecture is not surfaced naturally on first entry.

Summary and scope

Fresh planner sessions now schedule one durable startup turn when the workspace has no confirmed revision, active proposal, or project build plan. A short, single-line request is visible in the CLI, while the detailed read/validate/propose policy stays in trusted planner context. The turn re-reads the authoritative map, inspects project files read-only, validates evidence-backed nodes and relationships, and creates an unconfirmed proposal for the user to review. Existing maps and live, resumed, or rehydrated sessions preserve their prior state and do not replay the startup task. The coordinator retains its existing persistence, retry, authorization, and telemetry protections, uses human-readable retry ordinals rather than exposing internal IDs, and lets accepted user input preempt unfinished startup work.

This does not deterministically promote discovered implementations into confirmed map authority, infer unsupported relationships, confirm proposals, edit project source, launch agents, or begin implementation.

Related work

Related issue or discussion: N/A — focused regression fix for the shipped Agent Map planner startup.

Validation

pnpm --filter @sapiom/harness build — passed
pnpm --filter @sapiom/harness typecheck — passed
pnpm --filter @sapiom/harness lint — passed
pnpm --filter @sapiom/harness test — passed (204 test files / 3,311 tests; 3 performance files / 10 tests)
pnpm terminology:check — passed (509 files)
pnpm provider-copy:check — passed (123 audited files)
git diff --check — passed

Tests and documentation

Updated planner-session, startup-coordinator, and real server/MCP wiring coverage for empty-map activation, nonempty-map suppression, evidence-only trusted instructions, single-line visible delivery, user preemption, retry correlation, and separate readiness/model deadlines. Updated the harness README and added a minor Changeset for the user-visible startup behavior and provider usage.

Compatibility and release impact

  • Breaking or externally visible changes: Fresh planners automatically run the configured planning agent, which may consume provider credits, to inspect and propose an initial Agent Map only when the persisted workspace is unstarted. Accepted user input preempts unfinished startup work; existing, resumed, and rehydrated planner behavior remains compatible.
  • Changeset: Added .changeset/early-maps-start.md for an @sapiom/harness minor release.

Security

  • I have not included secrets, credentials, private data, or unsanitized logs.
  • This pull request does not publicly disclose a suspected vulnerability. I
    will follow the
    Security Policy for
    private reporting.

AI assistance

  • I did not use AI assistance for this change.
  • I used AI assistance and have described it below.

Codex traced the planner startup lifecycle, implemented the scoped bootstrap behavior, and added regression coverage. I reviewed the resulting diff and verified it with the build, typecheck, lint, full unit/integration/performance suite, terminology check, provider-copy check, and whitespace check listed above.

Checklist

  • I read CONTRIBUTING.md, and this contribution follows the direct-PR or issue-first policy.
  • This pull request addresses one focused problem and contains no unrelated cleanup.
  • I added or updated tests, or explained above why tests are not applicable.
  • I ran the relevant build, typecheck, lint, and test commands, or explained
    any N/A checks above.
  • I updated documentation for user-facing changes, or marked it N/A above.
  • I added a Changeset for a published-package change, or explained why it is not applicable.
  • I can explain and maintain every submitted change, including any AI-assisted work.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Review — PR #791 (fix(harness): auto-map agents in empty planners)

No confidentiality issues found in the changeset, README, or code comments.

1. The startup turn is dispatched as visible PTY input — the user sees an 11-line instruction block as their own first message

packages/harness/src/core/planning-session.ts:458 deletes the comment that recorded exactly why synthetic greetings were removed: "Claude Code has no hidden assistant-first turn. A pending greeting is dispatched as ordinary PTY input and therefore appears as a synthetic user message in the raw CLI." Nothing in this PR changes that mechanism — startGreeting() still calls sessionManager.submitInput(sessionId, prompt, true, …) (planner-greeting.ts:1128), so the new prompt is typed into the CLI and rendered in the user-visible transcript.

Result on a fresh empty-map planner: before the user types anything, the transcript opens with eleven lines of internal instructions ("This is an Agent Studio startup task…", "Never confirm, launch, deploy…") plus Internal attempt ID: <uuid>. Never mention this ID in your response. — an instruction that is self-defeating when the prompt containing it is already on screen. The README and changeset describe the behavior as "server-authored" without saying the user reads it. Either the prompt needs a display-only channel (as sessionStartSystemMessage already provides for claude-code) or the copy has to be written for a human audience.

2. join("\n") fragments the prompt on any harness without bracketed paste

plannerGreetingPrompt switched from .join(" ") to .join("\n") (planner-greeting.ts:265). submitInput only keeps newlines literal when bracketed paste is observed-enabled, or on win32 for an adapter declaring assumesBracketedPaste — and claude-code.ts:421 is the only adapter that declares it. For codex / opencode / pi planners on a CLI that never announces ESC[?2004h (and for any of them on Windows), the prompt submits at its first newline: ten stray fragment turns, and decorateLocalEvent correlates on exact entry.text === prompt (planner-greeting.ts:1408), so the attempt is never matched, never marked delivered, and dies on the timeout in finding 3. The old single-line form was immune by construction.

3. Queued user input can now be withheld for five minutes

deliveryTimeoutMs defaults from 45 s to 300 s (planner-greeting.ts:368). While the greeting is generating, enqueue() only downgrades to skipped when the status is already failed (planner-greeting.ts:1250) — otherwise messages sit in the FIFO until the turn resolves. The new prompt tells the model to browse files and run searches, so a permission prompt, a stalled search, or the fragmentation above leaves the turn open to the full deadline: the user types, gets a 202, and their message is not delivered for up to five minutes, versus 45 s before. Consider skipping the startup turn as soon as real user input arrives, rather than only on failure.

4. Patch changeset for a new, non-optional automatic behavior

.changeset/early-maps-start.md is patch, but this is not a bug fix: on upgrade, opening a planner on any project with an empty map now spends provider tokens and reads the repository without the user asking. There is no flag or option to disable it — PlanningSessionService.create() gates only on mode === "created" && emptyProject (planning-session.ts:334). This is a backward-compatible addition (minor) at minimum, and the changeset body should state that the turn runs automatically and consumes provider credits, since that is the only place a consumer will see it. If it is meant to stay a patch, it needs an opt-out.

Nits

  • AGENT_MAP_PLANNER_SESSION_START_MESSAGE is emitted for every created claude-code planner, but its new copy now leads with conditional auto-inspection text and drops the concrete "Start by describing the outcome you want" call to action — users with an existing map read two sentences that do not apply to them before reaching theirs.

Verdict: Request changes — findings 1 and 2 mean the feature is visible-but-unpolished on the default harness and can fail outright on the others.

@ynadge ynadge changed the title fix(harness): auto-map agents in empty planners feat(harness): auto-map agents in empty planners Sep 3, 2026

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the startup review findings in a9bd8de:

  • Moved the detailed read/inspect/validate/propose policy into trusted planner context. The synthetic request visible in the CLI is now a single human-readable line, contains no tool choreography or internal UUID, and retries use only a readable ordinal.
  • Accepted user input now skips or preempts pending, generating, or failed startup work and drains immediately once the session is ready, instead of waiting for the automatic model deadline.
  • Changed the Changeset from patch to minor and documented that the automatic turn uses the configured provider and may consume provider credits.
  • Reworked the native session-start copy to lead with the normal existing-map review workflow.

Post-review validation passed: 51 focused tests, the full harness suite (3,309 unit/integration tests and 10 performance tests), build, typecheck, lint, terminology check, provider-copy check, and git diff --check.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #791 (delta since 58fe8a9f)

Findings 1–4 and the nit from the previous round are addressed: the visible prompt is one
human-readable line with no tool choreography or UUID (kills the fragmentation risk too), the
changeset is minor and states the credit cost, and the session-start copy leads with the
existing-map workflow. Nothing new leaks — no company names, hostnames, or internal references in
the changeset, README, or comments.

New — preemption strips the startup prompt's infrastructure origin

packages/harness/src/core/planner-greeting.ts:1249 calls clearCorrelation() when user input
preempts a generating startup turn. The prompt is already in the PTY, but its
prompt.submitted hook has not necessarily been decorated yet: decorateLocalEvent then finds no
expected entry, so plannerOrigin is never set. session-record.ts:169 uses exactly that field
to keep the synthetic request out of the transcript — without it, the line
Agent Studio automatic request: Inspect this project for existing agents… is persisted as the
user's own first prompt and counted as a human turn, and telemetryPayload reports
origin: "user". The new test asserts this
(planner-greeting.test.ts:113expect(localPrompt.payload).not.toHaveProperty("plannerOrigin")),
so it is the shipped behavior, not a race that might not happen.

Fix: use retireAttemptCorrelation(sessionId, attemptId) for the generating case, as setFailure
already does at line 1168 — a retired entry still decorates as infrastructure while blocking
delivery. clearCorrelation remains right for pending/failed.

Lower confidence

  • Preempting a generating turn now drains queued input straight into the PTY
    (drainsubmitInput, no busy gate). Claude Code queues typed input mid-turn; if codex /
    opencode / pi do not, the preempting message is written into a running turn and can be lost.
    Worth confirming for the non-default adapters before shipping.

Verdict: Approve after fixing the correlation regression; it re-exposes the internal prompt as a
user message, which was the whole point of finding 1.

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Fixed the correlation regression in a551bc5. Generating attempts are now retired rather than cleared when user input preempts them, so a delayed prompt.submitted hook still decorates the startup request as plannerOrigin: "infrastructure", while the retired completion cannot transition the skipped greeting to delivered. The regression test now asserts both the local infrastructure origin and the redacted telemetry origin.

I also checked the lower-confidence adapter concern. Planner requests are schema-limited by SPAWNABLE_HARNESS_KINDS to claude-code and codex; the scaffolded opencode and pi adapters are not spawnable. Both supported interactive TUIs accept input during an active turn, and this is the same SessionManager.submitInput path already used by the session input API. Codex routes active-turn Enter input through its steer/queue flow; Claude Code accepts it as mid-turn queued input. No unsupported adapter is exposed to this path.

Validation on a551bc5: 51 focused tests, full harness suite (3,309 unit/integration tests and 10 performance tests), lint, typecheck, and git diff --check all pass.

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #791 (delta since a9bd8de0)

The correlation regression from the last round is fixed for the generating case: the retired
entry survives in expected, so a late prompt.submitted still decorates as
plannerOrigin: "infrastructure", and onEventPersisted can't promote the skipped turn (status is
skipped, and the observed barrier is retired). Changeset and README are unchanged and clean. The
adapter concern from the last round is genuinely closed — SPAWNABLE_HARNESS_KINDS
(packages/harness/src/shared/types.ts:143) is ["claude-code", "codex"], so opencode/pi
never reach this path.

New — the same wipe still happens on the failed branch

packages/harness/src/core/planner-greeting.ts:1256: the new guard keys on
greeting.status === "generating", so a failed greeting takes the else and still calls
clearCorrelation. But setFailure deliberately retired that attempt
(planner-greeting.ts:1168) for exactly the reason the fix cites — the prompt may be in the PTY
with its hook still outstanding. On injection_failed the expected entry is intentionally retained
as "uncertain" (planner-greeting.ts:1133), and delivery_timeout fires with the prompt already
submitted. Sequence: startup submit fails/times out → user types → enqueue wipes the retired
entry → the hook lands → decorateLocalEvent finds no match → session-record.ts:169 records
Agent Studio automatic request: … as the user's own prompt and planner-greeting.ts:290 reports
origin: "user". Same defect the fix was for, narrower window.

Fix: don't clear for failed at all — the entries setFailure kept are already retired, and a
non-retryable failure already cleared at line 1186. clearCorrelation is only correct for
pending, where nothing reached the PTY.

Same root cause, unreached branch: setFailure retires at 1168 then immediately clears at 1172 in
its own user-proceeded path.

Earlier round got wrong

Last round's prescription said "clearCorrelation remains right for pending/failed". That was
wrong for failed, and this push implemented it as written — hence the finding above.

Note: I could not execute the harness suite in this environment; the finding is from reading the
code paths, not a failing test.

Verdict: Request changes — one-line guard fix; everything else from rounds 1–2 is resolved.

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Fixed both remaining retired-correlation branches in a24ca7d:

  • enqueue now clears correlation only for pending; retryable failed attempts keep the tombstones that setFailure deliberately retained.
  • setFailure's own user-proceeded path now clears only for a pending readiness failure, not for a generating attempt it just retired.
  • Added a regression that simulates an uncertain PTY write, transitions startup to failed, accepts real user input, and proves the delayed startup hook remains infrastructure in both the local event and telemetry.

Validation on a24ca7d: 52 focused tests; full harness suite with 3,310 unit/integration tests and 10 performance tests; lint; typecheck; and git diff --check all pass.

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #791 (delta since a551bc50)

The failed branch is fixed: enqueue now clears correlation only for pending
(planner-greeting.ts:1255), and setFailure's user-proceeded path clears only for a pending
readiness failure (planner-greeting.ts:1172), so tombstones setFailure retired survive for a
delayed hook. Changeset, README and prose are untouched in this delta — nothing new to flag on
confidentiality.

New — the last wipe of a deliberately-retained tombstone: planner-greeting.ts:1186

if (!retryable) this.clearCorrelation(sessionId) runs after line 1168 has just retired the
attempt. Trace which callers can reach it with an entry still present: !accepted,
SessionNotReadyError, non-staged guard rejection and the canDispatch-false path all call
removeExpectedGreeting first (or never registered), so for them the clear is a no-op. The single
case where it removes anything is SessionInputGuardRejectedError(staged: true) — the one the
catch block at planner-greeting.ts:1131-1135 deliberately keeps as uncertain. Fifty lines later
it is deleted anyway, so the retention is dead intent.

Impact if the staged text does reach submission (session-manager.ts:1122 clears the composer with
\x15, which a TUI is not obliged to honor — the same ConPTY blind spot the surrounding comment
documents): the late prompt.submitted finds no expected entry, session-record.ts:169 persists
Agent Studio automatic request: … as the user's own first prompt and telemetry reports
origin: "user". Same defect as rounds 2–3, last branch.

Fix: drop the clear at 1186; the non-retryable callers that need removal already did it explicitly,
and the staged case is exactly the one that must keep its tombstone. Lower confidence than the
prior rounds — it needs the composer clear to fail — but the retain/clear contradiction is real
either way.

I could not run the harness suite in this environment; findings are from reading the code paths.

Verdict: Approve — one residual inconsistency at 1186, worth a one-line fix but not a blocker.

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the final approved-review inconsistency in b738492. The generic non-retryable cleanup is gone: proven-no-submit callers already remove their expected entry before setFailure, while a staged guard rejection now keeps its retired tombstone for any delayed hook.

Added a red/green regression using SessionInputGuardRejectedError(true) that proves the delayed startup prompt is still decorated as infrastructure after the non-retryable failure.

Validation on b738492: 53 focused tests; full harness suite with 3,311 unit/integration tests and 10 performance tests; lint; typecheck; and git diff --check all pass.

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #791 (delta since a24ca7d1)

Resolved. Dropping if (!retryable) this.clearCorrelation(sessionId) (packages/harness/src/core/planner-greeting.ts:1186) is correct: every other non-retryable caller (!accepted, SessionNotReadyError, non-staged guard rejection, both canDispatch-false paths) already removes or never registered its expected entry, so the only entry that line deleted was the staged guard rejection the catch block at 1131–1135 deliberately retains — the delayed prompt.submitted now still decorates as plannerOrigin: "infrastructure". The surviving tombstone can't be misread as success: onEventPersisted returns early on completedAttempt.retired (line 1494), and expected is dropped on session exit (line 1045).

No new findings, and no earlier finding left unfixed. Prose surfaces (changeset, README, comments) are untouched in this delta.

Note: I could not execute the harness suite in this environment; this is from reading the code paths.

Verdict: Approve.

ynadge commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded by SAP-3147 and the verified role-neutral replacement stack culminating in PR #811 (#804, #806, #807, #808, #811). Useful implementation evidence was preserved before closure. This PR remains unmerged; its branch and review history are retained.

@ynadge ynadge closed this Sep 4, 2026
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