feat(deploy): create Gmail labels from Step 3 Config UI - #79
Merged
Conversation
The GITHUB_TOKEN runs as github-actions[bot] (integration 15368) which is not in the repository ruleset bypass list. Auto-merge silently stalls because the merge actor cannot bypass the protect-branches ruleset. Switch to GH_ADMIN_PAT so merges execute as a repo admin with bypass permissions, and add a PR approval step. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The GITHUB_TOKEN runs as github-actions[bot] (integration 15368) which is not in the repository ruleset bypass list. Auto-merge silently stalls because the merge actor cannot bypass the protect-branches ruleset. Create a dedicated GitHub App with contents:write and pull_requests:write permissions. The workflow generates short-lived tokens via actions/create-github-app-token, and the app is added to the ruleset bypass list so auto-merge can execute. Includes setup script: scripts/setup-dependabot-app.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The GitHub App manifest flow had HTML encoding issues, so the app was created manually. Replace the setup script with a practical per-repo version that stores secrets and updates the ruleset bypass list for any repo in the org where the app is already installed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The ruleset bypass list grants the app admin-level merge permissions, but --auto still respects ruleset rules. Use --admin to bypass the ruleset directly, matching how the app's bypass_mode is configured. Tested: PRs #68 and #69 merged successfully via app token + --admin. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Dependabot-triggered pull_request events do not have access to repository secrets. Switch to pull_request_target which runs in the context of the base branch and can access APP_ID and APP_PRIVATE_KEY. This is safe because the workflow only targets dependabot[bot] PRs and does not checkout any PR code. Addresses Copilot review comment on PR #71. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Allow users to create new Gmail labels directly from the configuration form instead of switching to Gmail. Adds "+ New Label" button next to both trigger and processed label dropdowns with inline input, API creation via gmail.labels scope, automatic list refresh, and auto-selection of the newly created label. Closes #78
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds inline Gmail label creation to the Step 3 Configure UI (Gmail → Drive) via a client-side modal and per-control refresh, adds a new deploy helper Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant UI as Deploy UI
participant Lib as createGmailLabel()
participant Gmail as Gmail API
User->>UI: Click "+ New Label"
UI->>User: Show modal / input
User->>UI: Enter name & confirm
UI->>Lib: call createGmailLabel(fetch, accessToken, labelName)
Lib->>Gmail: POST /gmail/v1/users/me/labels (Authorization: Bearer ...)
Gmail-->>Lib: 201 Created / error response
Lib-->>UI: return label object or throw error (includes status + body)
UI->>UI: refresh label dropdowns, auto-select new labels or display error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
Address PR review feedback: throw a user-friendly error when accessToken is missing instead of silently returning null, and remove the no-op catch-rethrow wrapper.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for creating Gmail labels directly from the deploy/configure UI for the “Gmail to Drive By Labels” script, plus introduces Dependabot auto-merge automation changes.
Changes:
- Added a testable, injectable
createGmailLabel()helper tosrc/deploy/index.jswith unit tests. - Updated
deploy/index.htmlStep 3 Gmail-to-Drive config UI to allow inline label creation and refresh/auto-select behavior. - Added Dependabot auto-merge workflow changes and a repository setup script for the GitHub App–based auto-merge flow.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/deploy/index.js |
Adds GMAIL_API_BASE and an injectable createGmailLabel(fetchFn, accessToken, labelName) helper. |
src/deploy/tests/index.test.js |
Adds unit tests covering createGmailLabel() success + error paths. |
deploy/index.html |
Adds UI + client-side label creation flow and updates Gmail-to-Drive label dropdown rendering. |
.github/workflows/dependabot-automerge.yml |
Switches to pull_request_target and uses a GitHub App token to approve/merge Dependabot PRs. |
scripts/setup-dependabot-app.sh |
New script to configure repo secrets/ruleset bypass for the Dependabot auto-merge GitHub App. |
Comments suppressed due to low confidence (1)
.github/workflows/dependabot-automerge.yml:11
pull_request_targetruns in the base repo context with access to secrets; even with the Dependabot-only guard, it’s important to keep this workflow from ever checking out or executing PR-controlled code. Consider adding an explicit comment + a hard guard likeif: github.event.pull_request.head.repo.full_name == github.repositoryto ensure it only ever runs for same-repo PRs.
on:
pull_request_target:
branches:
- main
permissions:
contents: write
pull-requests: write
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/dependabot-automerge.yml:
- Around line 23-35: Extract the duplicated conditional into a single decision
step (e.g., add a step with id should_merge that computes the boolean using the
existing expressions based on steps.metadata.outputs.update-type and
steps.metadata.outputs.dependency-type and emits an output like run_merge via
GITHUB_OUTPUT), then replace the long repeated if expressions on the "Generate
app token" (id: app-token) and "Approve and auto-merge" steps with if:
steps.should_merge.outputs.run_merge == 'true'; this centralizes the logic and
makes future changes to the merge criteria one-place edits only.
In `@deploy/index.html`:
- Around line 920-921: The three label-related functions are not exposed on
window like other handlers, causing inconsistency with inline onclick usage;
attach showNewLabelInput, hideNewLabelInput, and handleCreateLabel to the global
object by assigning them to window (similar to how browseForFile and
confirmSetupDone are exposed) so the inline handlers can reliably call
window.showNewLabelInput, window.hideNewLabelInput, and
window.handleCreateLabel.
- Around line 860-883: The inline onclick string interpolation in
showNewLabelInput injects selectId and containerId directly into HTML (used by
handleCreateLabel and hideNewLabelInput), which risks future injection if ID
validation changes; replace the string-based inline handlers by creating the
input/button elements via document.createElement, set their ids/textContent, and
attach event listeners with addEventListener that call
handleCreateLabel(selectId, containerId) and hideNewLabelInput(containerId), and
then append them to the container; alternatively, if you must keep string HTML,
run selectId and containerId through a safe escaping function (e.g., escapeHtml)
before interpolation and update the focus call to reference the generated
element node instead of document.getElementById('new-label-input-' + selectId)
where applicable.
- Around line 791-833: This HTML file defines a local createGmailLabel that
duplicates and diverges from the testable createGmailLabel(fetchFn, accessToken,
labelName) in src/deploy/index.js; replace the inline implementation by invoking
the shared module function (or, if bundling/importing the module isn't feasible,
refactor this function to call into a small adapter that forwards to that
module) so there is a single source-of-truth for label creation and error
formatting, and ensure calls that follow (refreshLabelDropdown and
fetchGmailLabels) still run after the shared function resolves; if you cannot
import the module, at minimum copy the exact error message/formatting logic from
src/deploy/index.js into this createGmailLabel to keep behavior consistent.
In `@scripts/setup-dependabot-app.sh`:
- Around line 89-103: The Python here interpolates $APP_ID directly into the
inline Python snippet (inside the assignment to UPDATED using CURRENT), which is
unsafe and brittle; change the invocation so the shell passes APP_ID as an
environment variable and read it in Python via os.environ.get('APP_ID') (or via
sys.argv) instead of embedding $APP_ID into the source, update the logic that
checks actor_id by comparing against the env-provided app_id variable, and add a
simple guard that fails fast or logs when APP_ID is missing; locate the snippet
around the UPDATED assignment and modify the python3 -c code to import os and
read app_id = os.environ.get('APP_ID') (or use argv) rather than using the
interpolated $APP_ID.
- Around line 51-64: The glob "${HOME}/dependabot-google-app-scripts"*.pem can
expand to multiple files; update the KEY_FILE selection logic around the
for-loop that sets KEY_FILE and uses candidate so it explicitly
documents/handles first-match behavior—e.g., ensure the loop intentionally
selects the first existing match and add a clear comment and/or a debug/info
output naming the chosen file (referencing KEY_FILE and candidate) so it's
explicit which file was picked when multiple matches exist.
In `@src/deploy/tests/index.test.js`:
- Line 442: The import for createGmailLabel is placed near the bottom of the
test file; move it into the main import block with the other imports so all
exports from ../index are imported together. Update the existing require(...) at
the top that currently imports other symbols (add createGmailLabel to that list)
and remove the standalone const { createGmailLabel } = require('../index')
declaration at the bottom; this consolidates imports and keeps the tests
consistent and readable.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: febda071-01f6-4d7e-97b2-0b4d0be9f3f2
📒 Files selected for processing (5)
.github/workflows/dependabot-automerge.ymldeploy/index.htmlscripts/setup-dependabot-app.shsrc/deploy/index.jssrc/deploy/tests/index.test.js
- Fix JSDoc to match actual throw behavior (no longer returns null) - Throw on missing labelName instead of returning null silently - Escape IDs via escapeHtml() in inline onclick handlers for defense in depth - Expose showNewLabelInput, hideNewLabelInput, handleCreateLabel on window for consistency with browseForFile and confirmSetupDone - Consolidate createGmailLabel import to top of test file
The + New Label button was only shown when Gmail labels were loaded (select dropdown mode). Now it also appears in the text-input fallback that renders when labels haven't loaded yet or when the user has no existing labels. Also handles auto-filling text inputs after creation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deploy/index.html`:
- Around line 924-929: Guard the container lookup returned by
document.getElementById(containerId) before calling querySelectorAll to avoid a
null dereference if the element was removed; fetch the element into a local
variable (e.g., const container = document.getElementById(containerId)), check
if container is truthy, then call container.querySelectorAll('button') and
disable each button, otherwise handle the null case (e.g., skip button disabling
or re-enable the input) so input.disabled and btns operations in the form
submission flow are safe.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: ed906adc-64a5-4fec-97c9-d1a4d504db8a
📒 Files selected for processing (2)
deploy/index.htmlsrc/deploy/tests/index.test.js
Redesign the label creation UX per user feedback: - Single "New Label" button at the top of the Gmail to Drive config section - Modal popup that creates both the trigger label and its -archived counterpart in one action - Live preview showing both labels as user types - Auto-selects both labels in the config dropdowns after creation - Supports Enter key, Escape to close, and overlay click to dismiss
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deploy/index.html`:
- Around line 962-999: Make the two-label creation sequence idempotent: update
createGmailLabel to surface HTTP status (so callers can detect 409) and in the
try/catch flow treat a 409 as success/"already exists"; always add both name and
archivedName to the in-memory userGmailLabels set regardless of create result,
and move the UI refresh/selection calls (fetchGmailLabels,
refreshLabelDropdown(triggerSelectId, name),
refreshLabelDropdown(processedSelectId, archivedName)) plus resetting
input/create/cancel button state into a finally-style path so the selects are
refreshed and labels selected even after a partial failure; ensure error
handling still displays non-409 errors in errorEl and that createBtn text is
restored when done.
- Around line 821-827: The thrown error uses escapeHtml(labelName) which embeds
HTML-escaped entities into the Error message; change the error construction to
use the raw labelName (e.g., build msg using `Label "${labelName}" already
exists`) and do not call escapeHtml there — keep escapeHtml only at HTML
insertion sites (the renderer that uses textContent already handles safe
display). Update the branch that sets msg (and the subsequent throw new
Error(msg)) to include the unescaped labelName and remove escapeHtml references.
- Around line 930-947: Replace the multiple direct-close paths with a single
closeModal helper that does the teardown (remove overlay DOM node, remove
document keydown listener `onEsc`, and disable/cleanup buttons) and returns
early if a creation is in progress (e.g., guard with an `isCreating` flag);
update the overlay click handler, `onEsc`, and `cancelBtn` click handler to call
this `closeModal()` instead of calling `overlay.remove()` or
`document.removeEventListener('keydown', onEsc)` directly so all exits are
idempotent and blocked while create is in flight.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 0767a9b3-0518-4e0f-9662-cc2575ce3549
📒 Files selected for processing (1)
deploy/index.html
Comment on lines
+962
to
+999
| try { | ||
| // Create trigger label | ||
| await createGmailLabel(name) | ||
| previewTrigger.innerHTML = | ||
| '<span style="color:#137333;">\u2705 ' + | ||
| escapeHtml(name) + | ||
| ' — created</span>' | ||
|
|
||
| // Create archived label | ||
| await createGmailLabel(archivedName) | ||
| previewProcessed.innerHTML = | ||
| '<span style="color:#137333;">\u2705 ' + | ||
| escapeHtml(archivedName) + | ||
| ' — created</span>' | ||
|
|
||
| // Refresh the label list and auto-select both dropdowns | ||
| await fetchGmailLabels() | ||
| refreshLabelDropdown(triggerSelectId, name) | ||
| refreshLabelDropdown(processedSelectId, archivedName) | ||
|
|
||
| successEl.textContent = | ||
| 'Both labels created and selected in your configuration.' | ||
| successEl.style.display = '' | ||
| createBtn.textContent = 'Done' | ||
|
|
||
| // Auto-close after a short delay | ||
| setTimeout(function () { | ||
| overlay.remove() | ||
| document.removeEventListener('keydown', onEsc) | ||
| }, 1200) | ||
| } catch (err) { | ||
| errorEl.textContent = err.message | ||
| errorEl.style.display = '' | ||
| input.disabled = false | ||
| createBtn.disabled = false | ||
| cancelBtn.disabled = false | ||
| createBtn.textContent = 'Create Labels' | ||
| } |
There was a problem hiding this comment.
Handle the two label creations idempotently; retries currently get stuck after a partial success.
If createGmailLabel(name) succeeds and createGmailLabel(archivedName) then fails with 409 or a transient error, Gmail has already been mutated but the UI never refreshes/selects the first label. The next click then fails on the first create call, even though the only remaining work is creating/selecting the archived label. Also, because fetchGmailLabels() is best-effort, relying on it as the only refresh path can still leave the selects stale after a successful create. Please make this sequence recoverable: surface the HTTP status from createGmailLabel, treat 409 as “already exists”, add both known names to userGmailLabels, and refresh/select the controls in a finally-style path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/index.html` around lines 962 - 999, Make the two-label creation
sequence idempotent: update createGmailLabel to surface HTTP status (so callers
can detect 409) and in the try/catch flow treat a 409 as success/"already
exists"; always add both name and archivedName to the in-memory userGmailLabels
set regardless of create result, and move the UI refresh/selection calls
(fetchGmailLabels, refreshLabelDropdown(triggerSelectId, name),
refreshLabelDropdown(processedSelectId, archivedName)) plus resetting
input/create/cancel button state into a finally-style path so the selects are
refreshed and labels selected even after a partial failure; ensure error
handling still displays non-409 errors in errorEl and that createBtn text is
restored when done.
Collaborator
Author
|
@claude - review and fix comments and failing CI |
…nt label creation - Remove escapeHtml from thrown error messages (rendered via textContent) - Add shared closeModal helper with isCreating guard to prevent closing during label creation and fix leaked keydown listener - Make two-label creation idempotent: treat 409/duplicate as success, add labels to userGmailLabels on each step, so retries after partial failure work correctly - Surface isDuplicate and status on error objects for caller detection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Take main's pinned action hash for create-github-app-token. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
createGmailLabel()tosrc/deploy/index.js— testable, injectable function matching the existing patternsrc/deploy/index.jsCloses #78
Test plan
createGmailLabelunit tests pass (success, missing params, 409 duplicate, 500 server error)src/deploy/index.jsmaintains 100% coverageProjects/test) create correctlySummary by CodeRabbit
New Features
Chores
Tests