feat: support optional time in macOS/iOS/iPadOS update deadline#47005
Open
robbiet480 wants to merge 7 commits into
Open
feat: support optional time in macOS/iOS/iPadOS update deadline#47005robbiet480 wants to merge 7 commits into
robbiet480 wants to merge 7 commits into
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for specifying an explicit time component in Apple OS update deadline values, and aligns backend/frontend/docs to accept and describe the expanded formats.
Changes:
- Backend now parses
deadlineas either date-only or date-time, with helpers to detect and extract parts of the value. - Nudge (pre-macOS 14) behavior preserves backward compatibility while allowing explicit UTC times.
- Frontend validation and docs updated to accept/display date-time deadline formats.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/fleet/nudge.go | Uses parsed deadline date-time and applies conditional time handling for Nudge configs. |
| server/fleet/errors.go | Expands deadline validation error message to list supported formats. |
| server/fleet/app.go | Adds parsing helpers for deadline date/time and expands validation formats. |
| frontend/pages/ManageControlsPage/OSUpdates/components/AppleOSTargetForm/AppleOSTargetForm.tsx | Updates deadline validation regex and user-facing tooltip/help text. |
| ee/server/service/mdm.go | Attempts to pass user-specified deadline time through to DDM declaration (macOS 14+). |
| docs/Configuration/yaml-files.md | Documents accepted deadline formats and default time behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Allows the deadline field to accept an optional time component in addition to the existing date-only format. Supported formats: YYYY-MM-DD (date-only, backward compatible) YYYY-MM-DDTHH:MM (date + hour:minute) YYYY-MM-DDTHH:MM:SS (date + hour:minute:second) When no time is specified: - macOS 14+ (DDM): defaults to noon local time (T12:00:00) - Older macOS (Nudge): defaults to 20:00 UTC (noon PST) When a time IS specified: - DDM: interpreted as local time on the device - Nudge: interpreted as UTC Changes: - server/fleet/app.go: accept 3 deadline formats in Validate() and deadlineDateTimeLayouts; add HasDeadlineTime/DeadlineDateTime helpers - server/fleet/nudge.go: use parsed time from deadline instead of hardcoded 20:00 UTC when time is explicitly set - ee/server/service/mdm.go: use parsed time in DDM TargetLocalDateTime instead of hardcoded T12:00:00 - server/fleet/errors.go: update validation error message - frontend: update deadline validation regex and help text - docs: document the new time format options
robbiet480
force-pushed
the
feat/macos-deadline-time
branch
from
95bbea8 to
d91e0df
Compare
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis PR extends Apple OS update deadline handling to accept YYYY-MM-DD, YYYY-MM-DDTHH:MM, and YYYY-MM-DDTHH:MM:SS formats. Backend adds DeadlineDateTime(), DeadlineDatePart(), and HasDeadlineTime(), and updates validation and the invalid-deadline message. Frontend validation and help text are updated. MDM declaration now appends T12:00:00 only when no time is provided; nudge config converts explicit times to UTC and defaults date-only deadlines to 20:00 UTC. Tests are added/expanded and .worktrees/ is ignored in .gitignore. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In `@ee/server/service/mdm.go`:
- Around line 1457-1462: The code currently always uses
updates.DeadlineDatePart(), which strips the time portion; change the logic to
use the full deadline value when a time is provided: if
updates.HasDeadlineTime() is true, set deadlineValue = updates.Deadline.Value
(or the equivalent full string returned by the Deadline field) so the
user-specified time is preserved; otherwise keep the existing behavior of using
updates.DeadlineDatePart() + "T12:00:00". Update the assignment that uses
DeadlineDatePart(), referencing the updates.HasDeadlineTime() check and the
Deadline.Value (or getter) to select the correct value.
In
`@frontend/pages/ManageControlsPage/OSUpdates/components/AppleOSTargetForm/AppleOSTargetForm.tsx`:
- Line 222: The JSX tooltip string in AppleOSTargetForm uses the HTML entity
' instead of a direct apostrophe, so update the tooltip prop value (the
string passed to tooltip in AppleOSTargetForm) to use a straight single quote
character (') in place of ' to ensure correct rendering and cleaner JSX.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fa356205-78d8-426e-95d3-8ba1547511dd
⛔ Files ignored due to path filters (1)
docs/Configuration/yaml-files.mdis excluded by!**/*.md
📒 Files selected for processing (5)
ee/server/service/mdm.gofrontend/pages/ManageControlsPage/OSUpdates/components/AppleOSTargetForm/AppleOSTargetForm.tsxserver/fleet/app.goserver/fleet/errors.goserver/fleet/nudge.go
robbiet480
commented
Jun 6, 2026
Author
robbiet480
left a comment
robbiet480
left a comment
There was a problem hiding this comment.
All review comments addressed in the latest push:
- Copilot (3 issues): All 3 are now fixed — mdm.go time drop bug, app.go duplicate parsing, and frontend help text.
- CodeRabbit (2 issues): mdm.go time drop was already fixed in rebase;
'entity replaced with direct'in JSX tooltip.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.worktrees/public-ip-vital (1)
87-106: ⚡ Quick winAdd an explicit rejection test for the removed vitals.
Removing the positive cases updates coverage, but it doesn't lock the new contract. A negative test asserting
hardware_modelandhardware_vendorare now rejected would catch accidental reintroduction or partial validation drift.🤖 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 @.worktrees/public-ip-vital around lines 87 - 106, Add a negative test inside TestCalculateHostVitalsQueryDomesticVitals that asserts the removed vitals "hardware_model" and "hardware_vendor" are rejected: call the same function/path that builds/validates vitals (the code under test exercised by TestCalculateHostVitalsQueryDomesticVitals) with vital="hardware_model" and vital="hardware_vendor" and assert it returns an error or a rejection (e.g., no query produced or explicit validation failure) rather than building a SELECT; ensure the assertions mirror other table-driven cases (use the same test harness and expected failure checks) so accidental reintroduction of these vitals will fail the test.
🤖 Prompt for all review comments with 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.
Inline comments:
In @.codex/hooks/guard-dangerous-commands.sh:
- Around line 30-34: The current guard uses a single regex that requires
--force/-f to appear before the branch name, so commands like "git push origin
main --force" slip through; update the check that examines the COMMAND variable
(the existing echo "$COMMAND" | grep -qiE ... block) to instead ensure both "git
push" and a branch token (main|master) and a force flag (--force|-f) are present
anywhere in the command—either by splitting into three greps (one for git push,
one for (main|master), one for (--force|-f)) and blocking when all three match,
or by using a single regex that allows the force flag and branch to appear in
any order; keep the error message and exit behavior the same.
In @.codex/hooks/lint-on-save.sh:
- Around line 1-5: The hook script declares PostToolUse but the hooks registry
only registers PreToolUse, so add a PostToolUse entry in the hooks config that
matches shell hook scripts and includes Edit/Write matchers so the script can
run on save; update the registry to reference the existing PostToolUse action
name and point it to the lint/formatter hook script (the script invoking
PostToolUse), ensuring the new entry mirrors the PreToolUse wiring but uses
PostToolUse and includes the correct matchers for on-save runs.
---
Nitpick comments:
In @.worktrees/public-ip-vital:
- Around line 87-106: Add a negative test inside
TestCalculateHostVitalsQueryDomesticVitals that asserts the removed vitals
"hardware_model" and "hardware_vendor" are rejected: call the same function/path
that builds/validates vitals (the code under test exercised by
TestCalculateHostVitalsQueryDomesticVitals) with vital="hardware_model" and
vital="hardware_vendor" and assert it returns an error or a rejection (e.g., no
query produced or explicit validation failure) rather than building a SELECT;
ensure the assertions mirror other table-driven cases (use the same test harness
and expected failure checks) so accidental reintroduction of these vitals will
fail the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: edc1fdc0-847b-4cde-acdb-bcd20cb41267
⛔ Files ignored due to path filters (21)
.agents/skills/bump-migration/SKILL.mdis excluded by!**/*.md.agents/skills/cherry-pick/SKILL.mdis excluded by!**/*.md.agents/skills/find-related-tests/SKILL.mdis excluded by!**/*.md.agents/skills/fix-ci/SKILL.mdis excluded by!**/*.md.agents/skills/fleet-gitops/SKILL.mdis excluded by!**/*.md.agents/skills/lint/SKILL.mdis excluded by!**/*.md.agents/skills/new-endpoint/SKILL.mdis excluded by!**/*.md.agents/skills/new-migration/SKILL.mdis excluded by!**/*.md.agents/skills/openspec-apply-change/SKILL.mdis excluded by!**/*.md.agents/skills/openspec-archive-change/SKILL.mdis excluded by!**/*.md.agents/skills/openspec-explore/SKILL.mdis excluded by!**/*.md.agents/skills/openspec-propose/SKILL.mdis excluded by!**/*.md.agents/skills/project/SKILL.mdis excluded by!**/*.md.agents/skills/review-pr/SKILL.mdis excluded by!**/*.md.agents/skills/source-command-opsx-apply/SKILL.mdis excluded by!**/*.md.agents/skills/source-command-opsx-archive/SKILL.mdis excluded by!**/*.md.agents/skills/source-command-opsx-explore/SKILL.mdis excluded by!**/*.md.agents/skills/source-command-opsx-propose/SKILL.mdis excluded by!**/*.md.agents/skills/spec-story/SKILL.mdis excluded by!**/*.md.agents/skills/test/SKILL.mdis excluded by!**/*.mdAGENTS.mdis excluded by!**/*.md
📒 Files selected for processing (10)
.codex/agents/fleet-security-auditor.toml.codex/agents/frontend-reviewer.toml.codex/agents/go-reviewer.toml.codex/config.toml.codex/hooks.json.codex/hooks/goimports.sh.codex/hooks/guard-dangerous-commands.sh.codex/hooks/lint-on-save.sh.codex/hooks/prettier-frontend.sh.worktrees/public-ip-vital
✅ Files skipped from review due to trivial changes (3)
- .codex/config.toml
- .codex/hooks.json
- .codex/hooks/prettier-frontend.sh
robbiet480
force-pushed
the
feat/macos-deadline-time
branch
from
4718060 to
7669120
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #47005 +/- ##
==========================================
+ Coverage 67.04% 67.06% +0.01%
==========================================
Files 2894 2895 +1
Lines 225026 225268 +242
Branches 11772 11774 +2
==========================================
+ Hits 150867 151070 +203
- Misses 60489 60515 +26
- Partials 13670 13683 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In
`@frontend/pages/ManageControlsPage/OSUpdates/components/AppleOSTargetForm/AppleOSTargetForm.tsx`:
- Around line 37-43: The validateDeadline function currently only checks format
with regex and accepts impossible dates (e.g. 2026-02-31); update
validateDeadline to first test the string format then parse the date components
(year, month, day, optional time) and construct a Date object to verify the
calendar date matches the input (e.g., createdDate.getFullYear() === year,
getMonth()+1 === month, getDate() === day) and reject when they differ; apply
the same validation path for both date-only and datetime inputs so invalid
calendar dates are rejected client-side.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 50d282a0-3133-4c54-8bc9-a2ae99c017b6
⛔ Files ignored due to path filters (1)
docs/Configuration/yaml-files.mdis excluded by!**/*.md
📒 Files selected for processing (6)
.gitignoreee/server/service/mdm.gofrontend/pages/ManageControlsPage/OSUpdates/components/AppleOSTargetForm/AppleOSTargetForm.tsxserver/fleet/app.goserver/fleet/errors.goserver/fleet/nudge.go
Add test cases for: - Valid deadline formats with HH:MM and HH:MM:SS time components - Invalid deadline formats (hour > 24, minute >= 60) - HasDeadlineTime(), DeadlineDatePart(), DeadlineDateTime() helpers
Author
|
@coderabbitai resume |
Contributor
✅ Action performedReviews resumed. |
- Validate() now delegates to DeadlineDateTime() instead of duplicating the parse loop - DeadlineDatePart() returns empty string for inputs shorter than 10 characters - Docs updated to mention YYYY-MM-DDTHH:MM format alongside HH:MM:SS
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Allows the
deadlinefield inmacos_updates,ios_updates, andipados_updatesto accept an optional time component (HH:MM or HH:MM:SS) in addition to the existing date-only format.Problem
Currently the deadline only accepts
YYYY-MM-DD. The time is hardcoded:Admins have no way to specify a different cutoff time, e.g. end of business day.
Solution
The deadline field now accepts three formats:
YYYY-MM-DD— date-only, backward compatible, time defaults to existing behaviorYYYY-MM-DDTHH:MM— date + hour:minuteYYYY-MM-DDTHH:MM:SS— date + hour:minute:secondWhen a time is explicitly specified:
Changes
server/fleet/app.goValidate(); addHasDeadlineTime(),DeadlineDateTime(),DeadlineDatePart()helpersserver/fleet/nudge.goee/server/service/mdm.goTargetLocalDateTime, fall back to T12:00:00server/fleet/errors.gofrontend/.../AppleOSTargetForm.tsxdocs/Configuration/yaml-files.mdSummary by CodeRabbit
New Features
Improvements
Documentation
Tests
Chore