Remove hardcoded bot usernames, use vars.BOT_USERNAME - #469
Conversation
Replace hardcoded 'james-in-a-box' bot usernames with configurable vars.BOT_USERNAME repository variable (falling back to 'egg'). Changes: - Entry-point workflows now use vars.BOT_USERNAME || 'egg' - sdlc-hitl.yml moves self-trigger check from job-level if to step - build-mention-prompt.sh and config.py defaults updated to 'egg' - policy.py comment examples made generic - Test fixtures use 'test-bot' instead of specific bot name - Added documentation in reusable-workflows.md for setup Fixes #458
There was a problem hiding this comment.
No agent-mode design concerns.
This PR is infrastructure/configuration work — making bot identity configurable rather than hardcoded. The changes don't involve agent workflow design patterns:
- No pre-fetching of context that would constrain agent exploration
- No structured output requirements imposed on agents
- No post-processing pipelines parsing agent output
- No rigid procedures micromanaging agent behavior
- Security constraints remain sandbox-enforced
The self-trigger prevention moving from job-level if: to step-level is a reasonable GitHub Actions workaround that doesn't affect agent design.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Review: Remove hardcoded bot usernames, use vars.BOT_USERNAME
This PR addresses issue #458 by replacing hardcoded james-in-a-box bot usernames with configurable vars.BOT_USERNAME repository variables, and moving self-trigger prevention from job-level to step-level in sdlc-hitl.yml.
Issues Found
1. Incomplete Scope: Other workflows still have hardcoded defaults in resolve-inputs
The PR correctly updates entry-point workflows (on-check-failure.yml, on-pull-request.yml, etc.) to use vars.BOT_USERNAME, but several reusable workflows still have hardcoded james-in-a-box in their resolve-inputs fallbacks:
.github/workflows/on-review-feedback.yml:83:echo "bot_username=${{ inputs.bot_username || 'james-in-a-box' }}".github/workflows/on-mention.yml:81:echo "bot_username=${{ inputs.bot_username || 'james-in-a-box' }}".github/workflows/sdlc-pipeline.yml:98:echo "bot_username=${{ inputs.bot_username || 'james-in-a-box' }}".github/workflows/sdlc-work-loop.yml:194:echo "bot_username=${{ inputs.bot_username || 'james-in-a-box' }}"
These resolve-inputs fallbacks should also be updated to use 'egg' for consistency with sdlc-hitl.yml:66 which now uses 'egg'. While callers can override via inputs, the internal fallbacks should match the documented default.
Recommendation: Update these resolve-inputs fallbacks to 'egg' for consistency, or document why they're intentionally left as james-in-a-box.
2. Input default inconsistency between workflows
The PR updates sdlc-hitl.yml input default from james-in-a-box to egg (line 23), but other reusable workflows still define default: "james-in-a-box" in their inputs:
on-review-feedback.yml:31on-mention.yml:27sdlc-pipeline.yml:37sdlc-work-loop.yml:84reusable-review.yml:18reusable-autofix.yml:24reusable-conflict-resolve.yml:19
This creates an inconsistency: some workflows default to egg, others to james-in-a-box. If the goal is to allow external adopters to use generic defaults, all should be updated.
Recommendation: Either update all workflow input defaults to egg, or explicitly document that these are intentionally left for backward compatibility.
3. Minor: Documentation mentions only entry-point workflows
The new documentation in docs/guides/reusable-workflows.md (lines 149-175) explains vars.BOT_USERNAME usage but the example shows calling a reusable workflow (reusable-review.yml), not an entry-point workflow. Since the reusable workflows receive bot_username via the with: input block, this is correct—but it would be clearer to note that:
- Entry-point workflows use
vars.BOT_USERNAMEdirectly - When calling reusable workflows, you pass it via the
bot_usernameinput
What Looks Good
-
Self-trigger prevention move is correct: Moving from job-level
if:to step-level check inhandle-feedbackjob allows the check to use the configuredbot_usernamefromneeds.resolve-inputs.outputs. The implementation pattern matches other similar checks in the file (e.g., check-trigger job at line 108, handle-phase-approval at line 586). -
Step guards are properly chained: Subsequent steps correctly check
steps.auth.outputs.authorized == 'true'before proceeding. -
Test fixture updates are appropriate: Using
test-botinstead ofjames-in-a-boxin test fixtures is cleaner and avoids coupling tests to production values. -
Documentation and error messages updated: The gateway/policy.py error message now shows a generic example.
Summary
The core changes to entry-point workflows and sdlc-hitl.yml are correct. However, the PR leaves several reusable workflows with hardcoded james-in-a-box defaults and resolve-inputs fallbacks, creating an inconsistency between sdlc-hitl.yml (which now defaults to egg) and other workflows.
If this is intentional (leaving reusable workflow defaults unchanged for backward compatibility while only updating entry-points), please document that decision. Otherwise, the scope should be expanded to cover all workflows for consistency.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
| egg is addressing review feedback... |
Address review feedback on PR #469 by updating all remaining workflow bot_username defaults and resolve-inputs fallbacks from 'james-in-a-box' to 'egg' for consistency with sdlc-hitl.yml. Changes: - Update input defaults in 7 reusable workflows - Update resolve-inputs fallbacks in 4 workflows - Clarify entry-point vs reusable workflow usage in docs
|
Addressed all review feedback issues. Issue 1: Incomplete Scope - resolve-inputs fallbacksUpdated all four reusable workflows to use
Issue 2: Input default inconsistencyUpdated all seven workflows with bot_username input defaults to use
Issue 3: Documentation clarityUpdated
All workflows now consistently default to — Authored by egg |
|
egg feedback addressed. View run logs 2 previous review(s) hidden. |
There was a problem hiding this comment.
Agent-mode design re-review: No concerns.
Changes since 86b58ad address all three issues from the previous review:
- ✓ Resolve-inputs fallbacks now use
'egg'in all 4 workflows - ✓ Input defaults now use
"egg"in all 7 workflows - ✓ Documentation now clarifies entry-point vs reusable workflow usage
This remains infrastructure/configuration work with no agent-mode design implications. Ready for human approval.
— Authored by egg
|
egg agent-mode-design completed. View run logs |
There was a problem hiding this comment.
Re-Review: Feedback Addressed
All three issues from my previous review have been addressed correctly.
Verification Summary
Issue 1: resolve-inputs fallbacks ✅
All four resolve-inputs fallbacks now use 'egg':
on-review-feedback.yml:83on-mention.yml:81sdlc-pipeline.yml:98sdlc-work-loop.yml:194
Issue 2: input default inconsistency ✅
All seven workflow input defaults now use "egg":
on-review-feedback.yml:31on-mention.yml:27sdlc-pipeline.yml:37sdlc-work-loop.yml:84reusable-review.yml:18reusable-autofix.yml:24reusable-conflict-resolve.yml:19
Issue 3: documentation clarity ✅
Documentation in reusable-workflows.md (lines 162-196) now clearly explains:
- Entry-point workflows use
vars.BOT_USERNAMEdirectly - Reusable workflows receive
bot_usernamevia thewith:input block - Includes examples for both scenarios
Observation (Non-blocking)
The mention_patterns default in on-mention.yml:42 and its resolve-inputs fallback at line 83 still contain @james-in-a-box:
default: "@james-in-a-box,@egg"This is a separate parameter from bot_username and falls outside the scope of this PR (which targets bot username configuration), but may warrant a follow-up to ensure mention_patterns is consistent with the configured bot username. The documentation in docs/guides/github-automation.md:218 also references both mentions.
No blocking issues found.
— Authored by egg
|
egg review completed. View run logs |
Update @mention workflow documentation to remove reference to the old hardcoded bot username 'james-in-a-box'. This aligns with PR #469 which removed all hardcoded bot usernames and replaced them with the configurable BOT_USERNAME variable. Authored-by: egg
* docs: Remove hardcoded bot username from github-automation.md Update @mention workflow documentation to remove reference to the old hardcoded bot username 'james-in-a-box'. This aligns with PR #469 which removed all hardcoded bot usernames and replaced them with the configurable BOT_USERNAME variable. Authored-by: egg * Remove @Egg mention pattern from workflow defaults and docs - Update on-mention.yml to remove @Egg from default mention_patterns - Update github-automation.md to reference "the configured bot" instead of @Egg - Per request in #473 to stop supporting @Egg tagging Authored-by: egg --------- Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>
Remove hardcoded bot usernames, use vars.BOT_USERNAME
The codebase contained hardcoded
james-in-a-boxbot usernames in workflowinputs, shell script defaults, Python module defaults, and a job-level
if:condition. This prevented external adopters from using the SDLC workflows
with their own GitHub Apps. This was flagged as issue #9 in PR #457's review.
Entry-point workflows now use
${{ vars.BOT_USERNAME || 'egg' }}instead ofhardcoded values, allowing configuration via repository variables. The
self-trigger prevention check in
sdlc-hitl.ymlwas moved from the job-levelif:condition (which can't accessneedsoutputs) to an early step thatreads the configured
bot_username. Script and module defaults were changedfrom
james-in-a-boxto the genericegg. Documentation and test fixtureswere updated to use generic bot names.
After merging, set the
BOT_USERNAMErepository variable tojames-in-a-boxto preserve current behavior for this repository.
Fixes #458
Test plan:
vars.BOT_USERNAMEcorrectlyaction/build-mention-prompt.shdefaults toeggwhen BOT_USERNAME unsetAuthored-by: egg