Skip to content

task merge can merge a commit the gate never tested #418

Description

@johnkattenhorn

On Luvus 0.14.2 a task's gate and its merge look at different things. The gate runs sh -c <gate> in the lane's worktree when the lane calls task done, so it tests whatever is on disk at that moment. task merge later runs git merge --no-ff -- <branch> by branch name (run_gate_command and start_task_merge / integrate_branch in src/app/board.rs and src/git/local.rs). Nothing records which commit the gate passed.

I hit this two ways on a throwaway session:

  1. A commit made after the gate starts is merged ungated. I committed onto a lane's branch 40 ms after task.gate_running. The gate passed on the old HEAD (d2669a5). The merge commit's second parent was the new one (26d46e6).
  2. Uncommitted work passes the gate and is never merged. A lane ran luvus task done before committing. The gate passed on the untracked files in the worktree. task merge merged the branch, which didn't contain them. The task reads merged, and a second task done is refused with t6 is already merged, so the work can only be recovered by hand.

To reproduce the first case:

luvus --session scratch task add "demo" --gate 'sleep 5' --workspace-id <ws>
luvus --session scratch task start <id> --workspace-id <ws>
# in the lane's worktree:
git commit --allow-empty -m before && luvus task done <id>
# while the gate sleeps:
git commit --allow-empty -m after
luvus --session scratch task merge <id>
git log -1 --format=%P luvus/integration   # second parent is "after", which no gate ran on

What would close it:

  • Record the commit the gate tested on the task, for example gate_commit, and report it in task.gate_passed.
  • Have task merge merge that commit, or refuse with something like head_moved when the branch head is different.
  • Optionally, fail the gate, or at least say so in its output, when the worktree has uncommitted or untracked changes. The merge only ever takes commits.

I've worked around this outside Luvus for now. My gate wrapper refuses a dirty worktree and writes git rev-parse HEAD to the worktree's git dir, and the merge side compares that with the branch head before calling task merge. It works, but it depends on every gate being wrapped, and a gate written without the wrapper has no protection.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    status: needs triageNeeds maintainer review and classification

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions