3 Commits

Include renames

Author	SHA1	Message	Date
4 people	686957e743	MUL-7271: test(server): cut slow Go test time further (follow-up to #8295) (#8318) ... * test(server): reduce slow unit test waits Co-authored-by: multica-agent <[email protected]> * test(server): cut slow Go unit test time further Follow-up to the first pass on MUL-7271. Test-only changes plus a few package-private seams whose production values are unchanged. - Re-executed race-built test binaries no longer pay the race runtime's 1s atexit sleep (GORACE=atexit_sleep_ms=0 for child processes only). - pkg/agent: event-driven or shorter test-only bounds for handshake, drain and idle graces; redundant timeout variants merged. - daemon/execenv: git fixtures copied from a template repo built once; tests that re-ran the same worktree lifecycle folded into their supersets; ForkLock-guarded fake-CLI writes so parallel tests cannot hit ETXTBSY. - handler/service: cap and paging tests shrink their caps instead of seeding thousands of rows; lock waits probe pg_locks instead of sleeping; one shared pgxpool per service test binary; cheaper cron and RSA fixtures. - wecom: the phishing-link sweep reuses one parser and derives CRLF from LF with a sampled cross-check; fake sockets acknowledge instead of waiting out the ack timeout. - migrate: the migration corpus is parsed once; isolated DB tests run in parallel. Co-authored-by: multica-agent <[email protected]> * test(server): remove transport timeout unit tests Co-authored-by: multica-agent <[email protected]> * test(server): restore two non-timeout tests and make a bound load-bearing Follow-up to the review of #8318. All test-only; no production code changes. - daemon: restore the three read-liveness cases that the transport-timeout sweep took with it. They assert that a ping, a pong and an application frame each EXTEND the read deadline, so a healthy wakeup socket survives without traffic of our own — the deadline never fires on the happy path, which is why they are not transport-timeout tests. Verified they fail (all three) when the extension calls are removed from wakeup.go, and pass in 0.57s together. The pure "times out without peer traffic" case stays deleted. - pkg/agent: restore the parent-DEADLINE half of the opaque-env redaction pair. codex.go treats a parent deadline and a parent cancellation as one ending; only the Canceled arm was left covered. The fake now writes the secret to stderr before reading the handshake, so the check no longer races our own initialize write, and the budget is 2s against a measured 430-465ms launch. Verified it fails when the !contextEnded guard is removed. - pkg/agent: TestRunCollectQuietDoesNotWaitWhenTheAnswerIsIn had its bound widened to a literal 5s, which the shrunk 750ms drain grace can never reach — the test could no longer fail. It now raises the drain grace to 10s for its own duration and asserts under 3s: healthy runs take 0.19-0.44s, and a broken short-circuit takes 10.19s and fails. - wecom: gofmt the import block in relay_settle_budget_test.go. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]> Co-authored-by: J <[email protected]>	2026-09-14 14:33:50 +08:00
3 people	61ea48fd2e	MUL-6881 fix(daemon): keep one worktree branch per conversation in local worktree mode (#7815) ... * fix(daemon): keep one worktree branch per conversation in local worktree mode Every comment on a local_directory issue started a new task, and worktree mode gave each one a branch of its own forked from the user's HEAD. Turn two therefore stood in a tree that did not contain turn one's work, with that work stranded on another branch and nothing saying so. Tasks that share a conversation — the turns of one issue, one chat session — now share a branch, agent/<agent>/<issue>, and a follow-up checks it out at its tip instead of forking from HEAD. Env roots stay per task, so sibling tasks still run concurrently; when one already holds the conversation's branch, the other forks from its tip onto agent/<agent>/<issue>-<task> rather than falling back to HEAD. Replaying the user's uncommitted tree onto a continued branch cannot use the turn-one path: that merge takes the user's HEAD as its base, so it conflicts exactly when the agent edited the lines the user had left uncommitted. Each turn now records the snapshot it saw at refs/multica/local-state/<branch> and the next one replays only the difference — a cherry-pick whose parent is that snapshot. A genuine conflict leaves the branch as it stands and logs which files did not come across, rather than stranding the conversation. Teardown follows the same rule everywhere: a branch this prepare did not create belongs to the turns before it, so Finalize's read-only cleanup, Discard and the prepare rollbacks only ever drop a branch they created themselves. Co-authored-by: multica-agent <[email protected]> * fix(daemon): prove branch ownership and hand replay conflicts to the agent Review of the per-conversation worktree branch found four ways it could put the wrong tree in front of the agent. All four came from the same two gaps: the user's snapshot did not describe their whole directory, and a branch was recognised by its name. The snapshot is now one commit covering tracked edits AND untracked files, built in a private index inside the task's env root. It answers the question a continued branch actually has to ask — what has the user changed since the state I already carry — for every file, so a once-untracked file the agent edited is no longer reverted to the user's older copy each turn, and their later deletion of one carries. Redirecting GIT_INDEX_FILE also removes the .git/index.lock race the previous capture had to retry around (#7434). A conflict between the user's newer edits and the branch's own work is now the agent's to resolve, per the product decision on the issue: the worktree is handed over mid-merge, the turn's prompt names the files and says which side is which, and Finalize refuses to deliver — preserving the worktree — while anything is still unmerged. The snapshot only advances once the branch really carries it, so an unresolved edit is offered again next turn instead of being recorded as synced. Before this, the conflict was reset away and the user's change was lost from every later turn. Continuing a branch is decided by an ownership record — workspace, agent and conversation ids, recorded in the snapshot commit at refs/multica/local-state/<branch> — never by the branch name. A same-named branch the user created is left untouched and the task takes agent/<agent>/<issue>-<fingerprint>, which its own follow-ups then continue. The same check keeps two workspaces that mint one issue identifier apart. Those refs are now GC'd: every prepare drops the record of a branch that no longer exists, and the docs say where they live and how to clear them. Co-authored-by: multica-agent <[email protected]> * fix(daemon): tie a task branch's record to the branch, and quote untrusted paths Ownership proved that Multica once wrote a branch of this name, which stayed true after the user deleted it and created their own under the same name. No prepare runs in that gap, so the orphan sweep never sees the branch missing and the next turn continued straight into the user's unrelated work. The record now also pins the branch tip it was written at, as the second parent of the record commit, and a branch is only continued while that commit is still an ancestor of its tip. Deleted-and-recreated, force-moved and rebased branches all fail that test; committing on top of a delivered branch — the normal case — passes it and picks the user's commit up. Finalize re-records afterwards, because a checkpoint written at prepare time sits on the branch's base, which is exactly where a recreated branch would sit. Conflicted paths reaching the turn's prompt are quoted rather than wrapped in a code span. They come from the user's repository and git allows newlines, backticks and quotes in a filename, so a raw one could close its list item and continue as an instruction line of its own; the same quoting is applied to the Finalize error. The list is bounded at 50 names with the remainder counted. Also drops a duplicated pair of bullets in the Chinese docs and corrects two file comments that still described the stash-and-copy replay. Co-authored-by: multica-agent <[email protected]> * fix(daemon): make the branch record part of delivery, and bound the conflict list by bytes The record could still authorise a branch that was not ours. Finalize wrote it after removing the worktree and treated a failure as non-fatal, so a failed update-ref left the ref pointing at the record Prepare had written — whose checkpoint is the branch's base, which is exactly where a branch the user deletes and recreates from HEAD sits. The ancestor check then passed on their work. Two changes close it. A checkpoint is only written when it is a commit the branch could not sit at without this conversation's work: Finalize records the tip it actually delivered, and Prepare records early only once the branch has moved off the user's HEAD — which is the baseline commit, or a continued branch's own tip. And the record is now part of delivering: Finalize writes it BEFORE removing the worktree and fails the task if it cannot, so the worktree is still there to preserve, exactly as for a commit that could not be made. The checkpoint is the commit the caller delivered rather than a re-read of the branch ref, so a branch moved in that window fails the next ancestor check instead of being recorded as current. A sibling racing the very first turn of a conversation on a clean tree now finds no record yet and starts its own line of work rather than forking from a branch it cannot prove is ours. That is the price of the rule and it is the safe side. The conflict list in the prompt is bounded by rendered bytes rather than by entries — a git path is as long as the filesystem allows, so 50 entries bounded nothing — with the remainder counted and a single oversized path unable to overrun the budget either. Co-authored-by: multica-agent <[email protected]> * fix(daemon): give every task branch a commit of its own, and verify what a run delivers The checkpoint rule — record only a commit the branch could not sit at without this conversation's work — held in Prepare and not in Finalize. Finalize took whatever the worktree's HEAD was, so a run that reset its worktree back to the user's own HEAD had that plain user commit recorded as the branch's checkpoint, and a branch the user later recreated there passed the ownership check. Two changes make the rule total. Every branch this mode creates now gets a baseline commit of its own, empty when the user's tree was clean: without it the branch sits exactly where the user's HEAD does and nothing tells it apart from a branch they create at the same place. It also makes `git diff <baseline>.. <branch>` the agent's work on every branch rather than only on the ones that started dirty, which is what the surrounding comments already claimed. And Finalize verifies the delivery point before recording it: the tip has to BE the task's branch — not a detached checkout, not a branch someone moved underneath the run — and it has to still contain the anchor, which is that baseline or the checkpoint the previous turn recorded and this one verified. A delivery that fails either test is not recorded at all: the task fails, the worktree is preserved, and the error names what is missing. A sibling racing the first turn of a conversation forks from it again, since every branch now carries a recordable checkpoint from the moment it is prepared. Co-authored-by: multica-agent <[email protected]> * fix(daemon): keep the anchor on the commit that carries this turn's user edits A follow-up turn commits the user's newest edits as its own baseline, but the anchor stayed on the checkpoint the previous turn had recorded. A run that then reset its worktree back to the previous tip still passed verification — the old checkpoint is an ancestor of it — so the turn was recorded as having delivered edits the branch no longer had. The turn after that computed its replay from a state the branch never took, and the user's edits were simply absent from the worktree with nothing to indicate it. The anchor now follows every baseline this task commits, so it always names the newest commit carrying the state about to be recorded. Resetting past it fails the turn and preserves the worktree, as for the first-turn case. The conflict path had the same hole from the other end: there the branch does not carry the snapshot until the agent resolves and something is committed. When a turn that started mid-merge ends with the branch exactly where it began, the record keeps the state the branch is known to carry instead of advancing, and the next turn offers those edits again. A resolution in favour of the branch's own side leaves the same empty trace as discarding the merge, so this trades a merge that may have to be concluded twice against recording edits as delivered that may have been dropped. Co-authored-by: multica-agent <[email protected]> * fix(daemon): measure a delivery against the commit its own turn started from A turn that starts mid-merge only carries the user's edits once the agent resolves and something is committed, and the guard for that compared the delivered tip against the checkpoint an earlier turn had recorded. Those are not the same commit: the user committing on a delivered branch is supported and tested, so the branch tip a later turn starts from can already be past that checkpoint. A resolution that committed nothing then left the tip equal to the turn's own starting point but not to the older checkpoint, the guard did not fire, and the edits were recorded as delivered — missing from the turn after with nothing to indicate it. Both questions now measure against LocalWorktree.BaseCommit, the commit this turn actually started from. It is at least as new as the earlier checkpoint in every case, so the separate anchor field is gone: a delivered tip that contains this turn's starting point contains everything an earlier turn recorded, and a tip that does not is a run that reset away work already delivered. Co-authored-by: multica-agent <[email protected]> * test(daemon): cover the claim-to-branch wiring with a two-turn Prepare test Every multi-turn test so far drove PrepareLocalWorktree with hand-filled params, which skips the step that turns a claim into them: which field becomes the branch name, and which become the identity the branch is recorded under. This one runs two turns of one issue through Prepare itself, and a third for another workspace carrying the same issue identifier. Both halves of that mapping are load-bearing — blanking the conversation key or fixing the workspace id makes it fail. Co-authored-by: multica-agent <[email protected]> * fix(daemon): carry the worktree's own state across the preparation helper Prepare runs in a short-lived helper process and the Environment it built comes back to the daemon as JSON. LocalWorktree keeps everything Finalize reasons about in unexported fields, and ordinary struct marshalling drops those without a word — so the daemon finalized a worktree whose owner, snapshot, tracksState and createdBranch were all zero. Nothing failed; the guarantees were simply absent in production. No delivery verification ran, Finalize recorded nothing (leaving the checkpoint at the baseline Prepare had written inside the helper), a conflicted turn could not hold its snapshot back, and a read-only turn kept its branch. Every in-process test passed throughout, because they call PrepareLocalWorktree directly. Custom Marshal/Unmarshal now carry that state. aborted stays out of the wire deliberately: the daemon sets it after Prepare returns, so it belongs to the parent. The regression tests run two turns through PrepareIsolated — the real boundary — and assert the state arrives, that Finalize's record names the tip it delivered rather than the baseline, and that a read-only turn still drops its branch. Found by running the whole stack: a real daemon, a real local_directory in worktree mode, and two real turns. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-09-01 13:50:52 +08:00
Bohan Jiang	0a54725fe6	MUL-6567 fix(execenv): lock a local_directory repo's git section across processes (#7462) ... The lock meant to serialise git admin work on one repository was an in-process sync.Mutex, and no two contenders were ever in one process: a daemon prepares every task in a short-lived helper, and worktree mode deliberately skips the per-path task mutex, so N concurrent tasks on one local_directory were N processes issuing git commands at the same repo with nothing between them. `git stash create`, which captures the user's uncommitted work before the worktree is built, needs the repository's index lock and lost that race routinely — reproducibly in about two runs out of three for two concurrent prepares on a dirty repo. git reports the loss as exit status 1 with nothing on stderr, and nothing retries a failed task, so the run died undiagnosable and the issue rolled back to todo. - lockGitRoot now takes an advisory file lock in the repository's COMMON git dir alongside the mutex, reusing the flock/LockFileEx primitives the env-root claim relies on. The common dir is what makes one lock cover one repository: the guarded section contends both for the per-worktree index.lock and for the refs / worktrees/ state every linked worktree shares. The file lock is taken before the mutex so waiting is not itself serialised. A git dir that cannot hold a lock file falls back to the mutex; a BUSY lock fails the caller. - The capture retries, 5 attempts over ~1.5s, for the index-lock races owned by the user's own tools. `stash create` writes a commit object and returns its sha without touching the index, refs or working tree, so retrying repeats nothing. - runGitStdout renders git's stderr into its error instead of collecting it into ExitError.Stderr and dropping it, and the capture names .git/index.lock when git itself says nothing. Finalize and Discard take the same lock, so a prepare in a helper and a finalize in the daemon parent no longer overlap. Finalize reports its worktree as preserved if it cannot get the lock. Seven tests, each verified to fail against the code it fixes, including a second PROCESS holding the lock and two concurrent PrepareIsolated runs on one repo. The timeout message is pinned never to advise deleting a held lock file: the lock binds to an open file description, not a path, so unlinking it would let two tasks into the same section at once. Refs #7434	2026-08-24 01:12:39 +08:00