diff --git a/.yoi/tickets/00001KTZY8HK2/artifacts/relations.json b/.yoi/tickets/00001KTZY8HK2/artifacts/relations.json new file mode 100644 index 00000000..5d3664d6 --- /dev/null +++ b/.yoi/tickets/00001KTZY8HK2/artifacts/relations.json @@ -0,0 +1,13 @@ +{ + "version": 1, + "relations": [ + { + "ticket_id": "00001KTZY8HK2", + "kind": "related", + "target": "00001KV11DHGZ", + "note": "scope replacement concern is superseded by moving concrete scope to launch policy", + "author": "yoi ticket", + "at": "2026-06-14T02:14:43Z" + } + ] +} diff --git a/.yoi/tickets/00001KTZY8HK2/item.md b/.yoi/tickets/00001KTZY8HK2/item.md index efea4b20..4f597764 100644 --- a/.yoi/tickets/00001KTZY8HK2/item.md +++ b/.yoi/tickets/00001KTZY8HK2/item.md @@ -1,15 +1,78 @@ --- -title: 'Profile extend の authority field を明示的に置換できるようにする' -state: 'planning' +title: 'Profile extend API を廃止して import + Lua 代入に寄せる' +state: 'ready' created_at: '2026-06-13T07:31:09Z' -updated_at: '2026-06-13T07:31:25Z' +updated_at: '2026-06-14T02:14:43Z' assignee: null +readiness: 'implementation_ready' +risk_flags: ['profiles', 'lua-api', 'builtin-resources', 'migration'] --- -## 背景 +## Background -LocalTicketBackend によって作成されました。 +`yoi.profile.extend(base, overrides)` は base Profile と override object を Rust 側で deep merge する convenience API だった。しかし deep merge semantics が隠れるため、object field を「置換したい」のか「部分 merge したい」のかが Profile authoring 上で読み取りづらい。 -## 受け入れ条件 +今回の問題では、以下のような Profile が scope を置換するつもりで object を渡すと、`builtin:default` 側の inherited value と deep merge され、workspace write が残り得た。 -- 未定 +```lua +return yoi.profile.extend("builtin:default", { + scope = yoi.scope.workspace_read(), +}) +``` + +一方で Lua には通常の table 操作があるため、Profile 継承と field replacement は次の形で明示できる。 + +```lua +local p = yoi.profile.import("builtin:default") +p.worker.instruction = "$yoi/role/orchestrator" +p.feature.task.enabled = false +p.feature.ticket = { enabled = true, access = "lifecycle" } +return p +``` + +この形なら、top-level/nested field の代入は ordinary Lua assignment であり、merge/replace の意図が読みやすい。Profile scope については別 Ticket `00001KV11DHGZ` で concrete runtime authority を launch policy に移すため、`extend()` に replacement API を足す必要性はさらに下がった。 + +## Requirements + +- `yoi.profile.extend` を廃止する。 + - builtin Profile resources は `extend()` を使わず、`yoi.profile.import(...)` + explicit Lua assignment に移行する。 + - Profile authoring convention は ordinary Lua table mutation / assignment を基本とする。 +- `yoi.profile.import` は残す。 + - base Profile を取得して手元で変更し、`return p` する構造を公式パターンにする。 +- `extend()` の deep merge semantics に依存する builtin tests/resources を更新する。 +- `extend()` を public API として残す場合は deprecated diagnostic を出すか、削除して明確に fail させる。 + - どちらにするかは実装時に決めてよいが、builtin resources は使わない。 + - 不必要な compatibility alias は作らない。 +- Docs/tests/comments から「scope replacement API を足す」方向の記述を削除する。 + - scope/delegation_scope の concrete authority は `00001KV11DHGZ` の launch policy 側で扱う。 + - scope 以外でも object replacement が必要な場合は ordinary Lua assignment を使う。 +- Profile merge helper がどうしても必要になった場合は、`extend()` のような曖昧な名前ではなく、`deep_merge` / `shallow_merge` など semantics が明示された別 API として後続 Ticket で検討する。 + +## Acceptance criteria + +- Repository builtin Profile resources no longer call `yoi.profile.extend`. +- The Profile Lua API supports the official inheritance pattern: + +```lua +local p = yoi.profile.import("builtin:default") +-- mutate p explicitly +return p +``` + +- Tests cover that `import()` + assignment replaces object fields without deep merge surprise. +- Existing role Profiles still resolve to the same intended non-scope behavior: worker instructions, feature enablement, model/defaults, compaction, etc. +- If `extend()` is removed, calling it fails with a clear Lua/profile error. If deprecated instead, it emits/records a clear deprecation diagnostic and builtin resources do not use it. +- The old requirement to add `yoi.scope.replace(...)` / `replace = true` for scope replacement is removed or explicitly superseded. +- Validation: focused profile resolution tests and `cargo build -p yoi`. Run `nix build .#yoi` only if resource packaging or lockfile/package changes require it. + +## Non-goals + +- Moving concrete scope/delegation_scope out of Profiles; tracked by `00001KV11DHGZ`. +- Designing a general-purpose Lua table utility library. +- Preserving `extend()` compatibility indefinitely. +- Plugin/MCP permission design. + +## Related work + +- Move concrete scope to launch policy: `00001KV11DHGZ` +- Original scope replacement concern is superseded by this Ticket plus `00001KV11DHGZ`. diff --git a/.yoi/tickets/00001KTZY8HK2/thread.md b/.yoi/tickets/00001KTZY8HK2/thread.md index d9c73c1b..89101d84 100644 --- a/.yoi/tickets/00001KTZY8HK2/thread.md +++ b/.yoi/tickets/00001KTZY8HK2/thread.md @@ -28,4 +28,17 @@ LocalTicketBackend によって作成されました。 - `scope` と `delegation_scope` の merge/replace semantics が docs または test 名から読み取れる。 +--- + + + +## Decision + +決定: +- `yoi.profile.extend` に replace/clear API を足す方向ではなく、`extend` 自体を廃止する。 +- Profile 継承は `local p = yoi.profile.import("builtin:default"); p.field = ...; return p` の ordinary Lua assignment に寄せる。 +- deep merge helper が必要なら、semantics が明示された別 API として後続で検討する。 +- scope/delegation_scope の問題は `00001KV11DHGZ` で launch policy に移すため、この Ticket では scope replacement API を作らない。 + + --- diff --git a/.yoi/tickets/00001KV11DHGZ/item.md b/.yoi/tickets/00001KV11DHGZ/item.md index 761f8846..7fc8ebdf 100644 --- a/.yoi/tickets/00001KV11DHGZ/item.md +++ b/.yoi/tickets/00001KV11DHGZ/item.md @@ -1,27 +1,49 @@ --- -title: 'Orchestrator delegation scope を root read + worktree write に狭める' -state: 'planning' +title: 'Profile から concrete scope を外して launch policy で付与する' +state: 'ready' created_at: '2026-06-13T17:45:32Z' -updated_at: '2026-06-13T17:46:37Z' +updated_at: '2026-06-13T19:02:42Z' assignee: null -readiness: 'requirements_sync_needed' -risk_flags: ['scope', 'delegation-scope', 'orchestrator', 'profile-merge', 'spawnpod'] +readiness: 'implementation_ready' +risk_flags: ['scope', 'delegation-scope', 'profiles', 'launch-policy', 'orchestrator', 'spawnpod', 'restore'] --- ## Background -Orchestrator の direct scope は root workspace read-only であるべきだが、child Coder/Reviewer には implementation worktree への write と original workspace への read を委譲できる必要がある。 +Profile は reusable behavior / model / prompt / feature policy の単位であるべきだが、現在は concrete filesystem authority である `scope` / `delegation_scope` も持っている。これにより、runtime launch policy と Profile authority が衝突している。 -現在の暫定 Orchestrator profile は `resources/profiles/orchestrator.lua` で以下になっている。 +実際に起きた問題: -```lua -scope = "workspace_read" -delegation_scope = "workspace_write" +- `builtin:default` の workspace write scope が role profile 継承に混ざり、Orchestrator direct scope が workspace write を要求した。 +- 暫定対応として `resources/profiles/orchestrator.lua` に `scope = "workspace_read"` / `delegation_scope = "workspace_write"` を置いたが、これは scalar replacement に依存した非自明な workaround である。 +- Orchestrator の本来の authority は mixed shape である。 + - direct scope: original workspace root read + - delegation scope: original workspace root read + `/.worktree` write +- Profile の Lua API / merge semantics でこの mixed authority を表現しようとすると、`yoi.profile.extend()` の deep merge や authority-bearing field replacement の問題に引きずられる。 +- Restore では metadata snapshot を正本として尊重すべきであり、current Profile/default manifest 由来の scope で上書きしてはいけない。 + +根本方針: + +```text +Profile = reusable behavior / prompt / model / feature policy +Launch policy = concrete runtime authority / workspace root / cwd +metadata snapshot = restore 時の effective authority 正本 ``` -これは fresh launch の delegation 不足を避けるが、delegation が広すぎる。概念的には child に original workspace root への write まで委譲可能になっている。 +この Ticket は、Orchestrator delegation を狭めるだけでなく、built-in/Profile から concrete scope を外し、起動経路ごとの launch policy が effective `scope` / `delegation_scope` を確定する形へ整理する。 -手動 restore 復旧時に delegation を `.worktree` write のみに狭めたところ、Reviewer 起動で root workspace read を再委譲できず失敗した。正しい effective shape は以下。 +## Requirements + +- Reusable Profiles から concrete filesystem authority を外す。 + - Builtin role Profiles (`builtin:companion`, `builtin:orchestrator`, `builtin:coder`, `builtin:reviewer` など) は `scope` / `delegation_scope` を role behavior の正本として持たない。 + - `resources/profiles/orchestrator.lua` の `scope = "workspace_read"` / `delegation_scope = "workspace_write"` workaround を解消する。 + - Profile は model / worker instruction / feature policy / compaction 等の reusable behavior を担う。 +- Launch surface が concrete authority を構築する。 + - normal TUI / Companion launch は workspace write など、その起動経路の policy に基づいて direct scope を付与する。 + - Ticket Orchestrator launch は Orchestrator role policy に基づいて scope/delegation を付与する。 + - SpawnPod / role child launch は explicit delegated child scope を child direct scope として渡す。 + - Child delegation scope は明示的に必要な場合のみ付与し、profile inheritance から暗黙に継承しない。 +- Orchestrator launch policy を以下にする。 ```text direct scope: @@ -32,28 +54,22 @@ delegation_scope: write /.worktree ``` -現状の scalar profile intent では、この mixed delegation scope を簡潔に表現できない。object/table form は `profile.extend()` の deep merge と scope merge semantics により inherited value の置換が難しいため、既存 Ticket `00001KTZY8HK2` の profile replace/clear semantics と関連する。 - -## Requirements - -- Orchestrator fresh launch の effective scope を以下にする。 - - direct `scope`: original workspace root recursive read - - `delegation_scope`: original workspace root recursive read + original workspace `.worktree` recursive write - Reviewer/Coder child launch が必要とする root read と implementation worktree write を Orchestrator が再委譲できること。 - Orchestrator が child に original workspace root write を委譲できないこと。 -- Profile 継承は維持する。 - - `builtin:orchestrator` は `builtin:default` から reusable defaults を継承してよい。 - - inherited authority-bearing scope fields を意図せず加算しないこと。 -- 実装方法は `00001KTZY8HK2` の profile replacement/clear semantics と整合させる。 - - この Ticket で replacement API まで実装するか、Orchestrator launch context 側で role-specific delegation scope を構築するかは設計で決める。 - - いずれの場合も scalar string workaround への依存を増やさない。 -- Restore path と fresh launch path の effective scope が意図せず乖離しないようにする。 - - metadata snapshot は restore 時に尊重する。 - - 新規生成される Orchestrator metadata snapshot には narrowed delegation scope が保存される。 +- Restore path は metadata snapshot を尊重する。 + - metadata snapshot がある場合、current profile/default/launch policy で scope/delegation_scope を上書きしない。 + - 新規 launch で保存される metadata snapshot には launch policy 適用後の effective scope/delegation_scope が入る。 +- Existing Profile scope support の扱いを明確にする。 + - この Ticket で Profile schema から完全削除するか、built-in role Profiles では禁止/無視しつつ user Profile support を deprecated として残すかは実装時に決めてよい。 + - ただし built-in role launch の concrete authority は Profile scope に依存しないこと。 +- `00001KTZY8HK2` の Lua/profile replacement API は、この Ticket の前提にしない。 + - scope 問題は Profile replacement API で解くのではなく、concrete authority を launch policy へ移すことで解く。 + - scope 以外の field replacement が必要なら `00001KTZY8HK2` を後続の別問題として扱う。 ## Acceptance criteria -- Fresh Orchestrator launch の resolved/effective manifest が以下を満たす test がある。 +- Builtin Orchestrator Profile の resolved reusable behavior から broad `delegation_scope = "workspace_write"` 依存がなくなる。 +- Fresh Orchestrator launch の effective manifest が以下を満たす test がある。 - direct scope allows read on original workspace root - direct scope does not allow write on original workspace root - delegation scope allows read on original workspace root @@ -61,18 +77,27 @@ delegation_scope: - delegation scope does not allow write on original workspace root outside `.worktree` - Reviewer/Coder launch validation can be satisfied by the narrowed Orchestrator delegation scope. - Scope allocator does not conflict with the Companion/top-level `yoi` Pod's workspace write allocation. -- `resources/profiles/orchestrator.lua` no longer needs to express delegation as broad `"workspace_write"` unless a separate explicit override intentionally narrows it later in launch resolution. -- Tests cover both profile resolution / launch context and child delegation validation where practical. -- Validation: focused scope/profile/client tests, `cargo build -p yoi`, and `nix build .#yoi`. +- Normal Companion/TUI launch still receives the expected workspace write direct scope from launch policy, not from reusable Profile authority. +- SpawnPod child config still replaces inherited/profile scope with the explicitly delegated child scope. +- Restore from metadata snapshot preserves saved scope/delegation_scope and does not reapply current Profile/launch default authority over the snapshot. +- Tests cover at least: + - Profile resolution no longer leaking default workspace write into Orchestrator authority + - launch policy authority for Orchestrator + - launch policy authority for normal top-level/Companion launch where practical + - restore snapshot preservation + - child delegation validation for root read + worktree write +- Validation: focused scope/profile/client/pod tests and `cargo build -p yoi`. Run `nix build .#yoi` only if Cargo.lock, packaging, or resource inclusion changes require it. ## Out of scope - General Plugin/MCP permission design. -- Full replacement of profile scope semantics beyond what is needed here, unless this Ticket is intentionally merged with `00001KTZY8HK2`. - OS-level sandboxing of child processes. +- Designing a full replacement/clear Lua API for every Profile field. +- Full removal of user Profile `scope` compatibility unless the implementation chooses that as the cleanest route and updates tests/docs accordingly. ## Related work -- Profile replacement/clear semantics: `00001KTZY8HK2` -- Restore should preserve metadata manifest snapshot: current local fix in `crates/pod/src/pod.rs` +- Profile replacement/clear semantics follow-up: `00001KTZY8HK2` +- Restore should preserve metadata manifest snapshot: `9be3f132` - Orchestrator role profile: `resources/profiles/orchestrator.lua` +- SpawnPod child scope/delegation path: `crates/pod/src/spawn/tool.rs` diff --git a/.yoi/tickets/00001KV11DHGZ/thread.md b/.yoi/tickets/00001KV11DHGZ/thread.md index c21db9da..c5dd0ea3 100644 --- a/.yoi/tickets/00001KV11DHGZ/thread.md +++ b/.yoi/tickets/00001KV11DHGZ/thread.md @@ -4,4 +4,17 @@ LocalTicketBackend によって作成されました。 +--- + + + +## Decision + +決定: +- 旧方針の「Orchestrator delegation scope だけを狭める」ではなく、1 Ticket にまとめて「Profile から concrete scope を外し、launch policy が runtime authority を付与する」方針に広げる。 +- Profile は reusable behavior / prompt / model / feature policy を持つ層とし、filesystem `scope` / `delegation_scope` は起動経路が concrete workspace/cwd とともに決める。 +- Orchestrator の desired effective authority は launch policy で `direct read workspace` + `delegation read workspace, write workspace/.worktree` として構築する。 +- Lua/profile replacement API (`00001KTZY8HK2`) はこの scope 問題の前提にしない。scope 以外の replacement が必要なら後続として扱う。 + + --- diff --git a/crates/llm-worker/src/llm_client/transport.rs b/crates/llm-worker/src/llm_client/transport.rs index ae0b7cc2..9954e214 100644 --- a/crates/llm-worker/src/llm_client/transport.rs +++ b/crates/llm-worker/src/llm_client/transport.rs @@ -12,7 +12,8 @@ use async_trait::async_trait; use eventsource_stream::Eventsource; use futures::{Stream, StreamExt, TryStreamExt}; use reqwest::header::{ - ACCEPT, CONTENT_ENCODING, CONTENT_TYPE, HeaderMap, HeaderName, HeaderValue, RETRY_AFTER, + ACCEPT, CONTENT_ENCODING, CONTENT_LENGTH, CONTENT_TYPE, HeaderMap, HeaderName, HeaderValue, + RETRY_AFTER, TRANSFER_ENCODING, }; use serde_json::{Map, Value, json}; @@ -62,6 +63,40 @@ impl ResolvedAuth { } } +fn header_value_for_diagnostics(headers: &HeaderMap, name: &'static HeaderName) -> Option { + headers + .get(name) + .and_then(|value| value.to_str().ok()) + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(ToOwned::to_owned) +} + +fn response_header_diagnostics(headers: &HeaderMap) -> serde_json::Value { + serde_json::json!({ + "content_type": header_value_for_diagnostics(headers, &CONTENT_TYPE), + "content_encoding": header_value_for_diagnostics(headers, &CONTENT_ENCODING), + "transfer_encoding": header_value_for_diagnostics(headers, &TRANSFER_ENCODING), + "content_length": header_value_for_diagnostics(headers, &CONTENT_LENGTH), + }) +} + +fn sse_error_context(status: u16, headers: &serde_json::Value, source: &str) -> String { + let field = |name: &str| { + headers + .get(name) + .and_then(serde_json::Value::as_str) + .unwrap_or("") + }; + format!( + "SSE stream parse failed after HTTP {status}: {source}; content-type={}, content-encoding={}, transfer-encoding={}, content-length={}", + field("content_type"), + field("content_encoding"), + field("transfer_encoding"), + field("content_length") + ) +} + /// scheme 共通の HTTP 通信層。 pub struct HttpTransport { http_client: reqwest::Client, @@ -510,6 +545,7 @@ impl LlmClient for HttpTransport { .await { Ok(response) => { + let response_headers = response_header_diagnostics(response.headers()); emit_transport_trace( &request, "transport_http_headers_received", @@ -517,6 +553,7 @@ impl LlmClient for HttpTransport { "elapsed_ms": send_started.elapsed().as_millis() as u64, "status": response.status().as_u16(), "success": response.status().is_success(), + "headers": response_headers, }), ); response @@ -563,6 +600,9 @@ impl LlmClient for HttpTransport { ); let scheme = self.scheme.clone(); + let status = response.status().as_u16(); + let response_headers = response_header_diagnostics(response.headers()); + let transport_trace = request.transport_trace.clone(); let byte_stream = response.bytes_stream().map_err(std::io::Error::other); let event_stream = byte_stream.eventsource(); @@ -575,7 +615,21 @@ impl LlmClient for HttpTransport { Ok(events) => Ok(events), Err(e) => Err(e), }, - Err(e) => Err(ClientError::Sse(e.to_string())), + Err(e) => { + let source = e.to_string(); + let message = sse_error_context(status, &response_headers, &source); + if let Some(trace) = &transport_trace { + trace.emit( + "transport_sse_parse_error", + json!({ + "status": status, + "headers": response_headers.clone(), + "error": source, + }), + ); + } + Err(ClientError::Sse(message)) + } }) .map(|res| { let s: Pin> + Send>> = match res { @@ -675,6 +729,39 @@ mod tests { ) } + #[test] + fn sse_error_context_includes_response_headers() { + let headers = json!({ + "content_type": "application/octet-stream", + "content_encoding": "gzip", + "transfer_encoding": "chunked", + "content_length": "123", + }); + + let message = sse_error_context(200, &headers, "stream did not contain valid UTF-8"); + + assert!(message.contains("HTTP 200")); + assert!(message.contains("stream did not contain valid UTF-8")); + assert!(message.contains("content-type=application/octet-stream")); + assert!(message.contains("content-encoding=gzip")); + assert!(message.contains("transfer-encoding=chunked")); + assert!(message.contains("content-length=123")); + } + + #[test] + fn response_header_diagnostics_redacts_to_safe_header_subset() { + let mut headers = HeaderMap::new(); + headers.insert(CONTENT_TYPE, HeaderValue::from_static("text/event-stream")); + headers.insert(CONTENT_ENCODING, HeaderValue::from_static("identity")); + headers.insert("authorization", HeaderValue::from_static("Bearer secret")); + + let diagnostics = response_header_diagnostics(&headers); + + assert_eq!(diagnostics["content_type"], "text/event-stream"); + assert_eq!(diagnostics["content_encoding"], "identity"); + assert!(diagnostics.get("authorization").is_none()); + } + #[test] fn request_body_shape_counts_reasoning_encrypted_content() { let payload = request_body_shape_payload(&json!({ diff --git a/docs/report/2026-06-14-session-jsonl-partial-write-restore-failure.md b/docs/report/2026-06-14-session-jsonl-partial-write-restore-failure.md new file mode 100644 index 00000000..0146a4e5 --- /dev/null +++ b/docs/report/2026-06-14-session-jsonl-partial-write-restore-failure.md @@ -0,0 +1,52 @@ +# Session JSONL partial write caused restore failure + +## Summary + +`yoi-orchestrator` restore failed with a misleading Pod I/O error: + +```text +failed to restore pod yoi-orchestrator +I/O error: stream did not contain valid UTF-8 +``` + +The error was not caused by Panel, SendToPod, Unix socket IPC, or the LLM SSE stream. The active session JSONL file contained a corrupted line with invalid UTF-8 and invalid JSONL structure, so restore failed while reading persisted history before any model run could start. + +## Observed symptoms + +- Sending to `yoi-orchestrator` failed from multiple client paths with the same notice error. +- Restore also failed with the same UTF-8 I/O error. +- Enabling trace did not produce new transport trace entries for this failure, because restore failed before LLM transport execution. +- Raw socket checks showed the Pod socket/snapshot path itself was not the original source of the invalid UTF-8 error. + +## Corruption found + +The corrupted file was the active `yoi-orchestrator` session JSONL under `~/.yoi/sessions/...`. + +The corrupted record was a Bash `tool_result` containing truncated-output text similar to: + +```text +[showing last 80 of 311 lines ... +``` + +The line contained an incomplete UTF-8 sequence in the truncated-output header, likely the first two bytes of an em dash (`e2 80`, missing the final byte), followed immediately by the next JSON record on the same line. This made the file both invalid UTF-8 and invalid JSONL. + +The corrupted `tool_result` was manually replaced with a synthetic repair record preserving the original `call_id`, and the following assistant record was split back onto its own line. The repaired file was validated as UTF-8 and JSONL. + +## Impact + +- A single corrupted append in session history can make a Pod unrestorable. +- The current error surfaced as a generic I/O/UTF-8 error and did not name the session file, line, byte offset, or restore phase. +- Because the same phrase can also appear in transport/SSE decoding failures, the diagnosis path was initially confusing. + +## Lessons / improvement ideas + +- Session restore errors should include file path, line number, byte offset if available, and phase (`history restore`, `trace read`, `transport stream`, etc.). +- JSONL append should avoid leaving a partially written record followed by later records on the same line. Consider atomic record append safeguards, newline recovery, or corruption quarantine. +- Restore could offer a bounded repair/quarantine mode for malformed trailing or individual records, especially tool results. +- Bash truncated-output serialization should avoid multi-byte punctuation in structural prefixes or ensure all persisted records are validated before commit. +- Transport SSE UTF-8 failures and session-history UTF-8 failures should have clearly distinct error wording. + +## Related fixes made during investigation + +- Added safer SSE parse diagnostics in `llm-worker` so future provider-stream failures include HTTP status and selected safe response headers. +- Enabled local trace via `.yoi/override.local.toml` and manually set `record_event_trace = true` in the `yoi-orchestrator` metadata snapshot for future restores.