Add product and tech specs for user-configurable LSP servers (gh-8803)#11944
Add product and tech specs for user-configurable LSP servers (gh-8803)#11944bradleyjames wants to merge 11 commits into
Conversation
…devgh-8803) Spec PR for issue warpdotdev#8803 — defines a new `[[editor.language_servers]]` settings entry that lets users register custom LSP servers without modifying Warp's built-in language→server map. Specs: - PRODUCT.md — user-visible behavior. Required vs. optional fields, placeholder substitution (`{{workspace_root}}`, `{{workspace_slug}}`, `{{cache_dir}}`, `{{env_VAR}}`), filetype matching, per-workspace enable persistence, override-of-built-in semantics, and an Eclipse JDT-LS worked example demonstrating the field set is sufficient for non-trivial servers. - TECH.md — implementation plan grounded in the current codebase: descriptor parsing/validation, settings registration via `define_settings_group!`, custom-first dispatch, SQLite persistence schema with a `kind` column on `workspace_language_server`, and per-invariant verification mapping. Implementation continues on the existing code PR (brad/8803-lsp-plugins, warpdotdev#10700). Closes warpdotdev#8803
|
I'm starting a first review of this spec-only pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
The product and technical specs cover the intended custom LSP configuration surface and a phased implementation plan, but several sections contradict each other on behavior that implementers need to treat as source-of-truth requirements. The most serious gap is custom workspace-root selection: the product spec says the open Warp directory is the root, while the tech spec plans root-marker walking and tests Gemfile-based roots without defining any root marker field.
Concerns
- Root resolution/root-marker behavior is undefined and internally inconsistent.
- Placeholder escaping, validation error handling, settings hot-reload subscriptions, and schema-registration guidance conflict across sections.
- Security: launch logging needs explicit redaction rules before fully substituted command arguments, environment values, or initialization options can contain secrets.
Security
- The manual validation plan asks reviewers to confirm fully substituted launch arguments in logs, but the feature explicitly supports env placeholders and hard-coded env values that may carry tokens. The spec needs a redaction policy for command, args, env, and initialization options before implementation.
Verdict
Found: 1 critical, 5 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Resolves 7 findings from Oz's review of the user-configurable LSP servers spec (warpdotdev#11944): - Remove stale root-marker walk text; cross-reference the existing DetectedRepositories workspace-root chain instead - Align the three manager-subscription mentions to the canonical "no subscription, on-demand reads, no auto-restart" rule per invariant 19 - Replace stale hand-written SettingSchemaEntry references with the actual macro-generated registration; restate the residual risks in terms of the conventions that remain unenforced - Drop the `{{{name}}}` triple-brace escape (conflicts with Handlebars semantics); document the resulting gap explicitly - Align tech.md validation description with invariant 23's all-or-nothing rejection rule - Add invariant 32 committing to log redaction via CustomSecretRegex; replace "logs show fully-substituted args" manual validation step with a redaction-safe version; add tech.md guidance on what is safe to log - Define explicit `name` field constraints (1-64 chars from [A-Za-z0-9._-], no `.`/`..`, no leading `.` or `-`) in invariant 1 and wire into invariant 23's settings-error list
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds product and technical specs for user-configurable LSP server descriptors in Warp settings. The specs cover the intended settings shape, matching, placeholder expansion, footer behavior, persistence, runtime plumbing, and validation strategy.
Concerns
- The specs introduce a strategic freeze of all future built-in language support, which is broader than the linked custom-LSP configuration request and should be either confirmed as intentional scope or removed from this feature spec.
- The product and tech specs disagree on settings-error surfacing, especially whether invalid entries create synthesized per-entry banner keys or only the
editor.language_serverskey with details in logs. - The no-migration persistence plan can collide with built-in server enablement rows because custom descriptor names are not reserved from existing serialized built-in keys.
- The proposed redaction helper places app-layer privacy logic inside the lower-level
lspcrate path, which is a security-relevant feasibility gap for the log-redaction guarantee. - The placeholder slug uniqueness guarantee and the settings parser pseudo-code need tightening so implementation and tests do not encode behavior that contradicts the product invariants.
Security
- The log-redaction design depends on
crates/lspapplyingapp/src/settings/privacy.rs::CustomSecretRegex; that dependency direction is not currently available, so the spec needs an implementable ownership boundary before relying on it for secrets in commands, args, env values, or initialization options.
Verdict
Found: 0 critical, 6 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Resolves 6 findings from Oz's re-review of warpdotdev#11944: - Move the redaction primitive out of crates/lsp (which cannot depend on app/) to a trait injected from app at the LSP boundary; describe the injection point and verification flag for Phase 4 - Reserve the five built-in LSPServerType variant names from custom `name` in the validator and invariant 23 settings-error list; align the persistence section with the actual schema (kind column added by migration 2026-05-24-180000), removing the stale "zero schema changes" claim - Align tech.md banner contract with invariant 23: SettingsFileError carries one bare `editor.language_servers` key; per-entry detail goes to log::warn! and stays out of the banner - Replace the SettingsValue from_file_value Rust example (which had a wrong-type silent-accept bug) with prose describing the all-or- nothing behavior; tech docs use pseudo-code/prose, not real code - Soften the workspace_slug uniqueness invariant to match the truncated-hash implementation honestly - Remove the built-in-freeze product strategy claim from both product.md and tech.md (scope beyond warpdotdevGH-8803); preserve the design choice of keeping built-in/custom runtime tracks separate as its own merit
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec PR defines product and technical plans for user-configurable LSP servers via [[editor.language_servers]]. The overall scope is clear, but several details need tightening before implementation: command path trust boundaries, server-name identity/caching behavior, unknown-field handling, and schema generation for the documented filetype forms.
Concerns
- The
commandcontract allows only absolute paths or bare PATH names, but the implementation plan does not say how relative paths with separators are rejected or guarded. - Built-in name reservation and examples conflict around visible built-in names, and cache-directory uniqueness is underspecified on case-insensitive filesystems.
- The tech spec relies on serde/schema defaults in places where the product spec requires custom behavior: logging unknown fields and accepting both bare-string and inline-table
filetypesentries. - Phase 5 contradicts Phase 2b by reintroducing the blanket
Vec<LspServerDescriptor>settings path after the spec already explains why a custom newtype is necessary.
Security
- Custom server commands execute with the workspace root as cwd. Without explicit validation or a user-facing guard for relative paths, a descriptor such as
./serverorbin/servercan execute repository-controlled code when a matching file is opened and enabled.
Verdict
Found: 0 critical, 6 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Resolves 7 findings from Oz's third review of warpdotdev#11944, with deeper investigation against the actual implementation on the code branch to ensure correctness: - Drop the bare-string `filetypes` form from invariant 1 entirely: the implementation only accepts inline tables, all five worked examples already use inline tables, and supporting both forms would need custom Deserialize + custom JsonSchema for a small ergonomic win - Broaden the reserved-name list to cover both serialized variant names (RustAnalyzer, ...) AND binary display names (rust-analyzer, ...), case-insensitively. Reservation sourced from LSPServerType via EnumIter so it tracks the enum. Rename the five override examples to `my-<binary>` since the display names are now reserved - Add SECURITY rule for `command`: after `~`/`~/` expansion, value must be absolute or a bare name with no `/` or `\` separators; relative paths like `./server` or `bin/server` are rejected to prevent a malicious settings.toml from executing repo-controlled code via the workspace cwd - Specify case-insensitive `name` uniqueness in invariant 2 to prevent macOS/Windows cache-dir collisions between `ruby-lsp` and `Ruby-LSP` - Specify the unknown-field logging mechanism for invariant 24: a manual top-level-key walk before `serde_json::from_value`, with the known-field set sourced from `RawDescriptor`'s fields. Serde silently drops unknowns; this is what makes invariant 24's "logged with a warning" promise real without adding `serde_ignored` - Rewrite Phase 5's `SettingsValue` description to route through the `LspServerDescriptors` newtype from Phase 2b (the blanket `Vec<...>` path it previously described would skip matcher compilation) - Replace the last stale "no persistence migration" claim with text reflecting the actual additive `kind` column migration
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec PR defines a user-configurable LSP server feature with product behavior and a phased technical plan. The overall direction addresses the linked issue, but several spec contradictions and design gaps should be resolved before implementation proceeds.
Concerns
- The command-resolution trust boundary is incomplete for bare commands resolved through PATH when the child process cwd is the workspace root.
- The product spec contradicts itself on whether
initialization_optionsis passed verbatim or receives placeholder substitution. - The
filetypesschema alternates between inline-table-only and string-or-inline-table forms. - The tech spec describes the future
kindpersistence column as current architecture, which does not match the current schema and could cause implementation drift. - The logging section classifies workspace/cache paths as non-sensitive without a privacy-safe logging policy.
Security
- Bare-command PATH lookup can still resolve to repo-controlled binaries if PATH contains
.or relative/empty entries while the launch cwd is the workspace root. - Workspace roots and cache directories can contain usernames, customer names, or private repository names and should not be declared safe for verbatim logging without redaction or safe/full log handling.
Verdict
Found: 0 critical, 5 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Resolves 6 findings from Oz's fourth review of warpdotdev#11944. Investigation went through the actual implementation on the code branch and existing process-spawn sites in the codebase (MCP servers, built-in LSPs, CommandBuilder) to ground decisions in current Warp behavior. - Document the PATH-based residual risk for `command` resolution (invariant 16) honestly: bare-name lookup uses the OS process loader, which on Unix execvp and Windows cmd.exe walks the user's $PATH against the spawned cwd (workspace root) — same trust model as built-in LSPs use today via CommandBuilder. Spec chooses consistency with that precedent over a custom-LSP-only mitigation; PATH sanitization in CommandBuilder is tracked as a follow-up that would benefit all LSPs uniformly - Drop the leftover "string or inline-table" mention from invariant 23's settings-error list (consistent with the bare-string drop from the previous round) - Fix the invariant 26 "docs link from invariant 9" cross-reference: invariant 9 doesn't introduce a docs link or any new footer affordance. Install instructions live on the external docs page (per invariant 25's external-tooling JSON Schema) - Align invariant 17 with invariant 5 and the actual expand_json implementation: `initialization_options` is sent *after* placeholder substitution on string leaves, not verbatim. Same fix on the JDTLS worked-example prose and the field definition - Refine invariant 32's redaction classification: `workspace_root` and `cache_dir` are logged verbatim (matching today's built-in LSP log behavior) but explicitly called out as PII-bearing, since CustomSecretRegex is a token-pattern matcher, not a PII filter - Rewrite the "Current architecture" persistence bullet to describe master state (no kind column); the kind column is described as Phase 4 work in the existing Phase 4 persistence section
|
/oz-review |
|
This is your last |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds product and technical specs for user-configurable LSP server descriptors in settings.toml, including matching, placeholder substitution, per-workspace enablement, persistence, schema generation, and logging/redaction behavior.
Concerns
- The security model rejects relative
commandpaths before substitution, but does not require the same absolute-or-bare-name validation after{{env_VAR}}and other placeholders expand. - The persistence plan introduces
kind = 'Custom'rows but does not define the typed event/API shape that carries a custom server name through the existing persistence writer, whose current event only acceptsLSPServerType. - The product and tech specs disagree on whether the existing unsupported-language footer gains a docs-page affordance.
Security
- Resolve the post-substitution command validation gap so placeholder-expanded command values cannot bypass the relative-path rejection and execute workspace-controlled paths.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Resolves 3 findings from Oz's fifth review of warpdotdev#11944: - Document the trust-boundary decision around post-substitution `command`: the literal-form absolute-or-bare-name rule is NOT re-applied after placeholder substitution. Environment variables are user-controlled, the same as the literal text of settings.toml. Matches Warp's existing treatment of user-controlled inputs (MCP, terminal, built-in LSP PATH). Documented in invariant 16 and the tech.md validate.rs description so future readers see the decision and its reasoning - Describe the persistence event/API shape for custom servers. Implementation uses two ModelEvent variants (UpsertWorkspaceLanguageServer for built-ins, UpsertWorkspaceCustomLanguageServer for customs) with their own dispatch arms, rather than a single ServerKey-typed event. Each upsert function sets its own kind column literal; no runtime kind discriminator on the event because the variant choice at the call site already encodes it - Fix the second "docs link from invariant 9" contradiction (in tech.md). The external docs page exists for the feature (per invariants 25 and 26) but is reached through Warp's broader docs surface, not through any new footer affordance — invariant 9's no-new-affordance rule stays intact
|
/oz-review |
|
You've used all 5 |
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds product and technical specs for user-configurable custom LSP servers, covering settings shape, matching semantics, launch behavior, persistence, schema generation, testing, risks, and security-sensitive logging. The specs are generally thorough and grounded in the current LSP/settings architecture.
Concerns
- The testing matrix contradicts the stated all-or-nothing settings-error behavior by saying each error class produces a notification line, while the product and implementation sections say the UI surfaces only the single
editor.language_serversinvalid setting and writes per-entry details to logs. - The custom-server enable flow should explicitly document the trust boundary for executing a user-configured command from the unchanged footer CTA.
- The command validator should clarify whether Windows UNC/root-relative absolute paths are supported or intentionally rejected.
Verdict
Found: 0 critical, 1 important, 2 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Resolves 3 findings from Oz's sixth review of warpdotdev#11944: - Fix the test matrix row for invariant 23: the integration test must assert one banner line for the bare `editor.language_servers` key plus per-error-class detail on `log::warn!` output, NOT a separate notification line per error class (the old wording contradicted invariant 23 and Phase 2c) - Make the Enable-CTA trust boundary explicit alongside the footer Enable button handler description: the user's settings.toml is the trust boundary, the CTA is unchanged from built-in flow and is not a per-launch authorization prompt. Pre-launch verification surface is a v1.1+ follow-up per invariant 31 - Clarify the cross-platform "absolute path" definition for `command` in both product.md invariant 1 and the tech.md validator description: Unix `/`-rooted, Windows drive-letter (`C:\` / `C:/`) or UNC (`\\server\share`, `//server/share`). Windows root-relative (`\path`, `/path` without a drive) is rejected — it depends on the current drive at exec time, which is the cwd-dependent resolution the rule exists to prevent
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This specs-only PR proposes user-configurable LSP servers via [[editor.language_servers]], including product invariants, implementation phases, persistence, placeholder substitution, and validation coverage.
Concerns
- The name validation rules are not sufficient for a value that is also used as a filesystem path component on all supported platforms.
- The
globset::GlobMatcherserde sketch needs an explicit default/placeholder strategy or a different deserialization shape to be compile-feasible. - The invalid-settings behavior should distinguish between blocking future custom-server launches and leaving already-running servers untouched.
Security
- The tech-spec redaction plan does not cover all values that product invariant 32 says must be redacted, especially substituted
commandvalues and raw descriptor values emitted during validation logging.
Verdict
Found: 0 critical, 3 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Resolves 4 findings from Oz's seventh review of warpdotdev#11944: - Z (Windows-reserved names + trailing dots): change `lsp_server_cache_dir` to hash `server_name` into the path component (same SHA-256-prefix shape as `workspace_hash`) instead of using the literal name. This decouples the cache-dir filesystem segment from `name`'s allowed character set and eliminates the whole class of platform-specific path-segment edge cases (Windows reserved device names, trailing-dot stripping, extreme length) without a per-platform validator. Updated invariant 1's `name` description, invariant 2's case-insensitive uniqueness rationale, invariant 5's `{{cache_dir}}` description, the Phase 4 persistence section's name-reservation rationale, and the Phase 1 `lsp_server_cache_dir` description - AA (invariant 23 wording): "no custom servers run" → "no NEW custom servers are launched until the file is fixed (already-running servers keep running per invariant 19)"; eliminates the apparent conflict with invariant 19's no-restart rule - BB (redaction completeness): add substituted `command` to the⚠️ list in the launch-time redaction policy (was missing); add a new parse/validate-time logging policy describing how `log::warn!` from `parse.rs`/`validate.rs` includes only the safe summary (constrained `name` + error variant name + structural detail), with raw user-typed values kept inside the error struct and never reaching the log line, since these calls run before the `LogRedactor` is injected from app/ - CC (Phase 5 struct example): replace the `LspServerDescriptor` and `LspFiletypePattern` Rust code blocks with prose describing the field shape and the serde/schemars decisions (including the `#[serde(skip, default = "placeholder_matcher")]` rationale). The previous code example was missing `#[serde(skip)]` and the `default` function reference, so it wouldn't have compiled — per the saved rule that tech.md uses pseudo-code, not real Rust
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec PR defines product and technical plans for user-configurable custom LSP servers, including settings schema, filetype matching, per-workspace enablement, runtime spawning, logging/redaction, and tests.
Concerns
- The reserved built-in server names are ambiguous because the Pyright binary/display name in the current code is
pyright-langserver, while the product spec repeatedly listspyright. - The persistence section specifies writes for custom enablement state but does not spell out startup/read hydration for
kind = 'Custom'rows, so cross-restart persistence could be missed despite being a product invariant. - The env placeholder and
envvalue design leaves literal secrets insettings.tomlas the only reliable option for users whose environment is populated by shell startup files; that should be resolved or explicitly removed from v1 expectations before implementation.
Security
- The spec acknowledges that dynamic env values may require plaintext values in
settings.toml; this is a security-relevant v1 design gap because LSP env vars commonly include API tokens.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
- DD: narrow the reserved-name list to the five binary display names only (rust-analyzer, gopls, pyright-langserver, typescript-language- server, clangd); drop the variant-name reservation. SQLite kind column already prevents on-disk collisions; cache dir is hashed. Fixes the pyright/pyright-langserver discrepancy by sourcing from LSPServerType::binary_name(). Normalizes "pyright" → "pyright- langserver" in colloquial server-list mentions - EE: document the startup/read path in Phase 4 — loader get_all_workspace_language_servers_by_workspace reads the kind column and partitions rows into language_servers and custom_language_servers - FF: drop "to inject secrets" framing from invariant 32; env-capture limitation already covered in the existing follow-up
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec PR proposes user-configurable [[editor.language_servers]] entries, covering product behavior, settings validation, placeholder substitution, custom-vs-built-in dispatch, persistence, schema generation, tests, and several security-sensitive trust boundaries.
Concerns
- The tech plan does not account for the existing text-document open path deriving
TextDocumentItem.language_idfrom the built-inLanguageIdenum, which would break the primary custom-language flow for filetypes that Warp does not already know.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
GG: extend the Phase 3 "identity-free runtime" paragraph to call out TextDocumentService::did_open's LanguageId::from_path derivation — the one remaining built-in-only assumption below the spawn line. Phase 3 plumbing: did_open takes language_id: String from the caller (matched descriptor for customs, LanguageId::from_path().lsp_language_identifier() for built-ins). Verified via JDTLS log: did_open currently silently fails for non-built-in filetypes (JDTLS hid the symptom via project indexing; other servers won't).
|
/oz-review |
|
This is your last |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds product and technical specs for user-configurable LSP servers via [[editor.language_servers]], covering user-facing behavior, validation, placeholder substitution, runtime dispatch, persistence, schema generation, and phased test coverage.
Concerns
- No blocking spec issues found. The specs cover the linked issue scope and document the main feasibility and security-sensitive tradeoffs, including user-supplied commands, PATH resolution, per-workspace enablement, logging/redaction, and follow-up hardening.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
…ec skill CONTRIBUTING.md's example uses GH<num>, but it links to the write-product-spec skill which specifies the canonical form is gh-<num> (lowercase, hyphenated). All existing specs in the repo use the older GH<num> form; this is the first to use the new convention. Also drop the stale "Linear ticket not yet created... replace with specs/APP-XXXX/" placeholder from product.md — no other spec in the repo carries that note, and the skill treats the GitHub-issue form as a permanent valid id, not a temporary placeholder.
Description
Spec PR for issue #8803 — proposes a new
[[editor.language_servers]]settings entry that lets users register custom LSP servers without modifying Warp's built-in language→server map.Per CONTRIBUTING.md, this is the spec-only PR for design review. Implementation continues on the existing code PR (#10700, branch
brad/8803-lsp-plugins) — this is the "rarer case" the contributing guide mentions where a large spec is broken out so implementation can be reviewed separately.What's in this PR
specs/GH8803/product.md— user-visible behavior. Numbered testable invariants covering the field set (name,command,args,filetypes,env,initialization_options; required vs optional), placeholder substitution ({{workspace_root}},{{workspace_slug}},{{cache_dir}},{{env_VAR}}), filetype matching (glob and literal basename), per-workspace enable persistence, custom-overrides-built-in semantics, and an Eclipse JDT-LS worked example demonstrating the field set is sufficient for non-trivial servers.specs/GH8803/tech.md— implementation plan grounded in the current codebase. Modules touched (crates/lsp/src/descriptor/,app/src/settings/language_servers.rs,app/src/code/lsp_dispatch.rs,app/src/ai/persisted_workspace.rs,app/src/code/footer.rs), the SQLite migration that adds akindcolumn toworkspace_language_serverfor persisting custom enable state, per-invariant verification mapping, and risks.Linked Issue
Closes #8803
ready-to-specorready-to-implement.Testing
Docs-only change.
I have manually tested my changes locally with— N/A, docs-only../script/runScreenshots / Videos
N/A — docs-only change. Visual evidence of the implementation behavior is attached on the code PR.
Agent Mode