Add hook environment variables and break duration #47

justincampbell · 2025-09-30T19:55:31Z

Summary

Add environment variables to hooks: POMODORO_ID, POMODORO_DIRECTORY, POMODORO_COMMAND, POMODORO_ARGS
Add break duration environment variables: POMODORO_BREAK_DURATION_MINUTES, POMODORO_BREAK_DURATION_SECONDS
Make break and finish --break wait by default (can disable with --wait=false)
Refactor hook.Run() to use a Params struct for better maintainability

Changes

Enhanced hook system with contextual environment variables for better integration
Hooks can now access pomodoro ID, directory, command, and arguments
Break hooks receive duration information in both minutes and seconds
Improved user experience: breaks now block by default with countdown timer
Code quality improvements: cleaner API with params struct, use of library constants

Test plan

All 117 existing tests pass
Added 3 new tests for break duration environment variables
Verified hooks receive correct environment variables
Tested --wait=false flag to skip countdown

🤖 Generated with Claude Code

Hooks now receive context via environment variables: - POMODORO_ID: Timestamp/ID of the current pomodoro - POMODORO_DIRECTORY: Data directory path - POMODORO_COMMAND: Command that triggered the hook - POMODORO_ARGS: Full command-line arguments including flags This enables hooks to query pomodoro details using the show command: desc=$(pomodoro --directory "$POMODORO_DIRECTORY" show description "$POMODORO_ID") 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

- Add POMODORO_BREAK_DURATION_MINUTES and POMODORO_BREAK_DURATION_SECONDS environment variables to break hooks - Make break and finish --break wait by default (can disable with --wait=false) - Add shouldWait() helper to determine wait behavior based on context - Update hook.Run() to accept breakDuration parameter - Add tests for break duration environment variables 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

Replace the 6-parameter function signature with a Params struct for better maintainability and readability.

Copilot

Pull Request Overview

This PR enhances the hook system by adding contextual environment variables and improving the break command behavior. It makes hooks more useful by providing access to pomodoro metadata and makes break commands more user-friendly by waiting by default.

Add environment variables to hooks: POMODORO_ID, POMODORO_DIRECTORY, POMODORO_COMMAND, POMODORO_ARGS, and break duration variables
Refactor hook system to use a structured Params approach instead of just passing hook name
Make break commands wait by default with option to disable via --wait=false

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
hook/hook.go	Core hook system refactored to use Params struct and add environment variable support
cmd/util.go	Added utility functions for wait behavior and command argument extraction
cmd/start.go	Updated to use new hook.Params structure with pomodoro metadata
cmd/repeat.go	Updated to use new hook.Params structure with pomodoro metadata
cmd/finish.go	Updated to use new hook.Params structure and added break duration support
cmd/clear.go	Updated to use new hook.Params structure with pomodoro metadata
cmd/cancel.go	Updated to use new hook.Params structure with pomodoro metadata
cmd/break.go	Updated to use new hook.Params structure and made wait default behavior
test/hooks.bats	Added comprehensive tests for new environment variables and break duration features

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-30T19:56:18Z

cmd/util.go

+// getCommandArgs extracts the command-specific arguments from os.Args
+func getCommandArgs(cmd *cobra.Command) []string {
+	for i, arg := range os.Args {
+		if arg == cmd.Name() || (i > 0 && os.Args[i-1] == "pomodoro") {
+			if arg == cmd.Name() {
+				return os.Args[i+1:]
+			}
+		}
+	}
+	return cmd.Flags().Args()
+}


The getCommandArgs function has complex logic that may not handle all edge cases correctly. The condition (i > 0 && os.Args[i-1] == "pomodoro") on line 65 doesn't result in any action, making it redundant. Consider simplifying this logic or adding comments to explain the intended behavior for different argument patterns.

Copilot · 2025-09-30T19:56:18Z

cmd/break.go

+	if err := hook.Run(client, hook.Params{
+		Name:          "break",
+		Command:       "break",
+		Args:          getCommandArgs(cmd),
+		BreakDuration: d,
+	}); err != nil {


The PomodoroID field is missing from the hook.Params struct, unlike other commands that include it. This inconsistency means break hooks won't have access to the pomodoro ID environment variable, which could be useful for tracking which pomodoro the break is associated with.

Copilot · 2025-09-30T19:56:18Z

cmd/break.go

+		Name:    "stop",
+		Command: "break",
+		Args:    getCommandArgs(cmd),


The second hook call is missing both PomodoroID and BreakDuration fields that are provided in other similar hook calls. This creates inconsistency in the environment variables available to the 'stop' hook when called from different commands.

Suggested change

Name: "stop",

Command: "break",

Args: getCommandArgs(cmd),

Name: "stop",

Command: "break",

Args: getCommandArgs(cmd),

BreakDuration: d,

Address Copilot feedback: The stop hook called after a break should have access to the break duration environment variables.

- Simplify POMODORO_ID test to check for ID prefix instead of exact format (handles both -HH:MM and Z timezone formats) - Simplify test 42 to just verify POMODORO_ID is set instead of testing the show command within a hook (avoids timing/buffering issues in CI) Fixes test failures in GitHub Actions on both Ubuntu and macOS.

Copilot

Pull Request Overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Use proper regex to validate RFC3339 timestamp format instead of just checking for '20' prefix. Pattern matches YYYY-MM-DDTHH:MM:SS regardless of timezone format (Z or offset).

Copilot

Pull Request Overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-30T20:21:33Z

cmd/break.go

+	return hook.Run(client, hook.Params{
+		Name:          "stop",
+		Command:       "break",
+		Args:          getCommandArgs(cmd),
+		BreakDuration: d,
+	})


The PomodoroID field is missing from the hook.Params struct. This will result in an empty POMODORO_ID environment variable being passed to the stop hook, which is inconsistent with other commands that properly set this field.

justincampbell and others added 5 commits September 30, 2025 13:58

Simplify joinArgs using strings.Join

0572a45

Use TimeFormat constant for timestamp formatting

7d334d2

Refactor hook.Run to use Params struct

f92f6cd

Replace the 6-parameter function signature with a Params struct for better maintainability and readability.

Copilot AI review requested due to automatic review settings September 30, 2025 19:55

Copilot AI reviewed Sep 30, 2025

View reviewed changes

justincampbell added 2 commits September 30, 2025 16:04

Pass break duration to stop hook after break

da40e76

Address Copilot feedback: The stop hook called after a break should have access to the break duration environment variables.

Copilot AI review requested due to automatic review settings September 30, 2025 20:09

Copilot AI reviewed Sep 30, 2025

View reviewed changes

Improve POMODORO_ID test validation

544f889

Use proper regex to validate RFC3339 timestamp format instead of just checking for '20' prefix. Pattern matches YYYY-MM-DDTHH:MM:SS regardless of timezone format (Z or offset).

justincampbell requested a review from Copilot September 30, 2025 20:21

Copilot AI reviewed Sep 30, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add hook environment variables and break duration #47

Add hook environment variables and break duration #47

Uh oh!

justincampbell commented Sep 30, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 30, 2025

Uh oh!

Copilot AI Sep 30, 2025

Uh oh!

Copilot AI Sep 30, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Add hook environment variables and break duration #47

Are you sure you want to change the base?

Add hook environment variables and break duration #47

Uh oh!

Conversation

justincampbell commented Sep 30, 2025

Summary

Changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants