Skip to content

fix: expose currentTime and utcOffset to template logic in trigger() and init()#116

Open
Sup-ri-tha wants to merge 1 commit intoaccordproject:mainfrom
Sup-ri-tha:fix/currentTime-argumentNames
Open

fix: expose currentTime and utcOffset to template logic in trigger() and init()#116
Sup-ri-tha wants to merge 1 commit intoaccordproject:mainfrom
Sup-ri-tha:fix/currentTime-argumentNames

Conversation

@Sup-ri-tha
Copy link

@Sup-ri-tha Sup-ri-tha commented Mar 21, 2026

Fixes #115

Problem

trigger() and init() in TemplateArchiveProcessor pass currentTime and utcOffset in the arguments array but omit them from argumentNames.

evalDangerously() builds new Function(...argumentNames, code) - only named arguments are accessible inside the function. As a result, currentTime and utcOffset silently become undefined with no error or warning. [Confirmed via console.log in logic.ts , currentTime is undefined despite being passed by the caller.]

Proof

template-engine-bug

Fix

Added missing names to argumentNames in both trigger() and init():

// trigger()
argumentNames: ['data', 'request', 'state', 'currentTime', 'utcOffset'],

// init()
argumentNames: ['data', 'currentTime', 'utcOffset'],

Also fixed JSDoc typo: "UTC offer" → "UTC offset" in both methods.

Testing

All existing tests pass. The two failing tests (stress test, optional-nested) are pre-existing and unrelated to this change.

Checklist

  • Code follows project conventions
  • All tests pass
  • DCO sign-off included

Copilot AI review requested due to automatic review settings March 21, 2026 18:33
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to make currentTime and utcOffset available to template logic executed by TemplateArchiveProcessor.trigger() and init(), and corrects a JSDoc typo.

Changes:

  • Add currentTime/utcOffset to the argumentNames arrays passed to evalDangerously() in trigger() and init().
  • Fix JSDoc typo: “UTC offer” → “UTC offset”.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 120 to 123
code: compiledCode['logic/logic.ts'].code, // TODO DCS - how to find the code to run?
argumentNames: ['data', 'request', 'state'],
argumentNames: ['data', 'request', 'state', 'currentTime', 'utcOffset'],
arguments: [data, request, state, currentTime, utcOffset]
});
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

argumentNames is not used by JavaScriptEvaluator.evalDangerously when templateLogic: true (it dynamic-imports the module and calls instance[functionName](...arguments)). As a result, adding currentTime/utcOffset here will not make them available as in-scope variables inside template logic unless the template logic method signature explicitly declares them. If the intent is to inject these into scope, the evaluator/path needs to change accordingly (or the logic API/types should be updated to include these optional parameters).

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

That's right. argumentNames is not used in the templateLogic path. Reverted those changes. Instead applied defaults for currentTime and utcOffset before passing them in arguments.

Comment on lines 121 to 123
argumentNames: ['data', 'request', 'state', 'currentTime', 'utcOffset'],
arguments: [data, request, state, currentTime, utcOffset]
});
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

The JSDoc says currentTime/utcOffset “default to now/zero”, but this method currently forwards the optional values as-is. If callers omit them, template logic will receive undefined; consider defaulting currentTime to new Date().toISOString() and utcOffset to 0 before constructing the arguments array to match the documented behavior.

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

Done. currentTime now defaults to new Date().toISOString() and utcOffset to 0 when not provided.

Comment on lines 168 to 172
functionName: 'init',
code: compiledCode['logic/logic.ts'].code, // TODO DCS - how to find the code to run?
argumentNames: ['data'],
argumentNames: ['data', 'currentTime', 'utcOffset'],
arguments: [data, currentTime, utcOffset]
});
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

argumentNames is not used by JavaScriptEvaluator.evalDangerously when templateLogic: true (module import + method call uses only arguments). This change likely won’t affect whether currentTime/utcOffset are accessible inside init() logic unless they are declared as parameters on the template logic method (or the evaluator/path is changed).

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

Reverted those changes and instead applied defaults for currentTime and utcOffset before passing them in arguments.

Comment on lines 170 to 172
argumentNames: ['data', 'currentTime', 'utcOffset'],
arguments: [data, currentTime, utcOffset]
});
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

The JSDoc says currentTime/utcOffset “default to now/zero”, but init() currently forwards the optional values as-is. Consider applying defaults before building the arguments array so template logic gets deterministic values when callers omit them.

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

Done. currentTime now defaults to new Date().toISOString() and utcOffset to 0 when not provided.

* @param {[string]} currentTime - the current time, defaults to now
* @param {[number]} utcOffset - the UTC offer, defaults to zero
* @param {[number]} utcOffset - the UTC offset, defaults to zero
* @returns {Promise} the response and any events
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

The init() JSDoc @returns says “the response and any events”, but InitResponse in this file only contains state (no result/events). Please update the @returns description to match the actual return shape.

Suggested change
* @returns {Promise} the response and any events
* @returns {Promise<InitResponse>} the new state

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

Done.

…and init()

Signed-off-by: Supritha Rajashekar <supritharajashekar10@gmail.com>
@Sup-ri-tha Sup-ri-tha force-pushed the fix/currentTime-argumentNames branch from 4f5211f to 984bb79 Compare March 21, 2026 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: currentTime and utcOffset parameters are inaccessible inside template logic

2 participants