Skip to content
Merged
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
9 changes: 7 additions & 2 deletions Taskfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,10 @@ tasks:
MIN_COVERAGE: '{{.MIN_COVERAGE | default "72"}}'
cmds:
- |
test -f profile.cov || go test -coverprofile=profile.cov ./cmd/forst/... ./internal/... -count=1
if [ ! -f profile.cov ]; then
PKGS=$(go list ./cmd/forst/... ./internal/... ./nodert/... | paste -sd, -)
go test -coverprofile=profile.cov -coverpkg="$PKGS" ./cmd/forst/... ./internal/... ./nodert/... -count=1
fi
Comment on lines +826 to +829

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Files matching Taskfile.yml:\n'
fd -a '^Taskfile\.ya?ml$' . | sed 's#^\./##' | sort

printf '\nRelevant lines around coverage tasks:\n'
if [ -f Taskfile.yml ]; then
  sed -n '800,860p' Taskfile.yml | cat -n | sed 's/^/Taskfile.yml:/'
fi

printf '\nTaskfile shell/settings occurrences:\n'
rg -n 'set:|pipefail|coverpkg|check:coverage|ci:test|tasks:' Taskfile.yml || true

printf '\nShell parser behavior probe if mvdan/sh is available in source/docs:\n'
rg -n 'pipefail|mvdan/sh|POSIX shell|shell interpreter' -S . --glob '*/*.md' --glob '*.md' --glob 'Taskfile.*' || true

printf '\nGo/test coverage pattern occurrences:\n'
rg -n 'go list .*|coverpkg|profile\.cov|coverage_summary\.sh' . --glob '*.yml' --glob '*.yaml' --glob '*.sh' || true

Repository: forst-lang/forst

Length of output: 6412


🌐 Web query:

Taskfile go-task mvdan sh pipefail default shell pipe behavior set errexit

💡 Result:

In Task, shell behavior regarding error handling and pipelines is managed through set: and shopt: directives within the Taskfile [1]. By default, Task does not enable errexit or pipefail for commands [1]. Key behaviors and configurations: 1. Pipe Behavior and Pipefail: By default, shell pipelines (e.g., cmd1 | cmd2) in Task behave like standard POSIX shells, where the exit status of the pipeline is determined by the last command [2]. If you want a pipeline to fail if any command in the sequence fails, you must enable pipefail [1]. This can be configured at the task level or globally by adding it to the set: directive [1]: yaml tasks: my-task: set: [pipefail] cmds: - false | true # This will now cause the task to fail 2. Errexit (set -e): Similarly, errexit is not enabled by default. You can enable it via the set: directive (e.g., set: [errexit]) to ensure the task stops immediately if a command returns a non-zero exit status [1]. Note that Task automatically applies set -e behavior to individual commands to prevent issues where multiline string commands might otherwise ignore failures in preceding lines [3]. 3. Scope of Shell Options: - These settings are generally applied to cmd/cmds execution [4][1]. - Known limitations exist where these set: options do not apply to dynamically expanded variables (e.g., sh: expressions) [5] or status: command checks [4]. - If using shopt: for bash-specific options, ensure they are placed correctly; placing pipefail in shopt: instead of set: is an error [6][7]. 4. Troubleshooting: If shell options appear not to work, ensure you are using the correct directive (set: for POSIX options like pipefail, errexit, nounset; shopt: for bash-specific options) [1][6]. Task now includes validation that will warn you if you attempt to use an invalid or misplaced option [7]. For complex pipelines that need specific failure handling, you can also explicitly include set -o pipefail within the command string itself [2][8].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Taskfile root and relevant task headers:\n'
sed -n '1,120p' Taskfile.yml | cat -n
sed -n '816,849p' Taskfile.yml | cat -n

printf '\nCheck if pipefail/errexit appears in the exact command string or task settings:\n'
awk '
  NR==816,NR==849 {
    $0 = NR ":" $0
    match($0, /set:|errexit|pipefail|go list/mvdan/sh)
  }
' Taskfile.yml || true

printf '\nInspect Taskfile task-level options around ci:test and check:coverage:\n'
python3 - <<'PY'
from pathlib import Path
p=Path('Taskfile.yml')
lines=p.read_text().splitlines()
for start,end,name in [(816,836,'check:coverage'),(838,848,'ci:test')]:
    print(f'\n{name}: lines {start}-{end}')
    for i in range(start, min(end+1, len(lines)+1)):
        print(f'{i}: {lines[i-1]}')
PY

Repository: forst-lang/forst

Length of output: 6969


Enable pipefail for the go list | paste commands.

check:coverage has no shell options, and ci:test only declares set: [errexit]. Since Task pipelines use the last command exit status by default, a failing go list ./cmd/forst/... ./internal/... ./nodert/... can be masked by paste; PKGS would still be accepted and passed to -coverpkg. Add set: [errexit, pipefail] or include set -o pipefail before the command.

Also applies to: 845-847

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Taskfile.yml` around lines 826 - 829, Enable pipefail for the shell commands
in the check:coverage and ci:test task definitions so failures from go list in
the go list | paste pipeline propagate instead of being masked by paste. Update
the relevant task set options to include both errexit and pipefail, or
explicitly enable pipefail before each pipeline.

- bash scripts/check_coverage_threshold.sh profile.cov {{.MIN_COVERAGE}}

test:go:short:
Expand All @@ -839,7 +842,9 @@ tasks:
deps: [build:node-runtime]
cmds:
- task: build:vscode
- go test -race -covermode atomic -coverprofile=profile.cov -timeout=10m ./cmd/forst/... ./internal/... ./nodert/...
- |
PKGS=$(go list ./cmd/forst/... ./internal/... ./nodert/... | paste -sd, -)
go test -race -covermode atomic -coverpkg="$PKGS" -coverprofile=profile.cov -timeout=10m ./cmd/forst/... ./internal/... ./nodert/...
Comment on lines +845 to +847

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Package-list computation for -coverpkg is duplicated across three sites. The same go list ./cmd/forst/... ./internal/... ./nodert/... | paste -sd, - snippet is repeated verbatim; forst/scripts/coverage_summary.sh's header comment explicitly states it should mirror CI's ci:test, so any future glob change (e.g. a new module) risks silently diverging between copies.

  • Taskfile.yml#L845-L847: canonical ci:test definition — consider extracting the package-glob list into a single Taskfile var (e.g. COVERAGE_PKGS) referenced by both check:coverage and ci:test.
  • Taskfile.yml#L826-L829: reuse the same extracted variable instead of recomputing PKGS independently.
  • forst/scripts/coverage_summary.sh#L7-L8: reuse the same package-glob list (e.g. via a shared env var/script, or by generating this script's glob list from the Taskfile var) so CI and local coverage stay in sync automatically.
📍 Affects 2 files
  • Taskfile.yml#L845-L847 (this comment)
  • Taskfile.yml#L826-L829
  • forst/scripts/coverage_summary.sh#L7-L8
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Taskfile.yml` around lines 845 - 847, Extract the shared coverage package
glob into one canonical variable or script, then update Taskfile.yml lines
845-847 (ci:test) and 826-829 (check:coverage) to reuse it instead of
recomputing PKGS. Also update forst/scripts/coverage_summary.sh lines 7-8 to
consume the same shared source, keeping all coverage commands synchronized when
package globs change.


ci:e2e:
desc: Run CI E2E suite (runtime examples, node interop, sidecar, providers)
Expand Down
7 changes: 4 additions & 3 deletions forst/cmd/forst/lsp/health.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ func (s *LSPServer) handleHealth(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)

meta := buildMetadataSnapshot()
response := map[string]any{
"status": "healthy",
"service": "forst-lsp",
"version": Version,
"commit": Commit,
"date": Date,
"version": meta.version,
"commit": meta.commit,
"date": meta.date,
"timestamp": time.Now().UTC().Format(time.RFC3339),
}

Expand Down
8 changes: 5 additions & 3 deletions forst/cmd/forst/lsp/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,18 @@ func (s *LSPServer) handleInitialize(request LSPRequest) LSPServerResponse {
},
}

meta := buildMetadataSnapshot()

return LSPServerResponse{
JSONRPC: "2.0",
ID: request.ID,
Result: map[string]any{
"capabilities": capabilities,
"serverInfo": map[string]any{
"name": "forst-lsp",
"version": Version,
"commit": Commit,
"date": Date,
"version": meta.version,
"commit": meta.commit,
"date": meta.date,
},
},
}
Expand Down
10 changes: 0 additions & 10 deletions forst/cmd/forst/lsp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,16 +76,6 @@ type peerAnalysisCacheEntry struct {
ctx *forstDocumentContext
}

// Version information for LSP server
var (
// Version is the current version of Forst
Version = "dev"
// Commit is the git commit hash
Commit = "unknown"
// Date is the build date
Date = "unknown"
)

// NewLSPServer creates a new LSP server
func NewLSPServer(port string, log *logrus.Logger) *LSPServer {
debugger := NewCompilerDebugger(true)
Expand Down
47 changes: 41 additions & 6 deletions forst/cmd/forst/lsp/version.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,52 @@
package lsp

import "github.com/sirupsen/logrus"
import (
"sync"

"github.com/sirupsen/logrus"
)

type buildMetadata struct {
version string
commit string
date string
}

var (
buildInfoMu sync.RWMutex
buildInfo = buildMetadata{
version: "dev",
commit: "unknown",
date: "unknown",
}
)

// SetBuildMetadata sets injected compiler build metadata (version, commit, date).
func SetBuildMetadata(version, commit, date string) {
buildInfoMu.Lock()
buildInfo = buildMetadata{version: version, commit: commit, date: date}
buildInfoMu.Unlock()
}

func buildMetadataSnapshot() buildMetadata {
buildInfoMu.RLock()
defer buildInfoMu.RUnlock()
return buildInfo
}

// BuildInfo returns injected compiler build metadata (version, commit, date).
func BuildInfo() (version, commit, date string) {
return Version, Commit, Date
meta := buildMetadataSnapshot()
return meta.version, meta.commit, meta.date
}

// BuildInfoMap returns build metadata as a JSON-friendly map.
func BuildInfoMap() map[string]string {
meta := buildMetadataSnapshot()
return map[string]string{
"version": Version,
"commit": Commit,
"date": Date,
"version": meta.version,
"commit": meta.commit,
"date": meta.date,
}
}

Expand All @@ -21,5 +55,6 @@ func LogBuildInfo(log *logrus.Logger) {
if log == nil {
return
}
log.Infof("forst %s %s %s", Version, Commit, Date)
meta := buildMetadataSnapshot()
log.Infof("forst %s %s %s", meta.version, meta.commit, meta.date)
}
18 changes: 6 additions & 12 deletions forst/cmd/forst/lsp/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,10 @@ import (

func TestLogBuildInfo_logsVersionCommitDate(t *testing.T) {
t.Parallel()
Version = "1.2.3"
Commit = "abc123"
Date = "2026-07-08"
origVersion, origCommit, origDate := BuildInfo()
SetBuildMetadata("1.2.3", "abc123", "2026-07-08")
t.Cleanup(func() {
Version = "dev"
Commit = "unknown"
Date = "unknown"
SetBuildMetadata(origVersion, origCommit, origDate)
Comment on lines +13 to +16

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Remove parallel execution from tests that replace global metadata.

SetBuildMetadata locks each call, but it does not isolate the read-set-assert-restore sequence. Either test can overwrite the other test’s metadata before its assertion. Cleanup can also restore a stale value.

  • forst/cmd/forst/lsp/version_test.go#L13-L16: remove t.Parallel() from TestLogBuildInfo_logsVersionCommitDate.
  • forst/cmd/forst/lsp/version_test.go#L35-L38: remove t.Parallel() from TestBuildInfoMap.

As per coding guidelines, tests must be “precise, reproducing unit or integration tests.”

📍 Affects 1 file
  • forst/cmd/forst/lsp/version_test.go#L13-L16 (this comment)
  • forst/cmd/forst/lsp/version_test.go#L35-L38
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@forst/cmd/forst/lsp/version_test.go` around lines 13 - 16, Remove
t.Parallel() from TestLogBuildInfo_logsVersionCommitDate and TestBuildInfoMap in
forst/cmd/forst/lsp/version_test.go (lines 13-16 and 35-38). Keep both tests
sequential because they replace and restore shared build metadata via
SetBuildMetadata.

Source: Coding guidelines

})

var buf bytes.Buffer
Expand All @@ -35,13 +32,10 @@ func TestLogBuildInfo_logsVersionCommitDate(t *testing.T) {

func TestBuildInfoMap(t *testing.T) {
t.Parallel()
Version = "v"
Commit = "c"
Date = "d"
origVersion, origCommit, origDate := BuildInfo()
SetBuildMetadata("v", "c", "d")
t.Cleanup(func() {
Version = "dev"
Commit = "unknown"
Date = "unknown"
SetBuildMetadata(origVersion, origCommit, origDate)
})
m := BuildInfoMap()
if m["version"] != "v" || m["commit"] != "c" || m["date"] != "d" {
Expand Down
8 changes: 2 additions & 6 deletions forst/cmd/forst/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,7 @@ func runMain(argv []string) int {
setLogLevel(log, *logLevel)

// Set version information in LSP package
lsp.Version = Version
lsp.Commit = Commit
lsp.Date = Date
lsp.SetBuildMetadata(Version, Commit, Date)

if err := startLSPFunc(*port, log); err != nil {
return 1
Expand Down Expand Up @@ -176,9 +174,7 @@ func runMain(argv []string) int {
}

// Set version information in LSP package
lsp.Version = Version
lsp.Commit = Commit
lsp.Date = Date
lsp.SetBuildMetadata(Version, Commit, Date)

if err := handleDumpCommand(*filePath, *compression, *format, *phase, *summary, log); err != nil {
log.Error(err)
Expand Down
24 changes: 7 additions & 17 deletions forst/cmd/forst/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"encoding/json"
"fmt"
"forst/cmd/forst/lsp"
"forst/internal/compiler"
"forst/internal/ftconfig"
"io"
Expand Down Expand Up @@ -329,25 +330,14 @@ func TestCompilerArgsParsing(t *testing.T) {

func TestLSPVersionInjection(t *testing.T) {
// Test that version information is correctly injected into LSP package
originalVersion := Version
originalCommit := Commit
originalDate := Date
defer func() {
Version = originalVersion
Commit = originalCommit
Date = originalDate
}()
origVersion, origCommit, origDate := lsp.BuildInfo()
t.Cleanup(func() {
lsp.SetBuildMetadata(origVersion, origCommit, origDate)
})

// Set test values
Version = "test-version"
Commit = "test-commit"
Date = "test-date"
lsp.SetBuildMetadata("test-version", "test-commit", "test-date")

// Test the version injection logic that would be used in main
// This simulates the logic: lsp.Version = Version, etc.
lspVersion := Version
lspCommit := Commit
lspDate := Date
lspVersion, lspCommit, lspDate := lsp.BuildInfo()

if lspVersion != "test-version" {
t.Errorf("Expected LSP version test-version, got %s", lspVersion)
Expand Down
134 changes: 133 additions & 1 deletion forst/internal/gowork/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ func TestWriteRunGoMod_absoluteReplaceWhenCrossTree(t *testing.T) {
if !strings.Contains(s, want) {
t.Fatalf("missing absolute replace forst => %s:\n%s", compilerDir, s)
}
if _, err := exec.LookPath("go"); err == nil {
if _, err := exec.LookPath("go"); err == nil {
cmd := exec.Command("go", "mod", "tidy")
cmd.Dir = sandbox
cmd.Env = append(os.Environ(), "GOWORK=off")
Expand All @@ -401,3 +401,135 @@ func TestWriteRunGoMod_absoluteReplaceWhenCrossTree(t *testing.T) {
}
}
}

func TestAppendGoModReplaces_appendsAndDedupes(t *testing.T) {
dir := t.TempDir()
pkgDir := filepath.Join(dir, "gen", "api")
if err := os.MkdirAll(pkgDir, 0o755); err != nil {
t.Fatal(err)
}
path := filepath.Join(dir, "go.mod")
if err := os.WriteFile(path, []byte("module example.com/app\n\ngo 1.26.0\n\nreplace demo/api => ./old\n"), 0o644); err != nil {
t.Fatal(err)
}
replaces := []PackageReplace{
{ImportPath: "demo/api", Dir: pkgDir},
{ImportPath: "demo/auth", Dir: filepath.Join(dir, "gen", "auth")},
}
if err := AppendGoModReplaces(path, replaces); err != nil {
t.Fatal(err)
}
data, err := os.ReadFile(path)
if err != nil {
t.Fatal(err)
}
s := string(data)
if strings.Contains(s, "replace demo/api =>") && strings.Count(s, "replace demo/api =>") > 1 {
t.Fatalf("duplicate demo/api replace:\n%s", s)
}
if !strings.Contains(s, "replace demo/auth =>") {
t.Fatalf("missing demo/auth replace:\n%s", s)
}
}

func TestAppendGoModReplaces_emptySliceNoOp(t *testing.T) {
path := filepath.Join(t.TempDir(), "go.mod")
if err := os.WriteFile(path, []byte("module m\n"), 0o644); err != nil {
t.Fatal(err)
}
if err := AppendGoModReplaces(path, nil); err != nil {
t.Fatal(err)
}
}

func TestAppendGoModReplaces_missingFileErrors(t *testing.T) {
if err := AppendGoModReplaces(filepath.Join(t.TempDir(), "missing.mod"), []PackageReplace{
{ImportPath: "x", Dir: t.TempDir()},
}); err == nil {
t.Fatal("expected error for missing go.mod")
}
}

func TestAppendGoModReplaces_relativePathGetsDotPrefix(t *testing.T) {
dir := t.TempDir()
pkgDir := filepath.Join(dir, "pkg")
if err := os.MkdirAll(pkgDir, 0o755); err != nil {
t.Fatal(err)
}
path := filepath.Join(dir, "go.mod")
if err := os.WriteFile(path, []byte("module m\n\ngo 1.26.0\n"), 0o644); err != nil {
t.Fatal(err)
}
if err := AppendGoModReplaces(path, []PackageReplace{{ImportPath: "demo/pkg", Dir: pkgDir}}); err != nil {
t.Fatal(err)
}
data, err := os.ReadFile(path)
if err != nil {
t.Fatal(err)
}
if !strings.Contains(string(data), "replace demo/pkg => ./pkg") {
t.Fatalf("expected ./pkg prefix:\n%s", data)
}
}

func TestWorkspaceUseDirs(t *testing.T) {
t.Parallel()
_, err := WorkspaceUseDirs("/root", "/session", ForstRuntimeLink{})
if err == nil {
t.Fatal("expected error when ReplaceDir empty")
}
uses, err := WorkspaceUseDirs("/root", "/session", ForstRuntimeLink{ReplaceDir: "/forst"})
if err != nil {
t.Fatal(err)
}
if len(uses) != 2 || uses[0] != "/session" || uses[1] != "/forst" {
t.Fatalf("uses = %#v", uses)
}
}

func TestWriteGoWork_emptyUseDirsErrors(t *testing.T) {
if err := WriteGoWork(filepath.Join(t.TempDir(), "go.work"), nil); err == nil {
t.Fatal("expected error for empty use dirs")
}
}

func TestChildEnv_workspaceModeSetsGOWORK(t *testing.T) {
work := filepath.Join(t.TempDir(), "go.work")
env := ChildEnv([]string{"GOWORK=/parent/go.work"}, LinkPlan{
Mode: LinkWorkspace,
Workspace: work,
}, "/app")
found := false
for _, e := range env {
if e == "GOWORK="+work {
found = true
}
if strings.HasPrefix(e, "GOWORK=") && e != "GOWORK="+work {
t.Fatalf("unexpected GOWORK: %s", e)
}
}
if !found {
t.Fatalf("expected GOWORK=%s in %v", work, env)
}
}

func TestChildEnv_stripsReadonlyGOFLAGS(t *testing.T) {
env := ChildEnv([]string{"GOFLAGS=-mod=readonly -v"}, LinkPlan{Mode: LinkReplace}, "")
for _, e := range env {
if strings.Contains(e, "-mod=readonly") {
t.Fatalf("readonly GOFLAGS leaked: %s", e)
}
}
}

func TestGoModReplaceNeedsAbsolute(t *testing.T) {
if !goModReplaceNeedsAbsolute("/private/var/tmp", "/home/example/module") {
t.Fatal("expected absolute replace when sandbox is under /private and target is not")
}
if goModReplaceNeedsAbsolute("/a/b", "relative") {
t.Fatal("relative target should not need absolute")
}
if !goModReplaceNeedsAbsolute("/a/b", "/c/d") {
t.Fatal("expected absolute replace when mod and target roots differ")
}
}
Loading
Loading