Skip to content

feat(personas): minimal Mode 2 E2E hello persona#160

Open
khaliqgant wants to merge 1 commit into
mainfrom
feat/e2e-mode2-hello-persona
Open

feat(personas): minimal Mode 2 E2E hello persona#160
khaliqgant wants to merge 1 commit into
mainfrom
feat/e2e-mode2-hello-persona

Conversation

@khaliqgant
Copy link
Copy Markdown
Member

Summary

  • Adds personas/e2e-mode2-hello/ — a typed Mode 2 (single agent.ts handler) persona authored via definePersona, compiled to persona.json via agentworkforce persona compile.
  • Trigger: GitHub issues.opened / issues.labeled on AgentWorkforce/cloud. Handler filters for the hello label and posts a single confirmation comment via ctx.github.comment.
  • Exists to prove the Mode 2 path end-to-end with a persona that isn't cloud-small-issue-codex. Absolute minimum surface: one trigger, one ctx primitive call. No clone, no shell, no workflow, no runtime work.

Why

  • Reference Mode 2 persona (cloud-small-issue-codex) is heavyweight (clone + workflow + Slack + claim-comments + credential plumbing). A fresh minimal persona isolates the Mode 2 dispatch path itself from the codex/clone surface area, so a future Mode 2 regression has a 70-line test bed rather than a 1000-line one.
  • Doubles as the first typed-authoring persona in this repo, exercising the persona compile -> persona.json -> deploy --mode cloud flow end-to-end.

Deployment

Already deployed against the active workforce workspace via agentworkforce deploy ./personas/e2e-mode2-hello/persona.json --mode cloud --on-exists update:

  • agent: 478c7778-66c6-4581-ae6e-0447e71c9147
  • deployment: 740ff251-744e-4c28-a89e-a0ebf938f2c2

Live E2E fire is gated on AgentWorkforce/cloud#1349 (snapshot regression fix) landing + deploying — the persona definition itself is independent and deploys cleanly.

Test plan

  • agentworkforce persona compile personas/e2e-mode2-hello/persona.ts succeeds (writes valid persona.json)
  • agentworkforce deploy ... --mode cloud --on-exists update returns a deployment id (no silent cancel-default no-op)
  • Live: open an AgentWorkforce/cloud issue with the hello label after cloud#1349 deploys, verify the persona posts the confirmation comment

🤖 Generated with Claude Code

Adds personas/e2e-mode2-hello — a typed Mode 2 (single agent.ts handler)
persona that listens for GitHub `issues.opened`/`issues.labeled` on
AgentWorkforce/cloud, and posts one confirmation comment when the issue
carries the `hello` label. No clone, no shell, no workflow. Authored as
persona.ts via definePersona, compiled to persona.json via
`agentworkforce persona compile`.

Exists to prove the Mode 2 path end-to-end with a persona that is not
cloud-small-issue-codex and exercises the minimum surface (one trigger,
one ctx.github.comment call).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds a complete end-to-end test persona (e2e-mode2-hello) that listens for GitHub issue events, validates repository and issue state, checks for a "hello" label, and posts a single confirmation comment. The implementation includes the persona definition, event handler with comprehensive validation logic, and required package dependencies.

Changes

E2E Mode 2 Hello Persona

Layer / File(s) Summary
Persona definition and configuration
personas/e2e-mode2-hello/persona.ts, personas/e2e-mode2-hello/persona.json, personas/e2e-mode2-hello/package.json
Persona metadata, configuration contract, GitHub event triggers (issues.opened, issues.labeled), and @agentworkforce/runtime dependency are declared.
Event handler and validation logic
personas/e2e-mode2-hello/agent.ts
Handler validates event source (github), repository identity, issue state (open), and label presence (case-insensitive hello), then posts a confirmation comment. Includes payload coercion and label normalization helpers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A persona blooms in the cloud so bright,
Listening for issues with labels just right,
When "hello" appears on a GitHub thread,
A comment of cheer bounces back to the thread!
Simple, sweet, and Mode 2 complete! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(personas): minimal Mode 2 E2E hello persona' clearly summarizes the main change: adding a new minimal Mode 2 end-to-end test persona. It is concise, specific, and directly reflects the primary purpose of the changeset.
Description check ✅ Passed The description comprehensively explains the purpose, implementation, and deployment status of the new persona. It covers what was added, why it matters, and the current test status—all directly related to the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/e2e-mode2-hello-persona

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@personas/e2e-mode2-hello/agent.ts`:
- Around line 24-27: The current gate allows any issues.labeled webhook to
trigger the probe, causing duplicate comments; change the condition to only
proceed for issues.opened OR for issues.labeled events where the action is
'labeled' and the added label name === 'hello' (inspect event.action and
event.label.name on the payload), and before posting, query existing issue
comments to skip posting if a confirmation comment from this bot already exists
(or if the issue already had the hello label prior to this event). Update the
conditional that uses event and ctx.log (and the label-handling logic referenced
around the blocks you noted) so it only handles the specific label-addition case
and performs an idempotency check to avoid duplicate comments.
- Around line 32-35: The code defaults a missing repo.full_name to
REPO_FULL_NAME allowing malformed payloads through; change the logic in agent.ts
around fullName/REPO_FULL_NAME so missing repository.full_name fails closed: get
fullName with stringValue(repo?.full_name) without the fallback, log and return
if fullName is falsy (use ctx.log with a clear message), then only continue if
fullName === REPO_FULL_NAME (and otherwise log and return as it currently does).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c319ea78-a557-4090-a2ad-aff886a1477f

📥 Commits

Reviewing files that changed from the base of the PR and between 778d046 and b43fc15.

📒 Files selected for processing (4)
  • personas/e2e-mode2-hello/agent.ts
  • personas/e2e-mode2-hello/package.json
  • personas/e2e-mode2-hello/persona.json
  • personas/e2e-mode2-hello/persona.ts

Comment on lines +24 to +27
if (event.type !== 'issues.opened' && event.type !== 'issues.labeled') {
ctx.log('info', 'ignoring non-issue event', { type: event.type });
return;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

The current label gate can post duplicate comments for unrelated label events.

Once an issue has hello, every future issues.labeled event still satisfies Line 45 and can post another confirmation comment, which breaks the intended single-comment probe behavior.

Proposed fix
   const labels = readLabels(issue.labels ?? resource.labels);
   if (!labels.includes(LABEL)) {
     ctx.log('info', 'skipping issue without hello label', { labels, eventId: event.id });
     return;
   }
+  if (event.type === 'issues.labeled') {
+    const addedLabel = stringValue(asRecord(resource.label).name)?.toLowerCase();
+    if (addedLabel !== LABEL) {
+      ctx.log('info', 'skipping labeled event for non-hello label', { addedLabel, eventId: event.id });
+      return;
+    }
+  }

Also applies to: 44-47, 67-71

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@personas/e2e-mode2-hello/agent.ts` around lines 24 - 27, The current gate
allows any issues.labeled webhook to trigger the probe, causing duplicate
comments; change the condition to only proceed for issues.opened OR for
issues.labeled events where the action is 'labeled' and the added label name ===
'hello' (inspect event.action and event.label.name on the payload), and before
posting, query existing issue comments to skip posting if a confirmation comment
from this bot already exists (or if the issue already had the hello label prior
to this event). Update the conditional that uses event and ctx.log (and the
label-handling logic referenced around the blocks you noted) so it only handles
the specific label-addition case and performs an idempotency check to avoid
duplicate comments.

Comment on lines +32 to +35
const fullName = stringValue(repo?.full_name) ?? REPO_FULL_NAME;
if (fullName !== REPO_FULL_NAME) {
ctx.log('info', 'ignoring event for different repo', { fullName });
return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail closed when repository.full_name is missing.

Line 32 currently defaults missing repo metadata to AgentWorkforce/cloud, which can let malformed payloads pass the repo gate and write a comment anyway.

Proposed fix
-  const fullName = stringValue(repo?.full_name) ?? REPO_FULL_NAME;
-  if (fullName !== REPO_FULL_NAME) {
+  const fullName = stringValue(repo?.full_name);
+  if (!fullName) {
+    ctx.log('warn', 'missing repository.full_name', { eventId: event.id });
+    return;
+  }
+  if (fullName !== REPO_FULL_NAME) {
     ctx.log('info', 'ignoring event for different repo', { fullName });
     return;
   }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@personas/e2e-mode2-hello/agent.ts` around lines 32 - 35, The code defaults a
missing repo.full_name to REPO_FULL_NAME allowing malformed payloads through;
change the logic in agent.ts around fullName/REPO_FULL_NAME so missing
repository.full_name fails closed: get fullName with
stringValue(repo?.full_name) without the fallback, log and return if fullName is
falsy (use ctx.log with a clear message), then only continue if fullName ===
REPO_FULL_NAME (and otherwise log and return as it currently does).

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 4 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="personas/e2e-mode2-hello/agent.ts">

<violation number="1" location="personas/e2e-mode2-hello/agent.ts:32">
P1: Fail closed when `repository.full_name` is missing instead of defaulting it to the target repo, so malformed payloads cannot bypass the repository guard.</violation>

<violation number="2" location="personas/e2e-mode2-hello/agent.ts:45">
P2: `issues.labeled` events are not constrained to the newly added `hello` label, so adding unrelated labels can still post the hello comment.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

const resource = asRecord(event.payload);
const issue = maybeRecord(resource.issue) ?? resource;
const repo = asRecord(resource.repository);
const fullName = stringValue(repo?.full_name) ?? REPO_FULL_NAME;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Fail closed when repository.full_name is missing instead of defaulting it to the target repo, so malformed payloads cannot bypass the repository guard.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At personas/e2e-mode2-hello/agent.ts, line 32:

<comment>Fail closed when `repository.full_name` is missing instead of defaulting it to the target repo, so malformed payloads cannot bypass the repository guard.</comment>

<file context>
@@ -0,0 +1,99 @@
+  const resource = asRecord(event.payload);
+  const issue = maybeRecord(resource.issue) ?? resource;
+  const repo = asRecord(resource.repository);
+  const fullName = stringValue(repo?.full_name) ?? REPO_FULL_NAME;
+  if (fullName !== REPO_FULL_NAME) {
+    ctx.log('info', 'ignoring event for different repo', { fullName });
</file context>

}

const labels = readLabels(issue.labels ?? resource.labels);
if (!labels.includes(LABEL)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: issues.labeled events are not constrained to the newly added hello label, so adding unrelated labels can still post the hello comment.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At personas/e2e-mode2-hello/agent.ts, line 45:

<comment>`issues.labeled` events are not constrained to the newly added `hello` label, so adding unrelated labels can still post the hello comment.</comment>

<file context>
@@ -0,0 +1,99 @@
+  }
+
+  const labels = readLabels(issue.labels ?? resource.labels);
+  if (!labels.includes(LABEL)) {
+    ctx.log('info', 'skipping issue without hello label', { labels, eventId: event.id });
+    return;
</file context>

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant