Skip to content

feat: append session instructions while preserving configured rules - #585

Open
naiba wants to merge 3 commits into
agentclientprotocol:mainfrom
naiba:feat/session-system-prompt-append
Open

naiba wants to merge 3 commits into
agentclientprotocol:mainfrom
naiba:feat/session-system-prompt-append

Conversation

@naiba

@naiba naiba commented Oct 6, 2026

Copy link
Copy Markdown

Summary

ACP clients that give each session a stable role should not have to repeat that role in every user turn. Add the provider-neutral _meta.systemPrompt.append lifecycle extension and advertise it during initialization.

  • Apply append metadata on new, load, resume, and fork via native developerInstructions; leave Codex's base instructions, tools, history, and compaction alone.
  • Preserve effective user/project developer_instructions and CODEX_CONFIG precedence instead of silently replacing configured rules. Rebuild from configuration on load rather than appending to already-appended restored text.
  • Reject malformed metadata before session side effects. Reject append overrides on already-loaded threads, including idle threads, because Codex ignores those overrides. Clients must unload or use a fresh adapter process; this is not an in-place instruction editor.
  • Leave absent/blank metadata on the existing lifecycle path, without extra config reads or instruction overrides.

Related: #215, #379, #454, #546. This uses the same append-only metadata/capability shape proposed in #454 and addresses the configured-instruction preservation and real-native lifecycle concerns discussed in #215. #546 is AIR-specific and new-session-only; this path is provider-neutral. Replacement/base-prompt overrides remain out of scope. Happy to fold this work into an existing PR if preferred.

Tests

  • Metadata validation, UTF-8 boundaries, all four lifecycle paths, launch-config precedence, session isolation, pagination and loaded-thread rejection.
  • Real Codex + local fixture provider, no credentials: inspect actual model requests across multiple turns, native compaction, fork, process restart/load and an ordinary control session. Assert appended text occurs exactly once in developer messages and never in user messages, while existing developer and base instructions remain. Model responses are deterministic fixtures, not a live-LLM compliance benchmark.
  • Initialization expectations and AIR snapshots include the new capability; historical baseline recordings are unchanged, with the additive change explicitly allowed in the compatibility comparison.
  • Two test-harness portability corrections found during full validation: reap fixture app-server processes per test, and avoid assuming /workspace is not a symlink in file-change-report expectations.

Local validation:

  • bun run typecheck
  • bun run build
  • bunx vitest run --no-file-parallelism --retry=0 — 1,238 passed, 33 existing skips (opt-in E2E/baseline tests); no new skipped tests
  • bun run bundle:all — all six target binaries built

naiba and others added 2 commits October 6, 2026 05:45
Expose provider-neutral systemPrompt append metadata across native thread lifecycle paths. Preserve effective developer instructions and reject ignored overrides on loaded threads. Cover native multi-turn, compact, fork and restored sessions.

Co-authored-by: naiba/CloudCode <hi+cloudcode@nai.ba>
Preserve recovery tracking, session config metadata and fork subscriptions. Reject ambiguous AIR and append instruction overrides before side effects. Leave sufficient native serialization stack headroom on Node ARM64.

Co-authored-by: naiba/CloudCode <hi+cloudcode@nai.ba>
@naiba

naiba commented Oct 10, 2026

Copy link
Copy Markdown
Author

Merged upstream main through v2.2.2 (202e66e) into this PR; head is now d1c0315.

Integration preserves the new app-server recovery tracking, skipped-MCP configuration metadata, and fork subscriptions. AIR customInstructions retains its upstream behavior; supplying it together with nonblank systemPrompt.append is explicitly rejected before configuration reads or thread creation, rather than silently replacing either instruction source. Added behavioral coverage for both cases.

Also reproduced the existing 4,000-level JsonSnapshot test failing on Node 24 ARM64 with a stack overflow. Earlier native-serialization fallback (depth 64 instead of 256) fixes the original assertion without reducing test depth or skipping coverage.

Local validation on the merged tree:

  • typecheck and build passed;
  • Vitest with retries disabled: 1,519 passed, 36 existing opt-in skips;
  • real Codex + isolated local provider fixture passed across turns, compaction, fork and process restart;
  • all six executable targets built.

No live paid-model acceptance is claimed.

Advertise clear capability and rebuild configured developer rules for explicit empty append on unloaded sessions; preserve omitted metadata behavior and test native restart/reset.

Co-authored-by: naiba/CloudCode <hi+cloudcode@nai.ba>
@naiba

naiba commented Oct 10, 2026

Copy link
Copy Markdown
Author

Follow-up: explicit empty append now restores configured developer rules, and initialize advertises systemPrompt.clear: true. Omitted metadata still leaves the native path unchanged. This closes the workflow role-reset gap when reusing an unloaded native session. Added reset coverage with real Codex/local provider; full local suite: 1,520 passed, 36 opt-in skips, all six bundles built. Head: 1a32623.

@naiba

naiba commented Oct 11, 2026

Copy link
Copy Markdown
Author

Hi @nikita-ashihmin could you review this? thx

This branch has not been deployed

No deployments
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