Repository navigation
[2] Keep work tied to its account and task - #1347
tautvydasLiekis wants to merge 33 commits into
Conversation
Kimchi Code Review
Summary🚫 Merge request too large to review 📏 Size: 31 file(s) changed, +3856 / −342 lines 🧠 Cognitive load: 5/5 (1 = trivial, 5 = extremely demanding) Account/repo scoping, opt-in semantic matching, a new segment model, and work-link corrections all converge in the core input handler with subtle async invalidation rules. Removing branch fallback rewrites continuation semantics, and large new integration test suites add volume on top. ✂️ Suggested split
👉 Please split this merge request into the smaller merge requests suggested above — the AI review will run automatically on each of them. What to expectKimchi will analyze the changes in this pull request and post:
The review typically completes within a few minutes. This comment will be updated once the review is ready. Interact with Kimchi
ConfigurationReviews are configured by your organization admin. Powered by Kimchi — AI-powered code review by CAST AI |
![kimchi-review[bot]](https://avatars.githubusercontent.com/in/3223788?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚫 Merge request too large to review
📏 Size: 31 file(s) changed, +3856 / −342 lines
🧠 Cognitive load: 5/5 (1 = trivial, 5 = extremely demanding)
Account/repo scoping, opt-in semantic matching, a new segment model, and work-link corrections all converge in the core input handler with subtle async invalidation rules. Removing branch fallback rewrites continuation semantics, and large new integration test suites add volume on top.
✂️ Suggested split
- Return user identity from API-key verification — Adds userId plus endpoint/abort/retry options; standalone foundation for account scoping.
src/api/organizations.tssrc/api/organizations.test.ts
- Capture account and repository scope for each work — Introduces scope.json capture and verification with its test mock.
src/extensions/work-attribution/scope.tssrc/extensions/__mocks__/work-scope.ts
- Replace branch fallback with pasted-plan and artifact matching — Continuation resolves saved, pasted, or attributed files under matching scope; drops branch inference.
src/extensions/work-attribution/continuation.tssrc/extensions/work-attribution/continuation.test.ts
- Add opt-in semantic task matching — Stores intent text and compares it via the selected model; extends context mock with modelRegistry.
src/extensions/work-attribution/semantic.tssrc/extensions/__mocks__/context.ts
- Attribute requests by segment and scope with correction links — Core orchestration: segments, scope enforcement, matching decisions, and workLink revisions.
src/extensions/work-attribution.tssrc/extensions/work-attribution.test.tssrc/extensions/work-attribution/links.tssrc/extensions/work-attribution/summary.tssrc/extensions/work-attribution/continuation.integration.test.ts
- Propagate attribution to plans, ferments, and child agents — Tools pin originating requests, children inherit segments; documents the new attribution model.
src/extensions/permissions/index.tssrc/extensions/permissions/index.test.tssrc/extensions/ferment/tools/lifecycle.tssrc/extensions/ferment/tools/lifecycle.test.tssrc/extensions/agents/manager/agent-runner.tssrc/extensions/agents/manager/agent-runner.test.tsdocs/work-attribution.md
👉 Please split this merge request into the smaller merge requests suggested above — the AI review will run automatically on each of them.
1da526c to
3a9082d
Compare
2ea1635 to
d2f7946
Compare
ca16415 to
51e7e57
Compare
51e7e57 to
6c8264e
Compare
6c8264e to
489bb3a
Compare
| } | ||
| } | ||
|
|
||
| /** Separate inference using the model selected at input time; never changes the chat model. */ |
There was a problem hiding this comment.
question
Any reason work matching runs on the session model instead of the classifier candidates? permissions solved the same shape of problem already, right? 🤔 with per-candidate deadlines, graceful degradation. same deal here: small bounded json verdict, cost-sensitive, runs per delivered message when matching is on.
we could move classifier-models.ts to a shared location and reuse it here and keeping the session model as fallback when kimchi-dev/* refs don't resolve?
| workMatchingEnabled() ? "unknown" : "session", | ||
| workMatchingEnabled() ? "matching-unresolved" : "matching-disabled", | ||
| ) | ||
| const captured = await captureWorkScope(cwd) |
There was a problem hiding this comment.
nit
captureWorkScope is on the message-send hot path, so every prompt and queued message pays for an uncached git subprocess, repeated config reads, and occasionally a 1s API verification.
suggestions:
- Cache
workRepositoryper cwd and load config once per capture. - When identity verification expires, reuse the last-good identity if the key and API URL are unchanged, refreshing in the background. Otherwise, return
unknownand refresh asynchronously.
2 participants
Each model request is now tied to the account, repository and user input it belongs to, so a PR is charged only for its own task. Before this, a fresh session on the same branch could inherit unrelated work, and saved work had no account boundary.
Linked issue
Depends on #1320. No public issue is linked.
What does this PR do?
/work <plan path>, or pasting the complete retained plan, continues the work that wrote it. An accepted continuation keeps the session's later inputs in that work, like/work <plan>, until an input names other work./workrefuses a plan saved for another account or repository and keeps the current work./work matching onlets the selected model compare saved task text in separate paid calls. It is off by default./work link <source-work-id> <segment-id>records a correction and/work unlink <link-id>revokes it. Original request records stay unchanged. Continuing a verified plan also confirms the input that wrote it./resources. Work attribution and the PR/MR status are one built-in extension, "Cost per PR", on by default. Disabling it there, or withkimchi resources disable extensions.cost-per-pr, takes effect at the next start; other extensions stop recording work at once.How a message picks its work
flowchart TD M["Delivered user message"] --> X{"Explicit work choice?"} X -->|Yes| K["Keep that choice"] X -->|No| P{"Names recorded work or a saved plan?"} P -->|Yes| V{"One verified owner, same account and repository,<br/>no named file owned by other work?"} V -->|Yes| A["Continue the plan's work"] V -->|No| U["Keep current work; mark input unknown"] P -->|No| E{"Model matching on?"} E -->|No| K E -->|Yes| C["Ask the selected model"] C -->|Same task or one earlier task| I["Keep or continue it; saved as an inference"] C -->|Clearly new task| N["Start separate work"] C -->|Uncertain| UEach request's work, input segment and scope are saved before it is sent. New files sit next to
work.jsonunder~/.config/kimchi/harness/work/<workId>/:scope.json(original account and repository),plans/(plan snapshots) andintent.json(task text, only when model matching is on). This PR prices nothing; #1348 does.Evidence
On
92ab8abcd:submit_plan; a fresh Flash session that sentImplement .kimchi/plans/notes.md, …continued the plan's work before its first request, and the planning input was confirmed through a work link; a restart started separate work. The binary built without this PR kept the two sessions apart.pnpm checkpasses and the work attribution tests pass (392). CI on this head: build,pr-checks,master-checks, TUI e2e in 4 shards, ACP e2e and MCP e2e pass.Earlier, on
3983c2bf3after the final review fixes: CI passed (build,pr-checks,master-checks, TUI e2e in 4 shards, ACP e2e and MCP e2e); on the stack top95614e7c6,pnpm checkand the full unit suite passed except 5 tests that need tools this machine lacks; a new test covered a saved plan named together with a file outside every Git worktree; and the lab passed 73/73 default scenarios with a scripted model.Limits
/worknames the limit that stopped it.Checklist
pnpm run test) — all but 5 that need tools this machine lacks; see Evidencepnpm run check)🤖 Generated with Claude Code