Fix sessions download review findings

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
pelikhan · Oct 3, 2026 · Oct 3, 2026 · Oct 3, 2026 · Oct 3, 2026 · Oct 3, 2026
commit acf118257cda8aa9f95ee8bbfe6192725176d1ad
diff --git a/actions/setup/js/session_cli.cjs b/actions/setup/js/session_cli.cjs
@@ -23,7 +23,7 @@ function readSessionInput(inputPath) {
  * @returns {string}
  */
 function sessionCLI(args) {
-  const [mode, inputPath, engine] = args;
+  const [mode, inputPath, engine, customParser] = args;
   if (!inputPath) throw new Error("A session input path is required");
   if (mode === "reconstruct") {
     const { events, maskedValues } = collectUnifiedSession({
@@ -53,6 +53,20 @@ function sessionCLI(args) {
   }
   if (mode === "agent-markdown") {
     if (!engine) throw new Error("An engine is required to parse an agent log");
+    if (customParser) {
+      if (customParser !== "behavior_log_parser.cjs") throw new Error("Invalid custom log parser");
+      const parsed = require("./" + customParser).parseLog(readSessionInput(inputPath));
+      if (typeof parsed === "string") return parsed + "\n";
+      if (Array.isArray(parsed?.mcpFailures) && parsed.mcpFailures.length > 0) {
+        throw new Error(`MCP server(s) failed to launch: ${parsed.mcpFailures.join(", ")}`);
+      }
+      if (parsed?.maxTurnsHit) throw new Error("Agent execution stopped: max-turns limit reached");
+      if (Array.isArray(parsed?.logEntries) && parsed.logEntries.length > 0) {
+        return generateCopilotCliStyleSummary(parsed.logEntries) + "\n";
+      }
+      if (typeof parsed?.markdown === "string") return parsed.markdown + "\n";
+      return generateCopilotCliStyleSummary(parsed?.logEntries ?? []) + "\n";
+    }
     return generateCopilotCliStyleSummary(parseEngineSession(readSessionInput(inputPath), engine)) + "\n";
   }
   throw new Error(`Unsupported session CLI mode: ${mode}`);

diff --git a/actions/setup/session_parsers.go b/actions/setup/session_parsers.go
@@ -6,8 +6,8 @@ import "embed"
 // test fixtures or workflow handlers. The CLI uses the same sources as Actions.
 //
 //go:embed js/session_cli.cjs js/unified_session.cjs js/unified_session_payload.cjs js/unified_session_render.cjs
-//go:embed js/parse_claude_log.cjs js/parse_codex_log.cjs js/parse_copilot_log.cjs
-//go:embed js/parse_custom_log.cjs js/parse_gemini_log.cjs js/parse_pi_log.cjs
+//go:embed js/parse_claude_log.cjs js/parse_codex_log.cjs js/parse_copilot_log.cjs js/parse_gemini_log.cjs
+//go:embed js/parse_custom_log.cjs js/parse_pi_log.cjs
 //go:embed js/agent_session.cjs js/agent_session_render.cjs js/claude_session.cjs
 //go:embed js/codex_session.cjs js/codex_log_framing.cjs js/copilot_session.cjs
 //go:embed js/pi_session.cjs js/pi_session_redaction.cjs js/session_artifact.cjs

diff --git a/pkg/cli/logs_parsing_javascript.go b/pkg/cli/logs_parsing_javascript.go
@@ -51,7 +51,24 @@ func parseAgentLog(runDir string, engine workflow.CodingAgentEngine, verbose boo
 		return nil
 	}
 
-	output, err := runSessionParser(context.Background(), "agent-markdown", agentLogPath, engine.GetID())
+	var output []byte
+	var err error
+	if behaviorEngine, ok := engine.(*workflow.BehaviorDefinedEngine); ok {
+		if parserSource := behaviorEngine.GetLogParserScriptSource(); parserSource != "" {
+			output, err = runSessionParserWithSources(
+				context.Background(),
+				map[string][]byte{"behavior_log_parser.cjs": []byte(parserSource)},
+				"agent-markdown",
+				agentLogPath,
+				engine.GetID(),
+				"behavior_log_parser.cjs",
+			)
+		} else {
+			output, err = runSessionParser(context.Background(), "agent-markdown", agentLogPath, engine.GetID())
+		}
+	} else {
+		output, err = runSessionParser(context.Background(), "agent-markdown", agentLogPath, engine.GetID())
+	}
 	if err != nil {
 		return err
 	}

diff --git a/pkg/cli/session_parser.go b/pkg/cli/session_parser.go
@@ -14,6 +14,10 @@ import (
 )
 
 func runSessionParser(ctx context.Context, args ...string) ([]byte, error) {
+	return runSessionParserWithSources(ctx, nil, args...)
+}
+
+func runSessionParserWithSources(ctx context.Context, additionalSources map[string][]byte, args ...string) ([]byte, error) {
 	tempDir, err := os.MkdirTemp("", "gh-aw-session-parser-*")
 	if err != nil {
 		return nil, fmt.Errorf("failed to create session parser directory: %w", err)
@@ -35,6 +39,14 @@ func runSessionParser(ctx context.Context, args ...string) ([]byte, error) {
 	}); err != nil {
 		return nil, fmt.Errorf("failed to prepare session parser sources: %w", err)
 	}
+	for name, content := range additionalSources {
+		if filepath.Base(name) != name || name == "." || name == ".." {
+			return nil, fmt.Errorf("invalid additional session parser source name %q", name)
+		}
+		if err := os.WriteFile(filepath.Join(tempDir, name), content, constants.FilePermSensitive); err != nil {
+			return nil, fmt.Errorf("failed to prepare additional session parser source %s: %w", name, err)
+		}
+	}
 
 	cmd := exec.CommandContext(ctx, "node", append([]string{filepath.Join(tempDir, "session_cli.cjs")}, args...)...)
 	cmd.Dir = tempDir

diff --git a/pkg/cli/session_parser_test.go b/pkg/cli/session_parser_test.go
@@ -60,3 +60,62 @@ func TestParseAgentLogUsesSharedSessionParser(t *testing.T) {
 	require.NoError(t, err)
 	require.Contains(t, string(markdown), "Audit parser works")
 }
+
+func TestParseAgentLogUsesBehaviorDefinedParser(t *testing.T) {
+	t.Parallel()
+	requireSessionTestNode(t)
+	for _, test := range []struct {
+		name       string
+		parser     string
+		wantOutput string
+		wantError  string
+	}{
+		{
+			name: "structured parser",
+			parser: `function parseLog(logContent) {
+  return { markdown: "### Parser markdown", logEntries: [{ type: "assistant.message", data: { content: "Custom structured finding" } }], mcpFailures: [], maxTurnsHit: false };
+}`,
+			wantOutput: "Custom structured finding",
+		},
+		{
+			name: "MCP failure",
+			parser: `function parseLog(logContent) {
+  return { markdown: "### Parser markdown", logEntries: [], mcpFailures: ["custom server"], maxTurnsHit: false };
+}`,
+			wantError: "MCP server(s) failed to launch: custom server",
+		},
+		{
+			name: "max turns",
+			parser: `function parseLog(logContent) {
+  return { markdown: "### Parser markdown", logEntries: [], mcpFailures: [], maxTurnsHit: true };
+}`,
+			wantError: "max-turns limit reached",
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			root := t.TempDir()
+			writeSessionTestFile(t, root, "agent-stdio.log", "unrecognized custom log format\n")
+			engine, err := workflow.NewBehaviorDefinedEngine(&workflow.EngineDefinition{
+				ID:          "testparser",
+				DisplayName: "TestParser",
+				Behaviors: &workflow.EngineBehaviorDefinition{
+					Execution: &workflow.EngineExecutionDefinition{
+						CommandName: "testparser-cli",
+						StepName:    "Execute TestParser",
+					},
+					LogParser: test.parser,
+				},
+			})
+			require.NoError(t, err)
+			err = parseAgentLog(root, engine, false)
+			if test.wantError != "" {
+				require.ErrorContains(t, err, test.wantError)
+				return
+			}
+			require.NoError(t, err)
+			markdown, err := os.ReadFile(filepath.Join(root, "log.md"))
+			require.NoError(t, err)
+			require.Contains(t, string(markdown), test.wantOutput)
+		})
+	}
+}
diff --git a/pkg/cli/sessions_command.go b/pkg/cli/sessions_command.go
@@ -1,8 +1,10 @@
 package cli
 
 import (
+	"errors"
 	"fmt"
 	"os"
+	"path/filepath"
 
 	"github.com/github/gh-aw/pkg/constants"
 	"github.com/github/gh-aw/pkg/parser"
@@ -59,6 +61,7 @@ func runSessionsDownloadCommand(cmd *cobra.Command, args []string) error {
 	}
 	repoFlag, _ := cmd.Flags().GetString("repo")
 	if repoFlag != "" && components.Owner == "" {
+		// Session downloads also accept host-prefixed slugs for GHES and Proxima.
 		ownerRepo, host := repoutil.NormalizeRepoForAPI(repoFlag)
 		owner, repo, err := repoutil.SplitRepoSlug(ownerRepo)
 		if err != nil {
@@ -73,11 +76,36 @@ func runSessionsDownloadCommand(cmd *cobra.Command, args []string) error {
 	}
 	output, _ := cmd.Flags().GetString("output")
 	if output != "" {
-		if err := writeFileAtomically(output, content); err != nil {
+		if err := writeSessionFileAtomically(output, content); err != nil {
 			return fmt.Errorf("failed to write session to %s: %w", output, err)
 		}
 		return nil
 	}
 	_, err = cmd.OutOrStdout().Write(content)
 	return err
 }
+
+func writeSessionFileAtomically(path string, content []byte) error {
+	file, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".*")
+	if err != nil {
+		return err
+	}
+	tempPath := file.Name()
+	defer func() { _ = os.Remove(tempPath) }()
+	if _, err := file.Write(content); err != nil {
+		return errors.Join(err, file.Close())
+	}
+	mode := constants.FilePermSensitive
+	if info, err := os.Stat(path); err == nil {
+		mode &= info.Mode().Perm()
+	} else if !os.IsNotExist(err) {
+		return errors.Join(err, file.Close())
+	}
+	if err := file.Chmod(mode); err != nil {
+		return errors.Join(err, file.Close())
+	}
+	if err := file.Close(); err != nil {
+		return err
+	}
+	return os.Rename(tempPath, path)
+}
diff --git a/pkg/cli/sessions_download.go b/pkg/cli/sessions_download.go
@@ -125,6 +125,9 @@ func sessionArtifactName(names []string, base, legacy string) (string, error) {
 	}
 	var matches []string
 	for _, name := range names {
+		if base == "info" && (strings.HasSuffix(name, "-aw-info") || strings.HasSuffix(name, "-aw_info")) {
+			continue
+		}
 		if strings.HasSuffix(name, "-"+base) || (legacy != "" && strings.HasSuffix(name, "-"+legacy)) {
 			matches = append(matches, name)
 		}

diff --git a/pkg/cli/sessions_download_test.go b/pkg/cli/sessions_download_test.go
@@ -8,6 +8,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"runtime"
 	"strconv"
 	"strings"
 	"testing"
@@ -109,7 +110,7 @@ func TestSessionsDownloadUsageFirst(t *testing.T) {
 	require.NoError(t, err)
 	require.Contains(t, string(commands), "repos/owner/repo/actions/runs/123/artifacts")
 	require.Contains(t, string(commands), "--name usage")
-	require.Contains(t, string(commands), "-R owner/repo")
+	require.Regexp(t, `-R ([^ ]*/)?owner/repo`, string(commands))
 	require.NotContains(t, string(commands), "--name agent")
 }
 
@@ -174,6 +175,21 @@ func TestSessionsDownloadLegacyAgentUsesInfo(t *testing.T) {
 	require.Contains(t, string(commands), "--name info")
 }
 
+func TestSessionsDownloadWithoutEngineMetadata(t *testing.T) {
+	requireSessionTestNode(t)
+	output, err := func() (string, error) {
+		installSessionTestGH(t, map[string]map[string]string{
+			"agent": {
+				"pi-streaming.jsonl": `{"type":"message_end","message":{"role":"assistant","content":[{"type":"text","text":"Metadata-free Pi session"}]}}` + "\n",
+			},
+		})
+		return executeSessionTestCommand(t, "123", "--repo", "owner/repo")
+	}()
+	require.NoError(t, err)
+	require.NoError(t, validateSessionJSONL([]byte(output)))
+	require.Contains(t, output, "Metadata-free Pi session")
+}
+
 func TestSessionsDownloadPreviousVersions(t *testing.T) {
 	requireSessionTestNode(t)
 	for _, test := range []struct {
@@ -237,6 +253,37 @@ func TestSessionsDownloadMarkdownAndOutput(t *testing.T) {
 	}
 }
 
+func TestSessionsDownloadOutputFilePermissions(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("file permission bits are only enforced on Unix")
+	}
+	content := sessionTestHeader + sessionTestAgentEvent
+	tests := []struct {
+		name         string
+		existingMode os.FileMode
+		create       bool
+		wantMode     os.FileMode
+	}{
+		{name: "new file", wantMode: constants.FilePermSensitive},
+		{name: "restrict public file", existingMode: constants.FilePermPublic, create: true, wantMode: constants.FilePermSensitive},
+		{name: "preserve restrictive file", existingMode: 0o400, create: true, wantMode: 0o400},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			installSessionTestGH(t, map[string]map[string]string{"usage": {"aw_session.jsonl": content}})
+			file := filepath.Join(t.TempDir(), "session.jsonl")
+			if test.create {
+				require.NoError(t, os.WriteFile(file, []byte("old"), test.existingMode))
+			}
+			_, err := executeSessionTestCommand(t, "123", "--repo", "owner/repo", "-o", file)
+			require.NoError(t, err)
+			info, err := os.Stat(file)
+			require.NoError(t, err)
+			require.Equal(t, test.wantMode, info.Mode().Perm())
+		})
+	}
+}
+
 func TestSessionsDownloadErrors(t *testing.T) {
 	tests := []struct {
 		name      string
@@ -324,6 +371,13 @@ func TestSessionArtifactName(t *testing.T) {
 	}
 	_, err := sessionArtifactName([]string{"one-usage", "two-usage"}, "usage", "")
 	require.ErrorContains(t, err, "multiple usage artifacts")
+
+	name, err := sessionArtifactName([]string{"caller-info", "caller-aw-info"}, "info", "")
+	require.NoError(t, err)
+	require.Equal(t, "caller-info", name)
+	name, err = sessionArtifactName([]string{"caller-info", "caller-aw-info"}, "aw-info", "aw_info")
+	require.NoError(t, err)
+	require.Equal(t, "caller-aw-info", name)
 }
 
 func TestValidateSessionJSONL(t *testing.T) {

diff --git a/pkg/workflow/behavior_defined_engine.go b/pkg/workflow/behavior_defined_engine.go
@@ -475,6 +475,11 @@ func (e *BehaviorDefinedEngine) GetLogParserScriptId() string {
 	return e.logParserScriptFilename()
 }
 
+// GetLogParserScriptSource returns the generated parser source for local parsing.
+func (e *BehaviorDefinedEngine) GetLogParserScriptSource() string {
+	return e.buildWrappedLogParserScript()
+}
+
 // buildLogParserWriteStep generates a GitHub Actions step that writes the
 // behavior-defined engine's log-parser script to
 // ${RUNNER_TEMP}/gh-aw/actions/<engine-id>_log_parser.cjs so the post-agent