Skip to content

refactor(difc): extract modifyTags helper to deduplicate bulk label mutation pattern#2561

Closed
Copilot wants to merge 2 commits into
mainfrom
copilot/duplicate-code-fix-bulk-label-operations
Closed

refactor(difc): extract modifyTags helper to deduplicate bulk label mutation pattern#2561
Copilot wants to merge 2 commits into
mainfrom
copilot/duplicate-code-fix-bulk-label-operations

Conversation

Copilot AI commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

AddSecrecyTags, AddIntegrityTags, and DropIntegrityTags each duplicated the same lock–operate–log boilerplate, while single-tag variants already delegated to modifyTag. This adds a parallel modifyTags helper and refactors all three bulk methods to use it.

Changes

  • New helper modifyTags in internal/difc/agent.go — mirrors modifyTag but operates on []Tag, with conditional operational logging (only when len(tags) > 0):

    func (a *AgentLabels) modifyTags(labelType, action, pastTense string, tags []Tag, fn func()) {
        logAgent.Printf("Agent %s %s %d %s tags", a.AgentID, action, len(tags), labelType)
        a.mu.Lock()
        defer a.mu.Unlock()
        fn()
        if len(tags) > 0 {
            log.Printf("[DIFC] Agent %s %s %s tags: %v", a.AgentID, pastTense, labelType, tags)
        }
    }
  • Refactored bulk methods to delegate to the helper:

    func (a *AgentLabels) AddSecrecyTags(tags []Tag) {
        a.modifyTags("secrecy", "adding", "gained", tags, func() { a.Secrecy.Label.AddAll(tags) })
    }

Log formats and behavior are preserved exactly.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • example.com
    • Triggering command: /tmp/go-build1956433166/b334/launcher.test /tmp/go-build1956433166/b334/launcher.test -test.testlogfile=/tmp/go-build1956433166/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s 8206�� ache/go/1.25.8/x64/src/runtime/c-c=4 SxT3yfSsn x_amd64/vet (dns block)
  • invalid-host-that-does-not-exist-12345.com
    • Triggering command: /tmp/go-build1956433166/b319/config.test /tmp/go-build1956433166/b319/config.test -test.testlogfile=/tmp/go-build1956433166/b319/testlog.txt -test.paniconexit0 -test.timeout=10m0s conf�� _.a mon/httpcommon.go x_amd64/compile (dns block)
  • nonexistent.local
    • Triggering command: /tmp/go-build1956433166/b334/launcher.test /tmp/go-build1956433166/b334/launcher.test -test.testlogfile=/tmp/go-build1956433166/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s 8206�� ache/go/1.25.8/x64/src/runtime/c-c=4 SxT3yfSsn x_amd64/vet (dns block)
  • slow.example.com
    • Triggering command: /tmp/go-build1956433166/b334/launcher.test /tmp/go-build1956433166/b334/launcher.test -test.testlogfile=/tmp/go-build1956433166/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s 8206�� ache/go/1.25.8/x64/src/runtime/c-c=4 SxT3yfSsn x_amd64/vet (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build1956433166/b343/mcp.test /tmp/go-build1956433166/b343/mcp.test -test.testlogfile=/tmp/go-build1956433166/b343/testlog.txt -test.paniconexit0 -test.timeout=10m0s -obj�� /middleware/jqschema.go -importpath x_amd64/compile -import_runtime_runc -import_syscall=--version -ldflags="-O2" "-g" "-lpthread" x_amd64/compile -I _.a 820628/b151/ x_amd64/vet --gdwarf-5 ernal/mcp -o x_amd64/vet (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

Copilot AI changed the title [WIP] Refactor bulk label operations to use shared helper refactor(difc): extract modifyTags helper to deduplicate bulk label mutation pattern Mar 26, 2026
Copilot AI requested a review from lpcox March 26, 2026 05:09
lpcox added a commit that referenced this pull request Mar 26, 2026
… env vars (#2569)

🤖 *This PR was created by Repo Assist, an automated AI assistant.*

## Summary

Several DIFC flags in `internal/cmd/flags_difc.go` used `os.Getenv()`
directly in the `init()` function instead of following the
`getDefault*()` helper pattern documented in `flags.go`. Additionally,
the helper table in `flags.go` referenced
`getDefaultDIFCSinkServerIDs()` which did not exist — a stale
documentation error.

## Changes

**`internal/cmd/flags_difc.go`**
- Adds `getDefaultDIFCSinkServerIDs()` (was referenced in `flags.go`
table but missing)
- Adds `getDefaultGuardPolicyJSON()`
- Adds `getDefaultAllowOnlyOwner()`
- Adds `getDefaultAllowOnlyRepo()`
- Adds `getDefaultAllowOnlyMinIntegrity()`
- Updates `init()` to use all helper functions consistently

**`internal/cmd/flags.go`**
- Expands the helper table to list all 11 `getDefault*()` helpers,
including `getDefaultAllowOnlyScopePublic()` which existed but was not
documented

**`internal/cmd/flags_difc_test.go`**
- Updates `TestGetDefaultDIFCSinkServerIDs` to call
`getDefaultDIFCSinkServerIDs()` directly (previously tested
`os.Getenv()` directly, not the helper)
- Updates `TestGetDefaultGuardPolicyInputs` to call all new helpers,
using `t.Setenv()` for cleaner automatic cleanup

## Rationale

The `getDefault*()` pattern exists precisely to make each flag's
environment variable override explicit and testable. Flags bypassing
this pattern:
1. Are harder to test in isolation
2. Diverge from the codebase's own documented convention
3. Cannot be easily found via code search when looking for all env var
defaults

This is a pure refactor — no behavioural changes.
`envutil.GetEnvString("X", "")` is equivalent to `os.Getenv("X")` for
string flags (both return empty string as default), and the helpers for
`owner`, `repo`, `min-integrity`, and `policy-json` all default to `""`.

## Test Status

⚠️ **Infrastructure failure**: The CI environment for this workflow run
does not have Go module dependencies available (network access to
`proxy.golang.org` is blocked), so `make agent-finished` could not be
run. The changes are a pure refactor with no logic changes, and the
updated tests exercise the new helper functions directly using
`t.Setenv()` for safe parallelism.

The PR CI should run full tests in the normal build environment.




> [!WARNING]
> <details>
> <summary><strong>⚠️ Firewall blocked 1 domain</strong></summary>
>
> The following domain was blocked by the firewall during workflow
execution:
>
> - `proxy.golang.org`
>
> To allow these domains, add them to the `network.allowed` list in your
workflow frontmatter:
>
> ```yaml
> network:
>   allowed:
>     - defaults
>     - "proxy.golang.org"
> ```
>
> See [Network
Configuration](https://github.github.com/gh-aw/reference/network/) for
more information.
>
> </details>


> [!NOTE]
> <details>
> <summary><b>🔒 Integrity filter blocked 14 items</b></summary>
>
> The following items were blocked because they don't meet the GitHub
integrity level.
>
> - [#2567](#2567)
`list_pull_requests`: has lower integrity than agent requires. The agent
cannot read data with integrity below "merged".
> - [#2561](#2561)
`list_pull_requests`: has lower integrity than agent requires. The agent
cannot read data with integrity below "merged".
> - #2568 `list_issues`: has lower integrity than agent
requires. The agent cannot read data with integrity below "merged".
> - #2566 `list_issues`: has lower integrity than agent
requires. The agent cannot read data with integrity below "merged".
> - #2565 `list_issues`: has lower integrity than agent
requires. The agent cannot read data with integrity below "merged".
> - #2563 `list_issues`: has lower integrity than agent
requires. The agent cannot read data with integrity below "merged".
> - #2560 `list_issues`: has lower integrity than agent
requires. The agent cannot read data with integrity below "merged".
> - #2559 `list_issues`: has lower integrity than agent
requires. The agent cannot read data with integrity below "merged".
> - #2558 `list_issues`: has lower integrity than agent
requires. The agent cannot read data with integrity below "merged".
> - #2557 `list_issues`: has lower integrity than agent
requires. The agent cannot read data with integrity below "merged".
> - #2554 `list_issues`: has lower integrity than agent
requires. The agent cannot read data with integrity below "merged".
> - #1711 `list_issues`: has lower integrity than agent
requires. The agent cannot read data with integrity below "merged".
> - search_issues `search_issues`: has lower integrity than agent
requires. The agent cannot read data with integrity below "merged".
> - [#2278](#2278)
`issue_read`: has lower integrity than agent requires. The agent cannot
read data with integrity below "merged".
>
> To allow these resources, lower `min-integrity` in your GitHub
frontmatter:
>
> ```yaml
> tools:
>   github:
>     min-integrity: approved  # merged | approved | unapproved | none
> ```
>
> </details>


> Generated by [Repo
Assist](https://github.com/github/gh-aw-mcpg/actions/runs/23583377899) ·
[◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw-mcpg+%22gh-aw-workflow-id%3A+repo-assist%22&type=pullrequests)
>
> To install this [agentic
workflow](https://github.com/githubnext/agentics/tree/851905c06e905bf362a9f6cc54f912e3df747d55/workflows/repo-assist.md),
run
> ```
> gh aw add
githubnext/agentics@851905c
> ```

<!-- gh-aw-agentic-workflow: Repo Assist, engine: copilot, model: auto,
id: 23583377899, workflow_id: repo-assist, run:
https://github.com/github/gh-aw-mcpg/actions/runs/23583377899 -->

<!-- gh-aw-workflow-id: repo-assist -->
@lpcox lpcox closed this Mar 26, 2026
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.

[duplicate-code] Duplicate Code Pattern: AgentLabels bulk label operations missing shared helper

3 participants