From c2e382c95ba5ee18115e60b2b6f47dd86a3d875f Mon Sep 17 00:00:00 2001 From: Ben Sima Date: Sat, 29 Nov 2025 23:34:47 -0500 Subject: Store agent review notes as task comments The implementation is complete. Here's a summary of what was implemented **Changes to [Omni/Jr.hs](file:///home/ben/omni/Omni/Jr.hs):** 1. **`autoReview`** - Now adds a review comment with: - Commit SHA (short) - Test target (namespace) - Result (PASSED/FAILED) - Test output (truncated to 1000 chars) when tests fail 2. **`interactiveReview`** - Now adds a human review comment with: - Commit SHA (short) - Result (ACCEPTED/REJECTED) - Rejection reason (when rejected) 3. **`handleConflict`** - Now adds a merge conflict comment with: - Commit SHA (short) - Attempt number - List of conflicting files 4. **Helper functions added:** - `buildReviewComment` - Formats auto-review results - `buildHumanReviewComment` - Formats human review results - `buildConflictComment` - Formats merge conflict info - `truncateOutput` - Truncates long test output Task-Id: t-193.4 --- Omni/Jr.hs | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 2 deletions(-) (limited to 'Omni/Jr.hs') diff --git a/Omni/Jr.hs b/Omni/Jr.hs index 20117fd..49f94c8 100755 --- a/Omni/Jr.hs +++ b/Omni/Jr.hs @@ -229,6 +229,9 @@ handleConflict tid conflictFiles commitSha = do maybeCtx <- TaskCore.getRetryContext tid let attempt = maybe 1 (\c -> TaskCore.retryAttempt c + 1) maybeCtx + let conflictComment = buildConflictComment commitSha conflictFiles attempt + _ <- TaskCore.addComment tid conflictComment + if attempt > 3 then do putText "[review] Task has failed 3 times. Needs human intervention." @@ -252,6 +255,21 @@ handleConflict tid conflictFiles commitSha = do TaskCore.updateTaskStatus tid TaskCore.Open [] putText ("[review] Task " <> tid <> " returned to queue (attempt " <> tshow attempt <> "/3).") +-- | Build a review comment for merge conflicts +buildConflictComment :: String -> [Text] -> Int -> Text +buildConflictComment commitSha conflictFiles attempt = + Text.unlines + [ "## Auto-Review: Merge Conflict", + "", + "**Commit:** " <> Text.pack (take 8 commitSha), + "**Result:** ✗ MERGE CONFLICT", + "**Attempt:** " <> tshow attempt <> "/3", + "", + "### Conflicting Files", + Text.unlines (map ("- " <>) conflictFiles), + "Task returned to queue for conflict resolution." + ] + -- | Gather Gerrit-style conflict context for the coder gatherConflictContext :: String -> [Text] -> IO Text gatherConflictContext commitSha conflictFiles = do @@ -384,13 +402,11 @@ autoReview tid task commitSha = do putText "[review] Running automated review..." putText ("[review] Commit: " <> Text.pack (take 8 commitSha)) - -- Determine what to test based on namespace let namespace = fromMaybe "." (TaskCore.taskNamespace task) let testTarget = Text.unpack namespace putText ("[review] Testing: " <> Text.pack testTarget) - -- Run bild --test on the namespace (testCode, testOut, testErr) <- Process.readProcessWithExitCode "bild" @@ -400,6 +416,8 @@ autoReview tid task commitSha = do case testCode of Exit.ExitSuccess -> do putText "[review] ✓ Tests passed." + let reviewComment = buildReviewComment commitSha testTarget True testOut testErr + _ <- TaskCore.addComment tid reviewComment TaskCore.clearRetryContext tid TaskCore.updateTaskStatus tid TaskCore.Done [] putText ("[review] Task " <> tid <> " -> Done") @@ -411,6 +429,9 @@ autoReview tid task commitSha = do maybeCtx <- TaskCore.getRetryContext tid let attempt = maybe 1 (\ctx -> TaskCore.retryAttempt ctx + 1) maybeCtx + let reviewComment = buildReviewComment commitSha testTarget False testOut testErr + _ <- TaskCore.addComment tid reviewComment + if attempt > 3 then do putText "[review] Task has failed 3 times. Needs human intervention." @@ -432,6 +453,35 @@ autoReview tid task commitSha = do TaskCore.updateTaskStatus tid TaskCore.Open [] putText ("[review] Task " <> tid <> " reopened (attempt " <> tshow attempt <> "/3).") +-- | Build a review comment summarizing what was tested and the result +buildReviewComment :: String -> String -> Bool -> String -> String -> Text +buildReviewComment commitSha testTarget passed testOut testErr = + Text.unlines + [ "## Auto-Review", + "", + "**Commit:** " <> Text.pack (take 8 commitSha), + "**Test target:** " <> Text.pack testTarget, + "**Result:** " <> if passed then "✓ PASSED" else "✗ FAILED", + "", + if passed + then "All tests passed. Task accepted." + else + Text.unlines + [ "### Test Output", + "```", + Text.pack (truncateOutput 1000 (testOut ++ testErr)), + "```", + "", + "Task rejected and returned to queue for retry." + ] + ] + +-- | Truncate output to a maximum number of characters +truncateOutput :: Int -> String -> String +truncateOutput maxLen s + | length s <= maxLen = s + | otherwise = take maxLen s ++ "\n... (truncated)" + -- | Interactive review with user prompts interactiveReview :: Text -> TaskCore.Task -> String -> IO () interactiveReview tid task commitSha = do @@ -445,6 +495,8 @@ interactiveReview tid task commitSha = do case Text.toLower choice of c | "a" `Text.isPrefixOf` c -> do + let acceptComment = buildHumanReviewComment commitSha True Nothing + _ <- TaskCore.addComment tid acceptComment TaskCore.clearRetryContext tid TaskCore.updateTaskStatus tid TaskCore.Done [] putText ("Task " <> tid <> " marked as Done.") @@ -453,6 +505,8 @@ interactiveReview tid task commitSha = do putText "Enter rejection reason: " IO.hFlush IO.stdout reason <- getLine + let rejectComment = buildHumanReviewComment commitSha False (Just reason) + _ <- TaskCore.addComment tid rejectComment maybeCtx <- TaskCore.getRetryContext tid let attempt = maybe 1 (\ctx -> TaskCore.retryAttempt ctx + 1) maybeCtx let currentReason = "attempt " <> tshow attempt <> ": rejected: " <> reason @@ -472,6 +526,19 @@ interactiveReview tid task commitSha = do putText ("Task " <> tid <> " reopened (attempt " <> tshow attempt <> "/3).") | otherwise -> putText "Skipped; no status change." +-- | Build a human review comment +buildHumanReviewComment :: String -> Bool -> Maybe Text -> Text +buildHumanReviewComment commitSha accepted maybeReason = + Text.unlines + [ "## Human Review", + "", + "**Commit:** " <> Text.pack (take 8 commitSha), + "**Result:** " <> if accepted then "✓ ACCEPTED" else "✗ REJECTED", + case maybeReason of + Just reason -> "**Reason:** " <> reason + Nothing -> "" + ] + -- | Check if a commit can be cleanly cherry-picked onto live -- Returns Nothing if clean, Just [conflicting files] if conflict checkMergeConflict :: String -> IO (Maybe [Text]) -- cgit v1.2.3