Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 32 additions & 59 deletions .github/workflows/smoke-claude.lock.yml

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion .github/workflows/smoke-claude.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ runtimes:
checkout:
fetch: ["*"]
fetch-depth: 0
force-clean-git-credentials: true
safe-outputs:
allowed-domains: [default-safe-outputs]
add-comment:
Expand Down
4 changes: 2 additions & 2 deletions actions/setup/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ This action runs in all workflow jobs to provide scripts that can be used instea

The action copies:
- 226 `.cjs` JavaScript files from the `js/` directory
- 8 `.sh` shell scripts from the `sh/` directory
- 7 `.sh` shell scripts from the `sh/` directory

All files are copied to the destination directory (default: `${{ runner.temp }}/gh-aw/actions`). These files are generated by running `make actions-build` and are committed to the repository.

Expand All @@ -35,7 +35,7 @@ Default: `${{ runner.temp }}/gh-aw/actions`

### `files-copied`

The number of files copied to the destination directory (should be 234: 226 JavaScript files + 8 shell scripts).
The number of files copied to the destination directory (should be 233: 226 JavaScript files + 7 shell scripts).

## Example

Expand Down
7 changes: 2 additions & 5 deletions actions/setup/sh/clean_git_credentials.sh
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,12 @@ clean_git_config() {
# Get the workspace directory (defaults to current GITHUB_WORKSPACE)
WORKSPACE="${GITHUB_WORKSPACE:-.}"

# Collect all git config files to clean from workspace and /tmp/.
# Includes both repository configs and submodule configs under .git/modules/**/config.
# Use maxdepth 15 as a conservative cap to cover deeply nested submodules
# (.git/modules/<a>/modules/<b>/...) without unbounded traversal.
# Collect all .git/config files to clean from workspace and /tmp/
CLEANED=0
while IFS= read -r git_config; do
clean_git_config "${git_config}"
CLEANED=$((CLEANED + 1))
done < <(find "${WORKSPACE}" /tmp -maxdepth 15 -type f -name "config" \( -path "*/.git/config" -o -path "*/.git/modules/*/config" \) 2>/dev/null | sort -u)
done < <(find "${WORKSPACE}" /tmp -maxdepth 10 -name "config" -path "*/.git/config" 2>/dev/null | sort -u)

if [ "${CLEANED}" -eq 0 ]; then
echo "No .git/config files found, nothing to clean"
Expand Down
39 changes: 0 additions & 39 deletions actions/setup/sh/clean_git_credentials_checkout.sh

This file was deleted.

23 changes: 0 additions & 23 deletions actions/setup/sh/clean_git_credentials_pre_setup.sh

This file was deleted.

30 changes: 0 additions & 30 deletions actions/setup/sh/clean_git_credentials_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -144,36 +144,6 @@ assert "config still valid" "git config --file '${REPO}/.git/config' --list >/de
assert "core settings intact" "git config --file '${REPO}/.git/config' core.repositoryformatversion >/dev/null 2>&1"
echo ""

# ── Test 8: Cleans submodule git configs under .git/modules ─────────────────
echo "Test 8: Cleans submodule git config files"
REPO="${TEST_WORKSPACE}/repo8"
mkdir -p "${REPO}/.git/modules/sub" "${REPO}/.git/modules/sub/modules/nested"
cat >"${REPO}/.git/modules/sub/config" <<'EOF'
[core]
repositoryformatversion = 0
[credential "https://github.com"]
helper = store
[http "https://github.com/"]
extraheader = Authorization: Basic abc123
[remote "origin"]
url = https://x-access-token:ghs_sub@github.com/org/sub.git
EOF
cat >"${REPO}/.git/modules/sub/modules/nested/config" <<'EOF'
[core]
repositoryformatversion = 0
[credential]
helper = store
[http]
extraheader = Authorization: Basic def456
EOF
GITHUB_WORKSPACE="${REPO}" bash "${CLEAN_SCRIPT}" >/dev/null 2>&1
assert "submodule credential section removed" "! grep -q '\[credential' '${REPO}/.git/modules/sub/config'"
assert "submodule url extraheader removed" "! git config --file '${REPO}/.git/modules/sub/config' 'http.https://github.com/.extraheader' 2>/dev/null"
assert "submodule remote URL credentials stripped" "[ \"\$(git config --file '${REPO}/.git/modules/sub/config' remote.origin.url)\" = 'https://github.com/org/sub.git' ]"
assert "nested submodule credential section removed" "! grep -q '\[credential' '${REPO}/.git/modules/sub/modules/nested/config'"
assert "nested submodule http.extraheader removed" "! git config --file '${REPO}/.git/modules/sub/modules/nested/config' http.extraheader 2>/dev/null"
echo ""

# ── Summary ──────────────────────────────────────────────────────────────────
echo "Tests passed: ${TESTS_PASSED}"
echo "Tests failed: ${TESTS_FAILED}"
Expand Down
2 changes: 1 addition & 1 deletion docs/adr/26666-add-pre-agent-steps-support.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ Users could approximate "run before engine" behaviour by placing steps in `post-
- Users can inject pre-execution logic (context preparation, validation, environment mutations) at exactly the right lifecycle point — after setup, before the agent starts.
- The new field mirrors the naming and import-merge semantics of `pre-steps` and `post-steps`, keeping the mental model consistent and the feature discoverable.
- Secret expression validation is automatically enforced on the new field using the existing `validateStepsSecrets` infrastructure, preserving the security model without new code.
- Integration tests cover placement order (after force-clean-git-credentials, before engine execution) and import merge ordering.
- Integration tests cover placement order (after clean-git-credentials, before engine execution) and import merge ordering.

#### Negative
- The workflow extension model now has three named step positions (`pre-steps`, `pre-agent-steps`, `post-steps`) plus the main `steps` field; the distinction between `pre-steps` and `pre-agent-steps` requires documentation to avoid confusion.
Expand Down
4 changes: 2 additions & 2 deletions pkg/cli/agent_download.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import (
"os"
"os/exec"
"strings"
"time"

"github.com/github/gh-aw/pkg/console"
"github.com/github/gh-aw/pkg/constants"
"github.com/github/gh-aw/pkg/logger"
"github.com/github/gh-aw/pkg/workflow"
)
Expand Down Expand Up @@ -42,7 +42,7 @@ func downloadAgentFileFromGitHub(verbose bool) (string, error) {

// Create HTTP client with timeout
client := &http.Client{
Timeout: constants.DefaultHTTPClientTimeout,
Timeout: 30 * time.Second,
}

// Download the file
Expand Down
4 changes: 2 additions & 2 deletions pkg/cli/deps_security.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import (
"os"
"sort"
"strings"
"time"

"github.com/github/gh-aw/pkg/console"
"github.com/github/gh-aw/pkg/constants"
"github.com/github/gh-aw/pkg/logger"
)

Expand Down Expand Up @@ -136,7 +136,7 @@ func querySecurityAdvisories(ctx context.Context, depVersions map[string]string,
url := "https://api.github.com/advisories?ecosystem=go&per_page=100"

depsSecurityLog.Printf("Querying GitHub Security Advisory API: url=%s, dep_count=%d", url, len(depVersions))
client := &http.Client{Timeout: constants.DefaultHTTPClientTimeout}
client := &http.Client{Timeout: 30 * time.Second}
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/logs_rate_limit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
// TestRateLimitResponseUnmarshal verifies that the rateLimitResponse struct correctly
// unmarshals the JSON returned by `gh api rate_limit`.
func TestRateLimitResponseUnmarshal(t *testing.T) {
now := time.Now().Add(time.Second * 30).Unix()
now := time.Now().Add(30 * time.Second).Unix()
raw := []byte(`{
"resources": {
"core": {
Expand Down
4 changes: 1 addition & 3 deletions pkg/cli/mcp_add_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import (
"strings"
"testing"
"time"

"github.com/github/gh-aw/pkg/constants"
)

// createMockMCPRegistry creates a mock MCP registry server for testing
Expand Down Expand Up @@ -240,7 +238,7 @@ This is a test workflow for MCP server integration.
addCmd.Dir = setup.tempDir

// Set a timeout for each server addition
timeout := constants.DefaultHTTPClientTimeout
timeout := 30 * time.Second
addCmd.Env = os.Environ()

done := make(chan error, 1)
Expand Down
3 changes: 2 additions & 1 deletion pkg/cli/mcp_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io"
"net/http"
"strings"
"time"

"github.com/github/gh-aw/pkg/console"
"github.com/github/gh-aw/pkg/constants"
Expand Down Expand Up @@ -46,7 +47,7 @@ func NewMCPRegistryClient(registryURL string) *MCPRegistryClient {
return &MCPRegistryClient{
registryURL: registryURL,
httpClient: &http.Client{
Timeout: constants.DefaultHTTPClientTimeout,
Timeout: 30 * time.Second,
},
}
}
Expand Down
3 changes: 0 additions & 3 deletions pkg/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,6 @@ const DefaultToolTimeout = 60 * time.Second
// DefaultMCPStartupTimeout is the default timeout for MCP server startup
const DefaultMCPStartupTimeout = 120 * time.Second

// DefaultHTTPClientTimeout is the default timeout for internal HTTP clients
const DefaultHTTPClientTimeout = 30 * time.Second

// DefaultMaxEffectiveTokens is the default ET budget enforced by the AWF API proxy.
const DefaultMaxEffectiveTokens int64 = 25000000

Expand Down
18 changes: 6 additions & 12 deletions pkg/constants/constants_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,26 +351,20 @@ func TestNumericConstants(t *testing.T) {
func TestTimeoutConstants(t *testing.T) {
// Test new time.Duration-based constants
tests := []struct {
name string
value time.Duration
minValue time.Duration
checkExact bool
exactValue time.Duration
name string
value time.Duration
minValue time.Duration
}{
{"DefaultAgenticWorkflowTimeout", DefaultAgenticWorkflowTimeout, 1 * time.Minute, false, 0},
{"DefaultToolTimeout", DefaultToolTimeout, 1 * time.Second, false, 0},
{"DefaultMCPStartupTimeout", DefaultMCPStartupTimeout, 1 * time.Second, false, 0},
{"DefaultHTTPClientTimeout", DefaultHTTPClientTimeout, 1 * time.Second, true, time.Second * 30},
{"DefaultAgenticWorkflowTimeout", DefaultAgenticWorkflowTimeout, 1 * time.Minute},
{"DefaultToolTimeout", DefaultToolTimeout, 1 * time.Second},
{"DefaultMCPStartupTimeout", DefaultMCPStartupTimeout, 1 * time.Second},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.value < tt.minValue {
t.Errorf("%s = %v, should be >= %v", tt.name, tt.value, tt.minValue)
}
if tt.checkExact && tt.value != tt.exactValue {
t.Errorf("%s = %v, want %v", tt.name, tt.value, tt.exactValue)
}
})
}
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/parser/remote_fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
pathpkg "path"
"path/filepath"
"strings"
"time"

"github.com/cli/go-gh/v2"
"github.com/cli/go-gh/v2/pkg/api"
Expand Down Expand Up @@ -519,7 +520,7 @@ func downloadFileViaRawURL(owner, repo, filePath, ref string) ([]byte, error) {
remoteLog.Printf("Attempting raw URL download: %s", rawURL)

// Use a client with a timeout to prevent indefinite hangs on slow/unresponsive hosts.
rawClient := &http.Client{Timeout: constants.DefaultHTTPClientTimeout}
rawClient := &http.Client{Timeout: 30 * time.Second}

// #nosec G107 -- rawURL is constructed from workflow import configuration authored by
// the developer; the owner, repo, filePath, and ref are user-supplied workflow spec fields.
Expand Down
5 changes: 0 additions & 5 deletions pkg/parser/schemas/main_workflow_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -11199,11 +11199,6 @@
"type": "boolean",
"description": "When true, clones the repository's wiki git instead of the regular repository. The effective repository becomes \"{repository}.wiki\" (e.g. \"owner/repo.wiki\"). Defaults to false.",
"examples": [true, false]
},
"force-clean-git-credentials": {
"type": "boolean",
"description": "When true, persist credentials during checkout, then immediately run a post-checkout cleanup step that removes credentials from root and submodule git configs. Useful for submodule-safe cleanup behavior.",
"examples": [true, false]
}
}
},
Expand Down
8 changes: 0 additions & 8 deletions pkg/workflow/checkout_config_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,14 +237,6 @@ func checkoutConfigFromMap(m map[string]any) (*CheckoutConfig, error) {
cfg.Wiki = b
}

if v, ok := m["force-clean-git-credentials"]; ok {
b, ok := v.(bool)
if !ok {
return nil, errors.New("checkout.force-clean-git-credentials must be a boolean")
}
cfg.CleanGitCredentials = b
}

return cfg, nil
}

Expand Down
10 changes: 0 additions & 10 deletions pkg/workflow/checkout_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,6 @@ type CheckoutConfig struct {
// When true, the effective repository becomes "{repository}.wiki" (e.g. "owner/repo.wiki").
// Defaults to false.
Wiki bool `json:"wiki,omitempty"`

// CleanGitCredentials keeps actions/checkout credential persistence enabled and
// injects a follow-up cleanup step that removes credentials from git config files
// (including submodule configs) without using git submodule foreach.
CleanGitCredentials bool `json:"force-clean-git-credentials,omitempty"`
}

// checkoutKey uniquely identifies a checkout target used for grouping/deduplication.
Expand All @@ -127,7 +122,6 @@ type resolvedCheckout struct {
lfs bool
current bool // true if this checkout is the logical current repository
fetchRefs []string // merged fetch ref patterns (see CheckoutConfig.Fetch)
cleanCreds bool // true enables persist-credentials + injected cleanup step
// wiki is intentionally not stored here; use entry.key.wiki instead.
}

Expand Down Expand Up @@ -265,9 +259,6 @@ func (cm *CheckoutManager) add(cfg *CheckoutConfig) {
if len(cfg.Fetch) > 0 {
entry.fetchRefs = mergeFetchRefs(entry.fetchRefs, cfg.Fetch)
}
if cfg.CleanGitCredentials {
entry.cleanCreds = true
}
checkoutManagerLog.Printf("Merged checkout for path=%q repository=%q", key.path, key.repository)
} else {
entry := &resolvedCheckout{
Expand All @@ -279,7 +270,6 @@ func (cm *CheckoutManager) add(cfg *CheckoutConfig) {
submodules: cfg.Submodules,
lfs: cfg.LFS,
current: cfg.Current,
cleanCreds: cfg.CleanGitCredentials,
}
if cfg.SparseCheckout != "" {
entry.sparsePatterns = mergeSparsePatterns(nil, cfg.SparseCheckout)
Expand Down
Loading