fix: integrate develop and preserve invocation notification recovery
This commit is contained in:
@@ -0,0 +1,21 @@
|
||||
# Review finding severity contract mismatch during T-697
|
||||
|
||||
The independent Reviewer completed source/requirement analysis and validation,
|
||||
then its one `ReviewMergeRequest` submission was rejected with HTTP 422 because
|
||||
it used severity `medium`. The tool exposed a free-form string, while Backend
|
||||
validation accepts only `blocker`, `major`, `minor`, or `note`. No verdict was
|
||||
recorded by that failed call. The child preserved the source and reported the
|
||||
blocked submission rather than loop-retrying a mutation.
|
||||
|
||||
The parent inspected the committed review session and fresh MR authority,
|
||||
confirmed the same request/source/Ticket snapshot with no verdict, and submitted
|
||||
a fresh child turn to correct the input contract. The Reviewer independently
|
||||
selected supported severities and recorded `request_changes` successfully on
|
||||
the same MR. No alternate token/context, CLI/storage write, source movement or
|
||||
replacement MR was used to compensate for the rejection.
|
||||
|
||||
Suggested improvement: expose finding severity as the typed Backend enum in the
|
||||
LLM tool schema, and document allowed values in the Reviewer prompt. Rejecting
|
||||
an unadvertised enum after a costly review should not prevent durable findings
|
||||
from reaching the MR. This report is tool feedback, not a T-697 product
|
||||
constraint or a request to alter review judgments.
|
||||
@@ -0,0 +1,30 @@
|
||||
# Reviewer verdict のvalidation rejectionとone-call制約
|
||||
|
||||
## 確認した事実
|
||||
|
||||
T-703のMR `01a10b3b-87ec-7093-b0c4-a00a78095695`、source
|
||||
`3174142bd1314fe34c5de7b6a66654ff1c2a8af2` の独立ReviewerはRestoreとno-op
|
||||
attention checkの競合を見つけ、`ReviewMergeRequest`でrequest_changesを提出しようとした。
|
||||
severityに`medium`を指定したためHTTP 422となった。Backendの許容値は
|
||||
`blocker` / `major` / `minor` / `note` だった。
|
||||
|
||||
親の`ShowMergeRequest`再読でthreadはReviewRequested seq2までであり、verdictが
|
||||
永続化されていないことを確認した。Reviewerは「ReviewMergeRequestを正確に一度だけ
|
||||
呼ぶ」という指示に従い、validation rejection後も再提出を行わなかった。親のfollow-upでも
|
||||
同じ境界を報告したため、再提出を強制せずReviewerを停止した。
|
||||
|
||||
## 障壁
|
||||
|
||||
正しいBackend拒否を迂回すべきではない一方、mutationが行われていない入力validation失敗が
|
||||
一回限りの提出枠を消費する扱いだと、レビュー済みの指摘がMR authorityに残らずpendingになる。
|
||||
セッションでの報告をverdictや承認の代わりにはできない。
|
||||
|
||||
## 改善案(このTicketの実装対象ではない)
|
||||
|
||||
- Reviewer-facing tool schemaにseverityのBackend enumを明示し、不正値を入力時に防ぐ。
|
||||
- one-call指示について「一度のdurable verdict」と「validationで拒否された呼出」を区別できるか
|
||||
設計上判断する。無制限再提出や記録済みverdictの変更を認める提案ではない。
|
||||
- mutationなし/retry可否をtyped errorとして返せると、親とReviewerの双方が推測なしに扱える。
|
||||
|
||||
T-703では元のMR/source selectorを保持し、指摘を修正したcommitを通常pushした後、fresh
|
||||
exact-source reviewで修正内容とその証拠をMRに記録する。未記録のverdictを承認扱いしない。
|
||||
@@ -0,0 +1,18 @@
|
||||
# Reviewer SubWorkerがreview開始前に停止する
|
||||
|
||||
## 状況
|
||||
|
||||
T-698の公開済みMerge Requestに対して、`builtin:reviewer`を指定し、対象Ticket/Merge Request、writable delegated scope、command grantを明示してReviewer SubWorkerを3回起動した。
|
||||
|
||||
いずれも`review_requested` eventは正しいsource refとTicket revisionを記録したが、SubWorkerは直後に`Stopped`となった。session observationには最初のuser taskだけが存在し、assistant output、tool call、typed review verdictは記録されなかった。Merge Request threadにもapprove/request_changesは追加されなかった。
|
||||
|
||||
## 障壁
|
||||
|
||||
CoderはReviewer approvalを代行できず、親Workerには`ReviewMergeRequest` authorityがないため、実装・公開・検証が完了していてもfresh approvalを取得できない。同じ入力を再送するだけでは診断情報が増えず、review requestだけが複数残る。
|
||||
|
||||
## 改善案
|
||||
|
||||
- Reviewer起動がmodel run前に停止した場合、停止理由を親へ構造化して通知する。
|
||||
- `review_requested`を記録した後にReviewer sessionの開始に失敗した場合、request eventと失敗eventを関連付け、未解決reviewとして明示する。
|
||||
- profile/provider初期化、review worktree準備、capability bindingのどの段階で失敗したかを、credentialやhost pathを露出しないbounded diagnosticとして提供する。
|
||||
- 同一sourceへの再試行時は、先行Reviewerが一度も実行されていないことを識別できるようにする。
|
||||
@@ -0,0 +1,33 @@
|
||||
# SubWorker cwd attachment-root ambiguity during T-697
|
||||
|
||||
## Observed barrier
|
||||
|
||||
A delegated backend investigation was spawned with `cwd=crates/workspace-server`
|
||||
and explicit writable rules for both `crates/workspace-server` and
|
||||
`crates/server-api`, plus repository-root read access. The child reported that
|
||||
its available attachment exposed only the workspace-server crate; its observed
|
||||
file reads used `src/server.rs`/`src/lib.rs` and it could not access server-api.
|
||||
It also had no `ShowTicket` or parent-notification tool, despite the parent
|
||||
having typed Ticket authority. No implementation changes were made by that child.
|
||||
|
||||
The parent inspected the committed child session, released the idle child, and
|
||||
respawned at the repository root with the same delegated write boundaries and
|
||||
an explicit summary of the authoritative Ticket requirements. That child could
|
||||
then implement the cross-crate API boundary. No filesystem capability rejection
|
||||
was worked around through another write path.
|
||||
|
||||
## Suggested improvement
|
||||
|
||||
The advertised SubWorker `cwd` contract says that it changes only the tool
|
||||
working directory, not attachment roots. Show the effective attachment aliases,
|
||||
logical roots and readable/writable scopes explicitly to the child and parent,
|
||||
and test cross-crate reads/writes for a non-root cwd. If cwd intentionally
|
||||
narrows or rebases an attachment, document that instead of presenting it as
|
||||
only a command default. A denied/inaccessible sibling diagnostic should identify
|
||||
the effective logical attachment root without disclosing unauthorized paths.
|
||||
|
||||
Also expose which typed domain and parent-messaging tools will be available
|
||||
before spawning. Delegated implementation requires authoritative requirements;
|
||||
where Ticket reads are unavailable, the parent must deliberately provide the
|
||||
bounded contract rather than assume the child can recover it by ID. This report
|
||||
records tool ergonomics only; it is not a new T-697 product requirement.
|
||||
Reference in New Issue
Block a user