chore: record profile scope and restore diagnostics

This commit is contained in:
Keisuke Hirata 2026-06-14 14:30:18 +09:00
parent 1abce888ae
commit db95492a4f
No known key found for this signature in database
7 changed files with 310 additions and 44 deletions

View File

@ -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"
}
]
}

View File

@ -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`.

View File

@ -28,4 +28,17 @@ LocalTicketBackend によって作成されました。
- `scope``delegation_scope` の merge/replace semantics が docs または test 名から読み取れる。
---
<!-- event: decision author: hare at: 2026-06-14T02:14:43Z -->
## 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 を作らない。
---

View File

@ -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 + `<workspace>/.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 <original workspace root>/.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`

View File

@ -4,4 +4,17 @@
LocalTicketBackend によって作成されました。
---
<!-- event: decision author: hare at: 2026-06-13T19:02:42Z -->
## 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 が必要なら後続として扱う。
---

View File

@ -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<String> {
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("<none>")
};
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<S: Scheme> {
http_client: reqwest::Client,
@ -510,6 +545,7 @@ impl<S: Scheme + Clone + 'static> LlmClient for HttpTransport<S> {
.await
{
Ok(response) => {
let response_headers = response_header_diagnostics(response.headers());
emit_transport_trace(
&request,
"transport_http_headers_received",
@ -517,6 +553,7 @@ impl<S: Scheme + Clone + 'static> LlmClient for HttpTransport<S> {
"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<S: Scheme + Clone + 'static> LlmClient for HttpTransport<S> {
);
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<S: Scheme + Clone + 'static> LlmClient for HttpTransport<S> {
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<Box<dyn Stream<Item = Result<Event, ClientError>> + 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!({

View File

@ -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.