From aed136ff16649985482c8a08599ebdc8812683bc Mon Sep 17 00:00:00 2001 From: Samgu Lee Date: Thu, 22 Oct 2026 12:40:21 +0900 Subject: [PATCH] fix(init): make helper script executable --- .replworks/ARCHITECTURE.md | 4 +-- .replworks/PRODUCT_SPEC.md | 6 ++-- .replworks/TASKS.md | 13 ++++++++- internal/command/init_test.go | 32 ++++++++++++++++++++++ internal/command/sync_test.go | 35 ++++++++++++++++++++++++ internal/initproject/initproject.go | 9 ++++++ internal/initproject/initproject_test.go | 5 ++++ internal/syncproject/syncproject.go | 9 ++++++ internal/syncproject/syncproject_test.go | 5 ++++ internal/workspace/writable.go | 19 +++++++++++++ internal/workspace/writable_test.go | 20 ++++++++++++++ 11 files changed, 152 insertions(+), 5 deletions(-) diff --git a/.replworks/ARCHITECTURE.md b/.replworks/ARCHITECTURE.md index 66a9c60..263385b 100644 --- a/.replworks/ARCHITECTURE.md +++ b/.replworks/ARCHITECTURE.md @@ -86,7 +86,7 @@ Task Ledger Reader and Unverified Mark Scanner → Document Line Classifier - Make no write when required source text is `Failed`. - Process items in this order: AGENTS.md, then the `.replworks/` directory, then `.replworks/PRODUCT_SPEC.md`, `.replworks/TECH_STACK.md`, `.replworks/ARCHITECTURE.md`, `.replworks/TASKS.md`, then the helper-script state. - Mark an item `skipped` when it is present. Otherwise create it (AGENTS.md with the Canonical Text, the directory, and each document as an empty file). - - For standard selection, remove an existing helper script. For automatic-run selection, create the helper-script directory and file only when absent; never replace an existing helper script. + - For standard selection, remove an existing helper script. For automatic-run selection, create the helper-script directory and file only when absent; never replace an existing helper script, and ensure the helper script has execute permission. - Stop at the first write failure. - Inputs: Request with the selected document source, Canonical Text fact, optional Helper Script Text fact, and Workspace presence facts. - Outputs: Outcome with one `(item path, created | skipped)` entry per processed item, and execution errors. @@ -134,7 +134,7 @@ Task Ledger Reader and Unverified Mark Scanner → Document Line Classifier - Only after that, obtain the Canonical Text for the selected sync document. Record an execution error and stop when it is `Failed`. - Compare local text and Canonical Text for exact equality. - When equal: return `UpToDate` and make no write. - - When different: obtain the difference from the Difference Calculator, obtain Helper Script Text only for automatic-run selection, then replace AGENTS.md and adjust helper-script state. + - When different: obtain the difference from the Difference Calculator, obtain Helper Script Text only for automatic-run selection, then replace AGENTS.md and adjust helper-script state, including ensuring execute permission on the automatic-run helper script. - When replacement fails: keep the computed difference in the Outcome and record an execution error. - Inputs: Request with the selected document source, local AGENTS.md fact, Canonical Text fact, optional Helper Script Text fact, and helper-script presence fact. - Outputs: Outcome with `UpToDate | Replaced(difference) | ReplaceFailed(difference)`, and execution errors. diff --git a/.replworks/PRODUCT_SPEC.md b/.replworks/PRODUCT_SPEC.md index 3029291..9bcc302 100644 --- a/.replworks/PRODUCT_SPEC.md +++ b/.replworks/PRODUCT_SPEC.md @@ -122,13 +122,13 @@ - FR-001: repl init MUST present the user with exactly these choices, in this order: `https://www.repl.net/documents/agents.md` and `https://www.repl.net/documents/agents-auto.md`, then create AGENTS.md with the content of the selected document when AGENTS.md does not exist. - FR-002: repl init MUST create the .replworks/ directory in the project directory when it does not exist. -- FR-003: repl init MUST NOT modify an existing file except that selecting the standard AGENTS document MUST remove an existing `.replworks/scripts/ask-human.sh`. +- FR-003: repl init MUST NOT modify an existing file except that selecting the standard AGENTS document MUST remove an existing `.replworks/scripts/ask-human.sh`, and selecting the automatic-run document MAY add execute permission to that helper script. - FR-004: repl init MUST NOT create any file or directory other than AGENTS.md, .replworks/, the four documents defined in FR-036, and `.replworks/scripts/ask-human.sh` plus its parent directory when the automatic-run document is selected. - FR-005: repl init MUST print the path of each item it created. - FR-006: repl init MUST print the path of each item it skipped because the item already exists. - FR-007: repl init MUST exit with code 0 when it completes, including when it skipped items. - FR-036: repl init MUST create .replworks/PRODUCT_SPEC.md, .replworks/TECH_STACK.md, .replworks/ARCHITECTURE.md, and .replworks/TASKS.md as empty files when they do not exist. -- FR-037: When the automatic-run document is selected, repl init MUST create `.replworks/scripts/ask-human.sh` from the exact response text at `https://www.repl.net/scripts/ask-human.sh` when the file does not exist, and MUST leave an existing file unchanged. +- FR-037: When the automatic-run document is selected, repl init MUST create `.replworks/scripts/ask-human.sh` from the exact response text at `https://www.repl.net/scripts/ask-human.sh` when the file does not exist, and MUST preserve the content of an existing file while ensuring that it has execute permission. - FR-038: When the standard AGENTS document is selected, repl init MUST remove `.replworks/scripts/ask-human.sh` when it exists. ### status @@ -165,6 +165,8 @@ - FR-031: repl sync MUST replace the content of AGENTS.md with the selected document's content when the contents differ. - FR-039: When repl sync replaces AGENTS.md because its contents differ, selecting the automatic-run document MUST create the helper script from the exact script response when the script does not exist, and selecting the standard document MUST remove the helper script when it exists. - FR-040: When AGENTS.md content is identical to the selected document, repl sync MUST NOT create, modify, or remove the helper script. +- FR-041: When repl init creates or manages `.replworks/scripts/ask-human.sh` for the automatic-run document, the helper script MUST have execute permission. +- FR-042: When repl sync replaces AGENTS.md for the automatic-run document, the helper script MUST have execute permission whether it is created or already exists. - FR-035: repl sync MUST render removed difference lines in red and added difference lines in green when its output is a terminal, and MUST render plain difference lines otherwise. ### all commands diff --git a/.replworks/TASKS.md b/.replworks/TASKS.md index 869b332..bc69ed9 100644 --- a/.replworks/TASKS.md +++ b/.replworks/TASKS.md @@ -281,8 +281,19 @@ - External boundary: yes - Acceptance criteria: - The live script source at `https://www.repl.net/scripts/ask-human.sh` is observed and its response is stored byte-for-byte for automatic-run initialization and synchronization. - - Automatic-run init creates `.replworks/scripts/ask-human.sh` when absent and leaves an existing helper script unchanged. + - Automatic-run init creates `.replworks/scripts/ask-human.sh` when absent and preserves the content of an existing helper script. - Standard init removes an existing helper script and leaves AGENTS.md unchanged when it already exists. - Sync adjusts helper-script state only when AGENTS.md differs from the selected document; equal content performs no helper-script network request or file operation. - A failed helper-script retrieval prevents writes to the project for init and prevents AGENTS.md replacement for sync. - Unit tests cover helper-source retrieval, creation, deletion, skip behavior, and equal-content no-op behavior. + +- [X] T-027 Ensure ask-human.sh is executable + - Satisfies: PRODUCT_SPEC.md § Functional Requirements / init (FR-003, FR-037, FR-041); § Functional Requirements / sync (FR-040, FR-042); ARCHITECTURE.md § Components (C2, C5, C6) + - External boundary: no + - Acceptance criteria: + - Automatic-run init gives a newly created `.replworks/scripts/ask-human.sh` execute permission. + - Automatic-run init adds execute permission to an existing helper script without changing its content. + - Automatic-run sync gives a newly created helper script execute permission. + - Automatic-run sync adds execute permission to an existing helper script when AGENTS.md is replaced without changing its content. + - Sync with identical AGENTS.md does not modify the helper script. + - Unit and command-level tests pass. diff --git a/internal/command/init_test.go b/internal/command/init_test.go index 4e0dabb..0c0ef07 100644 --- a/internal/command/init_test.go +++ b/internal/command/init_test.go @@ -161,6 +161,13 @@ func TestInitUsesTheAutomaticDocumentWhenSelected(t *testing.T) { if content, err := os.ReadFile(filepath.Join(dir, ".replworks/scripts/ask-human.sh")); err != nil || string(content) != want { t.Fatalf("ask-human.sh = %q, err = %v, want %q", content, err, want) } + info, err := os.Stat(filepath.Join(dir, ".replworks/scripts/ask-human.sh")) + if err != nil { + t.Fatalf("stat ask-human.sh: %v", err) + } + if info.Mode().Perm()&0o111 == 0 { + t.Fatalf("ask-human.sh permissions = %o, want an executable file", info.Mode().Perm()) + } } func TestInitRepeatsAfterAnInvalidDocumentSelection(t *testing.T) { @@ -193,6 +200,31 @@ func TestInitRepeatsAfterAnInvalidDocumentSelection(t *testing.T) { } } +func TestInitMakesAnExistingAutomaticScriptExecutable(t *testing.T) { + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, ".replworks/scripts"), 0o755); err != nil { + t.Fatalf("setup: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, ".replworks/scripts/ask-human.sh"), []byte("script\n"), 0o640); err != nil { + t.Fatalf("setup: %v", err) + } + t.Chdir(dir) + + client, _ := servePage(t, "# AGENTS.md\n") + code, _, stderr := runCommandWithInput(t, client, "2\n", "init") + + if code != 0 || stderr != "" { + t.Fatalf("code = %d, stderr = %q, want success", code, stderr) + } + info, err := os.Stat(filepath.Join(dir, ".replworks/scripts/ask-human.sh")) + if err != nil { + t.Fatalf("stat ask-human.sh: %v", err) + } + if info.Mode().Perm()&0o111 == 0 { + t.Fatalf("ask-human.sh permissions = %o, want an executable file", info.Mode().Perm()) + } +} + func TestInitRejectsMissingDocumentSelectionWithoutNetworkOrWrites(t *testing.T) { dir := t.TempDir() t.Chdir(dir) diff --git a/internal/command/sync_test.go b/internal/command/sync_test.go index 5437dd8..ab49e12 100644 --- a/internal/command/sync_test.go +++ b/internal/command/sync_test.go @@ -97,6 +97,13 @@ func TestSyncUsesTheAutomaticDocumentWhenSelected(t *testing.T) { if content, err := os.ReadFile(filepath.Join(dir, ".replworks/scripts/ask-human.sh")); err != nil || string(content) != want { t.Fatalf("ask-human.sh = %q, err = %v, want %q", content, err, want) } + info, err := os.Stat(filepath.Join(dir, ".replworks/scripts/ask-human.sh")) + if err != nil { + t.Fatalf("stat ask-human.sh: %v", err) + } + if info.Mode().Perm()&0o111 == 0 { + t.Fatalf("ask-human.sh permissions = %o, want an executable file", info.Mode().Perm()) + } } func TestSyncRepeatsAfterAnInvalidDocumentSelection(t *testing.T) { @@ -122,6 +129,34 @@ func TestSyncRepeatsAfterAnInvalidDocumentSelection(t *testing.T) { } } +func TestSyncMakesAnExistingAutomaticScriptExecutableWhenReplacingAgents(t *testing.T) { + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, ".replworks/scripts"), 0o755); err != nil { + t.Fatalf("setup: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, "AGENTS.md"), []byte("# stale\n"), 0o644); err != nil { + t.Fatalf("setup: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, ".replworks/scripts/ask-human.sh"), []byte("script\n"), 0o640); err != nil { + t.Fatalf("setup: %v", err) + } + t.Chdir(dir) + + client, _ := servePage(t, "# AGENTS.md\n") + code, _, stderr := runCommandWithInput(t, client, "2\n", "sync") + + if code != 0 || stderr != "" { + t.Fatalf("code = %d, stderr = %q, want success", code, stderr) + } + info, err := os.Stat(filepath.Join(dir, ".replworks/scripts/ask-human.sh")) + if err != nil { + t.Fatalf("stat ask-human.sh: %v", err) + } + if info.Mode().Perm()&0o111 == 0 { + t.Fatalf("ask-human.sh permissions = %o, want an executable file", info.Mode().Perm()) + } +} + func TestSyncFailsWithoutASelectionBeforeNetworkOrWrite(t *testing.T) { dir := t.TempDir() local := "# stale\n" diff --git a/internal/initproject/initproject.go b/internal/initproject/initproject.go index 83f2b8e..635a5a0 100644 --- a/internal/initproject/initproject.go +++ b/internal/initproject/initproject.go @@ -37,6 +37,7 @@ type CanonicalSource interface { type Writer interface { Exists(rel string) fact.Fact[bool] CreateFile(rel, content string) error + MakeExecutable(rel string) error CreateDirectory(rel string) error DeleteFile(rel string) error } @@ -145,6 +146,10 @@ func (c Coordinator) Run() Outcome { } if scriptExists { + if err := c.Workspace.MakeExecutable(askHumanPath); err != nil { + outcome.Errors = append(outcome.Errors, err) + return outcome + } outcome.Items = append(outcome.Items, Item{Path: askHumanPath}) return outcome } @@ -152,6 +157,10 @@ func (c Coordinator) Run() Outcome { outcome.Errors = append(outcome.Errors, err) return outcome } + if err := c.Workspace.MakeExecutable(askHumanPath); err != nil { + outcome.Errors = append(outcome.Errors, err) + return outcome + } outcome.Items = append(outcome.Items, Item{Path: askHumanPath, Created: true}) return outcome diff --git a/internal/initproject/initproject_test.go b/internal/initproject/initproject_test.go index ec338e1..a95b8ce 100644 --- a/internal/initproject/initproject_test.go +++ b/internal/initproject/initproject_test.go @@ -64,6 +64,11 @@ func (f *fakeWriter) CreateFile(rel, content string) error { return nil } +func (f *fakeWriter) MakeExecutable(rel string) error { + f.events = append(f.events, "make-executable "+rel) + return nil +} + func (f *fakeWriter) CreateDirectory(rel string) error { f.events = append(f.events, "create-directory "+rel) if f.createDirErr != nil { diff --git a/internal/syncproject/syncproject.go b/internal/syncproject/syncproject.go index 1718fda..306a253 100644 --- a/internal/syncproject/syncproject.go +++ b/internal/syncproject/syncproject.go @@ -27,6 +27,7 @@ type Workspace interface { Exists(rel string) fact.Fact[bool] CreateDirectory(rel string) error CreateFile(rel, content string) error + MakeExecutable(rel string) error ReplaceFile(rel, content string) error DeleteFile(rel string) error } @@ -108,6 +109,11 @@ func (c Coordinator) Run() Outcome { if c.ManageScript && c.Auto && !scriptExists { return c.ensureScript(difference, scriptContent) } + if c.ManageScript && c.Auto { + if err := c.Workspace.MakeExecutable(askHumanPath); err != nil { + return Outcome{Difference: difference, Errors: []error{err}} + } + } return Outcome{Difference: difference} } @@ -142,6 +148,9 @@ func (c Coordinator) ensureScript(difference []diff.Entry, content string) Outco if err := c.Workspace.CreateFile(askHumanPath, content); err != nil { return Outcome{Difference: difference, Errors: []error{err}} } + if err := c.Workspace.MakeExecutable(askHumanPath); err != nil { + return Outcome{Difference: difference, Errors: []error{err}} + } return Outcome{Difference: difference, ScriptPath: askHumanPath, ScriptState: "created"} } diff --git a/internal/syncproject/syncproject_test.go b/internal/syncproject/syncproject_test.go index 487df0d..814dafc 100644 --- a/internal/syncproject/syncproject_test.go +++ b/internal/syncproject/syncproject_test.go @@ -49,6 +49,11 @@ func (f *fakeWorkspace) CreateFile(rel, content string) error { return nil } +func (f *fakeWorkspace) MakeExecutable(rel string) error { + f.events = append(f.events, "make-executable "+rel) + return nil +} + func (f *fakeWorkspace) DeleteFile(rel string) error { f.events = append(f.events, "delete-file "+rel) return nil diff --git a/internal/workspace/writable.go b/internal/workspace/writable.go index d0df2df..ca88143 100644 --- a/internal/workspace/writable.go +++ b/internal/workspace/writable.go @@ -38,6 +38,25 @@ func (w Writable) CreateFile(rel, content string) error { return nil } +// MakeExecutable adds execute permission bits to rel while preserving its +// existing read and write permission bits. +func (w Writable) MakeExecutable(rel string) error { + abs, err := w.resolve(rel) + if err != nil { + return err + } + + info, err := os.Stat(abs) + if err != nil { + return fmt.Errorf("checking %s: %w", rel, err) + } + if err := os.Chmod(abs, info.Mode().Perm()|0o111); err != nil { + return fmt.Errorf("making %s executable: %w", rel, err) + } + + return nil +} + // CreateDirectory creates the directory rel. func (w Writable) CreateDirectory(rel string) error { abs, err := w.resolve(rel) diff --git a/internal/workspace/writable_test.go b/internal/workspace/writable_test.go index 178bd3b..b7c51d8 100644 --- a/internal/workspace/writable_test.go +++ b/internal/workspace/writable_test.go @@ -66,6 +66,26 @@ func TestCreateFileFailsWhenTheFileExists(t *testing.T) { } } +func TestMakeExecutablePreservesExistingPermissions(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "ask-human.sh") + if err := os.WriteFile(path, []byte("script\n"), 0o640); err != nil { + t.Fatalf("setup: %v", err) + } + + if err := GrantWrite(New(dir)).MakeExecutable("ask-human.sh"); err != nil { + t.Fatalf("MakeExecutable() error = %v, want nil", err) + } + + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat: %v", err) + } + if got, want := info.Mode().Perm(), os.FileMode(0o751); got != want { + t.Fatalf("permissions = %o, want %o", got, want) + } +} + func TestCreateDirectory(t *testing.T) { dir := t.TempDir()