Skip to content

fix(harness): resolve URL base scripts relative to scaffold root #2613

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ralphbean merged 1 commit into from
Jun 24, 2026
Merged

fix(harness): resolve URL base scripts relative to scaffold root #2613

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 29 additions & 1 deletion internal/harness/compose.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,11 @@ func mergeBaseIntoChild(base, child *Harness) {
// because runtime treats it as a directory (uploaded recursively).
// Returns additional dependencies for the fetched scripts.
func resolveBaseScripts(ctx context.Context, base *Harness, baseURL string, allowlist []string, opts ComposeOpts) ([]Dependency, error) {
baseURLDir := urlDirPrefix(baseURL)
// Script paths in harness YAMLs are relative to the scaffold root (the
// parent of the harness/ directory), not the YAML file. Use
// urlParentDirPrefix to match the local resolution behavior where
// ResolveRelativeTo is called with absFullsendDir (the workspace root).
baseURLDir := urlParentDirPrefix(baseURL)
if baseURLDir == "" {
return nil, fmt.Errorf("cannot determine directory from base URL")
}
Expand Down Expand Up @@ -724,6 +728,30 @@ func urlDirPrefix(rawURL string) string {
return parsed.String()
}

// urlParentDirPrefix returns the parent of the directory containing the URL's
// file. Script paths in harness YAMLs are relative to the scaffold root (the
// parent of the harness/ directory), not the YAML file itself. This matches
// local resolution where ResolveRelativeTo uses absFullsendDir (the workspace
// root), which is the parent of the harness/ directory.
func urlParentDirPrefix(rawURL string) string {
cleanURL, _, _ := ParseIntegrityHash(rawURL)
parsed, err := url.Parse(cleanURL)
if err != nil {
return ""
}
dir := path.Dir(path.Dir(parsed.Path))

@waynesun09 waynesun09 Jun 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[MEDIUM] Hardcoded one-level parent assumption

path.Dir(path.Dir(parsed.Path)) assumes the harness YAML is always exactly one directory below the scaffold root. Today this holds (scaffold layout enforces it, and resolveHarnessPath in lock.go also hardcodes single-depth), so this is safe.

Non-blocking suggestion: consider a doc comment noting this constraint, or renaming to urlScaffoldRootPrefix to make the intent explicit.

if dir == "." || dir == "" {
return ""
}
if !strings.HasSuffix(dir, "/") {
dir += "/"
}
parsed.Path = dir
parsed.RawPath = ""
parsed.Fragment = ""
return parsed.String()
}

// urlIndexPath returns the path to the URL-to-hash index file.
func urlIndexPath(workspaceRoot string) string {
return filepath.Join(workspaceRoot, ".fullsend-cache", "url-index.json")
Expand Down
130 changes: 112 additions & 18 deletions internal/harness/compose_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1168,6 +1168,35 @@ func TestURLDirPrefix(t *testing.T) {
}
}

func TestURLParentDirPrefix(t *testing.T) {
tests := []struct {
input string
want string
}{
{
"https://raw.githubusercontent.com/org/repo/sha/harness/triage.yaml#sha256=abc123",
"https://raw.githubusercontent.com/org/repo/sha/",
},
{
"https://example.com/path/to/file.yaml",
"https://example.com/path/",
},
{
// File at domain root: parent of "/" is still "/"
"https://example.com/file.yaml#sha256=0000000000000000000000000000000000000000000000000000000000000000",
"https://example.com/",
},
{
"not-a-url",
"",
},
}
for _, tt := range tests {
got := urlParentDirPrefix(tt.input)
assert.Equal(t, tt.want, got, "urlParentDirPrefix(%q)", tt.input)
}
}

func setupScriptTestServer(t *testing.T, harnessContent []byte, scripts map[string][]byte) (*httptest.Server, fetch.FetchPolicy) {
t.Helper()
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -1205,9 +1234,10 @@ pre_script: scripts/pre.sh
post_script: scripts/post.sh
`)

// Scripts at /scripts/ (sibling to /harness/), matching real scaffold layout.
server, policy := setupScriptTestServer(t, baseContent, map[string][]byte{
"/harness/scripts/pre.sh": preScript,
"/harness/scripts/post.sh": postScript,
"/scripts/pre.sh": preScript,
"/scripts/post.sh": postScript,
})

hash := computeHash(baseContent)
Expand Down Expand Up @@ -1275,7 +1305,7 @@ validation_loop:
`)

server, policy := setupScriptTestServer(t, baseContent, map[string][]byte{
"/harness/scripts/validate.sh": validateScript,
"/scripts/validate.sh": validateScript,
})

hash := computeHash(baseContent)
Expand Down Expand Up @@ -1324,8 +1354,8 @@ forge:
`)

server, policy := setupScriptTestServer(t, baseContent, map[string][]byte{
"/harness/scripts/gh-pre.sh": forgePre,
"/harness/scripts/gh-post.sh": forgePost,
"/scripts/gh-pre.sh": forgePre,
"/scripts/gh-post.sh": forgePost,
})

hash := computeHash(baseContent)
Expand Down Expand Up @@ -1375,8 +1405,8 @@ post_script: scripts/base-post.sh
postScript := []byte("#!/bin/bash\necho base-post")

server, policy := setupScriptTestServer(t, baseContent, map[string][]byte{
"/harness/scripts/base-pre.sh": preScript,
"/harness/scripts/base-post.sh": postScript,
"/scripts/base-pre.sh": preScript,
"/scripts/base-post.sh": postScript,
})

hash := computeHash(baseContent)
Expand Down Expand Up @@ -1418,7 +1448,7 @@ pre_script: scripts/pre.sh
`)

server, policy := setupScriptTestServer(t, baseContent, map[string][]byte{
"/harness/scripts/pre.sh": []byte("#!/bin/bash"),
"/scripts/pre.sh": []byte("#!/bin/bash"),
})

hash := computeHash(baseContent)
Expand All @@ -1432,14 +1462,14 @@ role: test
base: `+baseURL+`
`)

// Allowlist only covers /harness/triage.yaml, not /harness/scripts/
// Allowlist only covers /harness/triage.yaml, not /scripts/
_, _, err := LoadWithBase(context.Background(), path, ComposeOpts{
WorkspaceRoot: cacheDir,
FetchPolicy: policy,
OrgAllowlist: []string{server.URL + "/harness/triage.yaml"},
})
// The allowlist check is prefix-based, so /harness/triage.yaml as prefix
// does NOT cover /harness/scripts/pre.sh
// does NOT cover /scripts/pre.sh
require.Error(t, err)
assert.Contains(t, err.Error(), "not in allowed_remote_resources")
}
Expand Down Expand Up @@ -1519,10 +1549,10 @@ pre_script: scripts/pre.sh
// Pre-populate base harness in cache
require.NoError(t, fetch.CachePut(cacheDir, "https://example.com/harness/triage.yaml", baseContent))
// Pre-populate script in cache
require.NoError(t, fetch.CachePut(cacheDir, "https://example.com/harness/scripts/pre.sh", preScript))
require.NoError(t, fetch.CachePut(cacheDir, "https://example.com/scripts/pre.sh", preScript))
// Add URL index entry
scriptHash := fetch.ComputeSHA256(preScript)
require.NoError(t, urlIndexPut(cacheDir, "https://example.com/harness/scripts/pre.sh", scriptHash))
require.NoError(t, urlIndexPut(cacheDir, "https://example.com/scripts/pre.sh", scriptHash))

baseURL := "https://example.com/harness/triage.yaml#sha256=" + hash

Expand Down Expand Up @@ -1560,7 +1590,7 @@ pre_script: scripts/pre.sh
`)

server, policy := setupScriptTestServer(t, baseContent, map[string][]byte{
"/harness/scripts/pre.sh": scriptContent,
"/scripts/pre.sh": scriptContent,
})

hash := computeHash(baseContent)
Expand Down Expand Up @@ -1629,7 +1659,7 @@ pre_script: scripts/pre.sh
`)

server, policy := setupScriptTestServer(t, baseContent, map[string][]byte{
"/harness/scripts/pre.sh": preScript,
"/scripts/pre.sh": preScript,
})

hash := computeHash(baseContent)
Expand Down Expand Up @@ -1676,7 +1706,7 @@ forge:
`)

server, policy := setupScriptTestServer(t, baseContent, map[string][]byte{
"/harness/scripts/gh-validate.sh": forgeValidate,
"/scripts/gh-validate.sh": forgeValidate,
})

hash := computeHash(baseContent)
Expand Down Expand Up @@ -1795,9 +1825,9 @@ validation_loop:
`)

server, policy := setupScriptTestServer(t, baseContent, map[string][]byte{
"/harness/scripts/pre.sh": preScript,
"/harness/scripts/post.sh": postScript,
"/harness/scripts/validate.sh": validateScript,
"/scripts/pre.sh": preScript,
"/scripts/post.sh": postScript,
"/scripts/validate.sh": validateScript,
})

hash := computeHash(baseContent)
Expand Down Expand Up @@ -1947,6 +1977,70 @@ func TestResolveBaseScripts_InvalidBaseURL(t *testing.T) {
assert.Contains(t, err.Error(), "cannot determine directory")
}

// TestLoadWithBase_URLBase_ScriptsRelativeToScaffoldRoot verifies that URL
// base script resolution matches local resolution: scripts are relative to
// the scaffold root (parent of harness/), not to the YAML file's directory.
// This mirrors the real scaffold layout where harness/ and scripts/ are siblings.
func TestLoadWithBase_URLBase_ScriptsRelativeToScaffoldRoot(t *testing.T) {
preScript := []byte("#!/bin/bash\necho pre")
postScript := []byte("#!/bin/bash\necho post")

baseContent := []byte(`
agent: agents/triage.md
role: test
model: opus
pre_script: scripts/pre.sh
post_script: scripts/post.sh
`)

// Mount scripts at /scripts/ (sibling to /harness/), matching real layout.
// The YAML lives at /harness/triage.yaml, so urlDirPrefix gives /harness/.
// Scripts should resolve relative to / (the scaffold root), not /harness/.
server, policy := setupScriptTestServer(t, baseContent, map[string][]byte{
"/scripts/pre.sh": preScript,
"/scripts/post.sh": postScript,
})

hash := computeHash(baseContent)
dir := t.TempDir()
cacheDir := filepath.Join(dir, "cache")

baseURL := server.URL + "/harness/triage.yaml#sha256=" + hash

path := writeTestHarness(t, dir, "child.yaml", `
agent: agents/child.md
role: test
base: `+baseURL+`
`)

h, deps, err := LoadWithBase(context.Background(), path, ComposeOpts{
WorkspaceRoot: cacheDir,
FetchPolicy: policy,
OrgAllowlist: []string{server.URL + "/"},
})
require.NoError(t, err)

assert.Equal(t, "agents/child.md", h.Agent)

// Scripts resolved to local cache paths
assert.NotEmpty(t, h.PreScript, "pre_script should be resolved")
assert.NotEmpty(t, h.PostScript, "post_script should be resolved")
assert.True(t, filepath.IsAbs(h.PreScript), "pre_script should be absolute cache path")
assert.True(t, filepath.IsAbs(h.PostScript), "post_script should be absolute cache path")

// Verify cached content matches
preContent, err := os.ReadFile(h.PreScript)
require.NoError(t, err)
assert.Equal(t, preScript, preContent)

postContent, err := os.ReadFile(h.PostScript)
require.NoError(t, err)
assert.Equal(t, postScript, postContent)

// Dependencies: 1 for base harness + 2 for scripts
require.Len(t, deps, 3)
}

func TestURLIndexPut_EmptyWorkspaceRoot(t *testing.T) {
err := urlIndexPut("", "https://example.com/script.sh", "abc123")
assert.NoError(t, err)
Expand Down
Loading