Skip to content

Separate stable agent identity from mutable presentation - #136

Merged
schickling merged 23 commits into
mainfrom
schickling-assistant/2026-07-31-agent-presentation-main-transplant
Aug 1, 2026
Merged

Separate stable agent identity from mutable presentation#136
schickling merged 23 commits into
mainfrom
schickling-assistant/2026-07-31-agent-presentation-main-transplant

Conversation

@schickling-assistant

@schickling-assistant schickling-assistant commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Problem

Agent Spec overloads one identity string as both the durable automation key and the human label. Renaming that label risks routing, state, task, and process continuity. The sibling name file also creates a competing source of truth.

Contract

  • Preserve positional, child, TOML, and JSON identity as the sole stable automation ID. There is no identity rename or compatibility alias.
  • Add optional, independently mutable name and description presentation fields. They are non-unique and never routing, selection, authorization, state, or lifecycle aliases.
  • Retire the sibling name file without a read/write shim.
  • Provide source-preserving KDL-only st2 rename and st2 describe; JSON, TOML, and explicitly Nix-owned declarations refuse mutation.
  • Serialize cooperating local presentation writers with one persistent catalog lock, recheck exact source bytes/version, and publish by atomic replace plus directory fsync.
  • Patch live PTY presentation by exact task ID. Every managed PTY may carry the stable owner/schema/optional-description projection; only the primary agent PTY receives native displayName. Presentation failure never authorizes lifecycle action.
  • Keep Nix ownership-marker adoption as a deployment gate before presentation authoring is used.

Current integration

Current exact head: 70729a34823487e31adfeaee78581e729aa5389c.

Merge commit 6c95a74bd16a21e4aff732eb8b6721d4083336c2 has parents:

  • prior reviewed PR head 79b0e483d1a2f324386ac57498ca45d78157f4c7;
  • st2 main at integration time, 8ca0f3653aae6ca6ebcb880f82e9296016df6e08.

The conflict resolution preserves both independent contracts added to tests/nomad_survival.rs: managed color-environment persistence from main, and presentation metadata continuity from this PR.

The packaged PTY dependency pins merged PR #142 commit 504ac7332895fe1fa3767b530dcd99f091f56cda. Its source tree is identical to the previously reviewed PR head 0deb3f0.

Fifteen adversarial findings are fixed with regressions:

  • 4061a1d: terminate and reap the metadata child after early stdin failure.
  • 624550c: keep direct-child reaping within the existing deadline and hand off any remaining wait.
  • afb40bb: retain resolved host and source identity separately, so dotted hosts are never parsed out of a bus ID.
  • d758dd1: put metadata stdin writing under the existing child deadline.
  • 47d205f: use caller-thread nonblocking writes so an escaped undrained reader cannot retain a writer thread or payload.
  • ffffb1f: enforce the shared deadline before every nonblocking write attempt, including continuously progressing writes.
  • aad301e: preserve the literal st2-owned presentation snapshot at initial spawn while expanding ordinary task tags.
  • 9c4c18e: clear dedicated presentation fields byte-preservingly in CRLF declarations.
  • 391a684: suppress a PTY display name equal to its lifecycle ID at spawn.
  • 222cab5: preserve a pre-provisioned external eval requester route without restoring presentation-name routing.
  • 1cac690: keep lifecycle-equal display names suppressed during later live reconciliation.
  • 3d00dcd: filter exact observed metadata, run lifecycle work first, and visibly bound each presentation-repair batch.
  • 3187b79: reject external requesters that collide with admitted presentation names before spawn.
  • 3187b79: rotate the bounded presentation-repair batch so persistent early failures cannot starve later drift.
  • 70729a3: reject cross-spelling collisions between admitted bus IDs and bare identities before spawn.

The final undrained-reader regression uses a direct kernel pipe, with no shell, external command, or process-lifetime dependency (cf8adbe).

Verification

Surface Result
GitHub exact-head Nix run #253 PASS
exact-head owner unit selection 206 passed, 0 failed
reconcile integration selection 33 passed, 0 failed
run integration selection 28 passed, 0 failed
cargo check --workspace --all-targets PASS
canonical external-requester eval repro PASS
presentation authoring unit selection 6 passed, 0 failed
stdin failure/deadline/undrained-reader regressions 4 passed, 0 failed
expired cleanup deadline regression PASS
presentation source authoring 9 passed, 0 failed
stable-ID message routing 7 passed, 0 failed
live no-restart presentation continuity PASS
inherited managed-color contract PASS
packaged PTY fleet contract PASS
hooks, completions, help, atomic PTY snapshot PASS
git diff --check PASS

The local full Nix gate was indeterminate before evaluation because the configured binary cache returned HTTP 522. GitHub's independent exact-head full Nix gate completed successfully. The optional repository-wide rustfmt probe remains red on pre-existing formatting outside this semantic merge; no formatting-only rewrite is included.

Review result and remaining gate

Adversarial review has covered identity/routing separation, actor guardrails, Nix refusal, source-shape preservation, local writer serialization, symlink refusal, exact-ID PTY patching, metadata idempotence and fleet bounds, failure-to-lifecycle isolation, dotted-host identity, bounded child I/O/cleanup, PTY lifecycle/display-name separation, canonical eval requester routing, requester/presentation-name collisions, and bounded-batch fairness. Fifteen exact-head findings were fixed with regressions; no inline thread remains unresolved.

A final adversarial pass is required on 70729a3. The design is accepted in direction. The VRS decision remains Status: draft and explicitly requires Nathan's acceptance of the final successor head before merge. This PR remains unmerged and undeployed until that exact-head acceptance and the final adversarial pass complete.

Deployment ordering

  1. Merge this accepted st2 head.
  2. Publish the portable st2 release and repin downstream artifacts.
  3. Deploy the Nix ownership marker with compatible st2 and PTY binaries as one cohort.
  4. Enable presentation authoring only after that provenance is live.

References

Refs #128.
Refs #127.
Supersedes #133 and #112.
Supersedes the presentation design in #80; remaining #80 work is runner-derived launch identity.
Depends on merged compoundingtech/pty#142.

agent-session-id: dev3.cos-fr-relay
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty

myobie commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Review result for exact head dcf77b758adc5d6260af5f55781745a2a758bcce: RED; keep this PR draft. The exact-head Nix check is green, but the decision and dependency boundaries are not ready.

The approved core direction is narrower: the existing positional/identity wire remains the sole automation key; optional mutable name is non-unique presentation; presentation never routes, selects, authorizes, or changes a live runtime incarnation.

Six blockers remain:

  1. The VRS decision surface is too large. description, its 1,000-scalar contract, KDL authoring, authority, the persistent writer lock, sibling-name retirement, and the all-PTY tag envelope are new decisions that need explicit acceptance.
  2. The claimed authority boundary is not enforceable as written. The CLI trusts caller-controlled ST_AGENT, while absence becomes unrestricted operator mode. This is a trusted-fleet guardrail, not a capability boundary.
  3. Nix ownership is not proved. Refusal depends on meta { managed-by "nix" }, but no current emitter produces that marker. Existing rendered Nix-owned KDL can therefore be treated as mutable.
  4. The publisher can still lose an external write. It rechecks bytes/version and then renames. A supervisor, sync process, or direct writer can commit between those operations and be overwritten; the local lock covers only cooperating st2 writers.
  5. Runtime/dependency boundaries are not ready. The proposal puts agent.actor.path and presentation tags on every PTY, while the accepted launch-identity boundary limited canonical-agent tags to the canonical PTY. PTY PR142 is also review-RED at 706e4f5 for busy-GC and stale unlocked metadata/event writes.
  6. Unrelated and contradictory changes must leave this slice. Explicit-root message routing weakens fail-closed catalog routing and is unnecessary for presentation. The VRS says authoring waits for matching evals and says to keep the PTY pin unchanged, while this head ships authoring and repins the draft dependency. The new INVARIANTS.md entry duplicates material that belongs in the owning VRS.

Please converge as three bounded slices:

  • A — first: reduce PR136 to a small ASD-STE100 VRS-only decision draft with source links. Settle description, authority semantics, publisher/concurrency guarantees, sibling-name migration, and canonical-vs-all-PTY metadata. Remove message-routing changes, INVARIANTS.md duplication, draft dependency pins, and all source-authoring/runtime code. This exact final VRS head needs Nathan's review.
  • B — after A and a merged PTY dependency: implement read/project only—Agent Spec parsing and validation, roster output, exact-ID PTY projection, and no-restart continuity proofs. Do not include source mutation, message routing, or eval-spec expansion.
  • C — later: add authoring only after st2 has a real Nix ownership signal and a publisher contract that cannot lose supervisor/sync writes. Handle sibling-name retirement with an explicit bounded migration.

Do not mark PR136 ready or request Nathan review until slice A is small, internally consistent, source-linked, and fully green.

@schickling
schickling marked this pull request as ready for review July 31, 2026 16:03

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dcf77b758a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/agent_author.rs Outdated
Comment thread src/agent_author.rs Outdated
Comment thread src/agent_author.rs
agent-session-id: dev3.cos-fr-relay
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
@schickling-assistant
schickling-assistant marked this pull request as draft July 31, 2026 16:17
@schickling-assistant

Copy link
Copy Markdown
Contributor Author

Addressed at 47047dedb16053d86675658bcd31ef76c014ca6b; the PR is back in draft.

Triage of the six blockers:

  1. Discuss - VRS scope. I retained description, KDL authoring, sibling-name retirement, and the all-owned-PTY tag envelope because Johannes explicitly approved this requirements change and requested the end-to-end contract. The decision remains Status: draft, names Nathan as the acceptance authority, and is not mergeable on the current approval state. I did not split the approved contract into a different design unilaterally.
  2. Fixed - authority wording. The VRS, decision, README, and implementation docs now say exactly that caller-supplied ST_AGENT is an operational trusted-fleet guardrail, not an authenticated capability; same-UID callers can change or remove it, and absence selects the operator path.
  3. Follow-up gate - Nix ownership. Refusal applies only to declarations explicitly marked meta { managed-by "nix" }; st2 cannot infer an unmarked generator. The VRS now requires Nix emitters to publish that marker before a binary with authoring is activated. The downstream emitter/adoption change is therefore an activation gate, tracked in Epic: stable agent identity and mutable presentation metadata #128, rather than an unproved property of this implementation.
  4. Discuss - publisher boundary. The accepted contract is intentionally one persistent local lock for cooperating st2 writers plus an observed-source recheck. The VRS explicitly excludes direct same-UID writers, independently synchronized hosts, distributed CAS, and adversarial isolation. Preventing a non-cooperating writer between check and rename would require a different authoritative storage/coordination design; this PR does not claim that guarantee.
  5. Follow-up gate / discuss - runtime metadata. PTY feat: add transactional catalog bootstrap #142's busy-GC and unlocked stale-writer findings remain blockers and must be review-green before this PR can merge. The all-owned-PTY actor/description tags remain intentional: they identify every PTY resource owned by the stable agent, while only the primary task receives the mutable native display name. st2 still routes and mutates solely by exact task ID.
  6. Fixed / discuss - unrelated and contradictory changes. The explicit-root message-routing change and duplicate INVARIANTS.md entry are removed. The stale text saying to keep the PTY pin unchanged is corrected. The exact draft dependency pin remains so this end-to-end draft is reproducible; it remains a merge gate rather than being presented as an accepted release dependency.

Current proof after the fixes on the capped ARM builder:

  • changed Rust files pass rustfmt --check;
  • cargo check --workspace --all-targets passes;
  • Agent Spec discovery: 33/33;
  • presentation authoring: 6/6;
  • roster/status: 3/3;
  • git diff --check passes.

There were no inline review threads to resolve. The architectural Discuss items remain visible here for Nathan's decision; the concrete fixes and follow-up gates are recorded in the PR body.

agent-session-id: dev3.cos-fr-relay
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
agent-session-id: dev3.cos-fr-relay
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
agent-session-id: dev3.cos-fr-relay
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
@schickling-assistant

Copy link
Copy Markdown
Contributor Author

Follow-up correction to blocker 6 at exact head 79b0e483d1a2f324386ac57498ca45d78157f4c7:

The black-box eval caught that removing the entire message-routing change also restored an older infallible send fallback. In a populated catalog, st2 message send "Shared Worker" --catalog ... then created a flat mailbox named after mutable presentation metadata instead of refusing it.

The correction does not restore the review-rejected RoutingMode or let explicit --root bypass a populated catalog. Inbox and archive operations now reuse the existing fail-closed box resolver:

  • an exact declared stable identity resolves its native resource box;
  • a missing identity in a populated or malformed catalog fails before any mailbox write;
  • a mutable display name is never an alias;
  • a proven catalog-less legacy raw root retains its existing flat mailbox behavior.

Proof against PTY 0deb3f0:

  • message CLI: 7/7;
  • presentation authoring: 9/9;
  • packaged st2 + PTY fleet producer-consumer contract: PASS;
  • black-box presentation contract: 9/9, including stable-ID-only routing and cleanup.

The PR remains draft and the prior inline threads remain resolved.

@schickling-assistant
schickling-assistant marked this pull request as ready for review July 31, 2026 17:20

myobie commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

HOLD — exact review RED

Reviewed exact head 79b0e483d1a2f324386ac57498ca45d78157f4c7.

The three prior code defects are fixed: positional identity without children, compact inline field-clear separator handling, and post-create file mode under restrictive umask. Their review threads are resolved.

Merge remains blocked:

  • The change is still an unsplit end-to-end scope: 29 files, +2766/-88.
  • Five VRS files carry a material decision surface that still needs its bounded contract and acceptance.
  • Nix-owned authoring depends on an ownership marker, but no current producer/provenance path establishes that marker before activation.
  • The all-owned-PTY actor/tag projection conflicts with the accepted PR VRS: define runner-owned stable launch identity #80 boundary, where only the canonical agent PTY receives canonical agent identity metadata.
  • Owner verification is incomplete: the broad suite excludes two tests and does not provide the full exact owner gate.
  • PTY feat: add transactional catalog bootstrap #142 at exact 0deb3f0ad6eb07f30ad9dea6392b843d4495ad5f is independently owner-GREEN, but the dependency remains unmerged and its GitHub state is READY rather than draft.

Please return PR #136 to draft. Keep it unmerged and undeployed, and keep this exact head outside Nathan's review TODO until these blockers are resolved.

@schickling-assistant

Copy link
Copy Markdown
Contributor Author

Addressed the actionable proof gap at exact head 79b0e483d1a2f324386ac57498ca45d78157f4c7.

Fix - owner verification: the repository's exact owner gate now passes without exclusions:

  • nix flake check --print-build-logs: PASS
  • package/hermetic Rust selection: PASS
  • completions, help, hook replacement, atomic PTY snapshot: PASS
  • packaged PTY fleet contract against 0deb3f0: PASS

Discuss - scope and VRS acceptance: the 29-file end-to-end scope, draft VRS decision, description, cooperating-writer lock boundary, sibling-name retirement, and all-owned-PTY tags remain unchanged. Johannes explicitly requested the end-to-end implementation and has now directed that the PR remain ready for review. This does not resolve the architectural disagreement or substitute for Nathan's acceptance.

Follow-up gate - Nix provenance: downstream Nix emitter support is now pending, but compatible binaries and the marker must still deploy as one gated cohort before authoring is activated. This PR does not claim deployed provenance today.

Discuss - PTY metadata boundary: all owned PTYs retain the stable actor and optional description tags, while only the primary task receives the native display name. The conflict with PR #80's canonical-PTY-only identity boundary remains visible for explicit acceptance.

Follow-up gate - dependency: PTY #142 is independently owner-green and ready for review at 0deb3f0, but remains unmerged.

Per the explicit owner direction, this PR remains ready for review. It is not merge-ready or deployable: merge/deployment remain held on Nathan's bounded VRS/tag acceptance, the PTY dependency landing, and the downstream provenance/deployment gate. The exact head stays outside Nathan's review TODO until those design gates are accepted.

There are no new unresolved inline threads; the architectural Discuss items remain open in this conversation.

@myobie

myobie commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Nathan likes this direction and wants it to advance. CoS reviewed exact head 79b0e48. The design is accepted in direction, but this head is not merge-ready: GitHub reports a conflict with current main, and the PTY dependency pin still names the pre-merge PR #142 head. Please merge current main normally, do not rewrite branch history, resolve the current-main changes, and repin to the compatible merged PTY revision. Keep this contract in the successor: existing identity is the sole stable automation ID; name and description are optional, non-unique presentation and never routing aliases; the sibling name file is retired; a presentation change never restarts work. Every managed PTY may carry the stable owner-path/schema/description projection, but only the primary agent PTY gets native displayName, role=agent, and run.role=coding-agent. This narrows the remaining PR #80 work to runner-derived launch identity. Keep Nix ownership-marker adoption as a deployment gate before presentation authoring is used. Rerun the full owner, live-continuity, source-authoring, message-routing, and packaged-PTY gates on the exact successor. Nathan will review that final VRS head before merge.

# Conflicts:
#	flake.lock
#	flake.nix
#	tests/nomad_survival.rs
@schickling-assistant

Copy link
Copy Markdown
Contributor Author

@codex review

Please adversarially review exact successor 6c95a74bd16a21e4aff732eb8b6721d4083336c2, especially the semantic merge of managed-color and presentation continuity, the merged PTY provenance pin, stable-ID-only routing, source-authoring concurrency/refusal boundaries, and the guarantee that metadata failure cannot authorize lifecycle action.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6c95a74bd1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/run.rs Outdated
agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
@schickling-assistant

Copy link
Copy Markdown
Contributor Author

@codex review

Please re-review exact successor 4061a1d9d5ae1add726768ab64b70cee9cadee87. Verify the P2 remediation terminates the process group and reaps the direct metadata child on missing/failed stdin without weakening the existing timeout bound, and recheck the final semantic merge.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4061a1d9d5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/run.rs Outdated
agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
@schickling-assistant

Copy link
Copy Markdown
Contributor Author

@codex review

Please adversarially review exact successor 624550cbb369b68865f442d975f34891685db034. In particular, verify that the shared cleanup path preserves the existing deadline, never blocks the caller after expiry, eventually reaps the direct child, terminates the process group on both early-stdin failure and ordinary timeout, and does not weaken the final semantic merge or stable-identity contract.

@schickling-assistant

Copy link
Copy Markdown
Contributor Author

@myobie exact successor 624550cbb369b68865f442d975f34891685db034 is ready for Nathan's final acceptance:

  • normal current-main merge and merged PTY provenance are preserved;
  • both adversarial cleanup findings are fixed with regressions;
  • the full local owner gate is green (201 passed), GitHub Nix run 239 is green, and no inline thread remains unresolved;
  • the final exact-head Codex pass is now running.

Please review/approve this exact VRS head when the final adversarial pass is clean.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 624550cbb3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/run.rs
Comment thread src/agent_author.rs Outdated
agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aad301e9a7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/agent_author.rs Outdated
agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
@schickling-assistant

Copy link
Copy Markdown
Contributor Author

@codex review

Please perform the final adversarial review of exact head 9c4c18e619fa0474865a49484b3110d633ec5bf4. Recheck the complete semantic merge and all eight remediations. The latest fix must clear dedicated presentation fields from CRLF declarations without changing any other source bytes. Exact-head GitHub Nix run 247 is green with 206 owner tests, and no inline thread remains unresolved.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9c4c18e619

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/run.rs
agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
@schickling-assistant

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

st2/src/message.rs

Lines 361 to 364 in 391a684

if discovered.specs.is_empty() && discovered.errors.is_empty() {
return Ok(flat());
}
anyhow::bail!("no agent '{id}' found in catalog {}", root.display())

P1 Badge Preserve flat replies to undeclared eval requesters

When --root is a canonical-agent eval catalog, discovery finds valid agent specs, but the requester is intentionally external and undeclared. The canonical fixtures send completion to requester (for example, tests/eval_run_e2e.rs:322 and :510), so this condition now makes resolve_inbox reject those sends instead of writing <root>/requester/inbox; because the scripts suppress the command error, wait_done times out and the canonical completion judge fails. Keep display-name routing rejected, but retain an explicit flat-bus route for external eval requesters.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
@schickling-assistant

Copy link
Copy Markdown
Contributor Author

Addressed the exact-head Codex P1 and two additional independent-audit P1s in three narrow commits:

  • 222cab5: canonical eval now pre-provisions one real external requester inbox; catalog routing accepts only that explicit non-symlink flat route, continues rejecting presentation-name routing, and rejects a requester colliding with an admitted Agent Spec. The canonical eval reproduced a 30s completion timeout before the fix and passes after it.
  • 1cac690: lifecycle-equal presentation names normalize to a clear operation in the shared desired projection, so both initial spawn and later adoption preserve --no-display-name. The live PTY regression preserves PID and creation identity across two passes and emits no redundant metadata event.
  • 3d00dcd: PTY inventory now carries observed display metadata, exact observed projections queue zero child processes, unrelated tags remain untouched, lifecycle work executes before presentation repair, and each pass visibly bounds repair to eight children (16s worst case under the existing 2s child bound).

Local exact-head proof: 206/206 library tests; 33/33 reconcile tests; 28/28 run tests; canonical eval repro green; live PTY continuity green; cargo check --workspace --all-targets and git diff --check green. Waiting for the exact-head GitHub Nix gate before requesting another final review.

@schickling-assistant

Copy link
Copy Markdown
Contributor Author

@codex review

Please perform the final adversarial review of exact head 3d00dcd30f2c0c2d921b41af2151f1eace9bf060. Recheck the complete semantic merge and all twelve remediations, especially the three latest:

  • a canonical eval requester is an explicitly provisioned external flat mailbox, never a presentation-name route or admitted Agent Spec;
  • a primary name equal to its PTY lifecycle ID remains suppressed at both spawn and live adoption;
  • exact observed PTY presentation queues no child, unrelated tags are ignored, lifecycle work precedes metadata repair, and fleet repair is visibly bounded per pass.

Exact-head GitHub Nix run 251 is green; owner unit selection is 206/206; reconcile/run integrations are 33/33 and 28/28; canonical eval and live no-restart PTY regressions are green; no inline thread remains unresolved.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d00dcd30f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/eval_run.rs
Comment thread src/run.rs Outdated
agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
@schickling-assistant

Copy link
Copy Markdown
Contributor Author

@codex review

Please adversarially review exact head 3187b79e08e916ce9ce1156a3a4c6091d5b405fb, including the normal merge conflict resolution and all successor fixes. Focus especially on the two latest contracts: (1) canonical external requester identities must remain identity-routed but must fail closed before spawn if they collide with any admitted presentation name; (2) presentation repair must remain lifecycle-after, bounded to eight sequential children per pass, and eventually attempt every queued drift even when early patches fail persistently. GitHub Nix run #252 passed on this exact head; local gates are 206 unit, 33 reconcile, and 28 run tests, plus the canonical hermetic eval and workspace/all-target check. All 14 prior threads are resolved.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

routes.insert(spec.identity.clone(), route);

P2 Badge Reject collisions between canonical route spellings

When host evalhost admits valid identities worker and evalhost.worker, inserting the latter's bare identity overwrites the former's bus-ID route. A kickoff addressed to bare worker still resolves uniquely to the first spec, but canonical_sup becomes evalhost.worker, so admitted_route delivers the kickoff and reads completion from the second agent's inbox. Strict validation only rejects duplicate bus IDs, not this cross-spelling collision, so reject duplicate keys here instead of silently replacing an admitted route.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

agent-session-id: dev3.dotfiles-cos-misc-issue-1329-upstream
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /nix/store/qlk5xbdfmj5nn1q145j0fx73f2pmdjj7-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/2km8dbiyv3wc484l1hd8n2lh65121qvq-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
@schickling-assistant

Copy link
Copy Markdown
Contributor Author

Addressed the exact-head route-spelling collision in 70729a3.

Canonical route admission now uses no-overwrite map entries for both each bus ID and bare identity, so a cross-spelling collision fails before any task spawns instead of silently redirecting kickoff/completion. The new fail-closed e2e case admits identities worker and evalhost.worker: before the fix it launched and timed out after routing to the wrong inbox; after the fix it rejects with duplicate canonical route before the spawn side effect.

Local proof on 70729a3: 206/206 owner tests, 15/15 eval e2e tests, cargo check --workspace --all-targets, and git diff --check all green. Waiting for the exact-head GitHub Nix gate before requesting the next final review.

@schickling-assistant

Copy link
Copy Markdown
Contributor Author

@codex review

Please perform the final adversarial review of exact head 70729a34823487e31adfeaee78581e729aa5389c. Recheck the complete semantic merge and all fifteen remediations, with particular attention to canonical route-key uniqueness: every admitted bus ID and bare identity must map to exactly one declaration; a cross-spelling collision such as identities worker and evalhost.worker must fail before spawn rather than overwrite a route. Also retain the prior exact-head requester/presentation collision and bounded fair presentation-repair contracts.

GitHub Nix run #253 passed on this exact head. Local exact-head gates are 206/206 owner tests, 15/15 eval e2e tests, 33/33 reconcile, 28/28 run, workspace/all-target check, and diff hygiene. No inline thread remains unresolved.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 70729a3482

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@schickling-assistant

Copy link
Copy Markdown
Contributor Author

@myobie Final acceptance request for exact head 70729a34823487e31adfeaee78581e729aa5389c.

This successor preserves the accepted identity/presentation design and resolves the semantic merge against current main. Fifteen adversarial findings now have regressions; the final Codex pass found no major issues on this exact head; GitHub Nix run #253 passed; all review threads are resolved. Please approve this exact successor head if it preserves your acceptance, so it can be merged without weakening the VRS gate.

@schickling
schickling merged commit 96a7351 into main Aug 1, 2026
1 check passed
@schickling schickling added area:agent-spec Agent Spec format, parsing, and the agent-spec crate · Set: manual area:identity Agent, session, run, and launch-generation identity · Set: manual area:pty PTY sessions and terminal integration · Set: manual origin:agent Filed or primarily produced by an AI agent · Set: manual type:feature New user-visible or system capability · Set: manual labels Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:agent-spec Agent Spec format, parsing, and the agent-spec crate · Set: manual area:identity Agent, session, run, and launch-generation identity · Set: manual area:pty PTY sessions and terminal integration · Set: manual origin:agent Filed or primarily produced by an AI agent · Set: manual type:feature New user-visible or system capability · Set: manual

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants